Re: [Xen-devel] APIC MSRs query

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2011-05-17 at 14:43 +0100, Jan Beulich wrote:
> >>> On 17.05.11 at 15:25, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> > Hello,
> > 
> > I am currently cleaning up the APIC code for the sake of 
> > shutdown/reboot/crashdump and have a query about the (modified for 
> > brevity) snippet of code:
> > 
> > uint64_t msr_content;
> > rdmsrl(MSR_IA32_APICBASE, msr_content);
> > msr_content |= MSR_IA32_APICBASE_ENABLE | MSR_IA32_APICBASE_EXTD;
> > msr_content = (uint32_t)msr_content;
> > wrmsrl(MSR_IA32_APICBASE, msr_content);
> > 
> > which is added into apic.c in changeset b622e411eef8, and has propagated 
> > elsewhere in the codebase during subsequent cleanups etc.
> > 
> > The MP spec and x2apic spec states that bits [35:12] of 
> > MSR_IA32_APICBASE is the base APIC MMIO address.  Is there reason why 
> > the code (almost always) clears the top 4 bits, or is it just an 
> > overlooked mistake?
> 
> I think this is a benign mistake. Benign because I don't think there is
> a meaningful (to Xen at least) number of systems that would not
> have their LAPIC at the default address (which fits in 32 bits).

That "msr_content = (uint32_t)msr_content;" seems to be pretty
deliberate, what else would it be trying to do? 

FWIW enable_x2apic in Linux seems to have a similar construct which
throws away the top half of the MSR:

void enable_x2apic(void)
{
       int msr, msr2;

       rdmsr(MSR_IA32_APICBASE, msr, msr2);
       if (!(msr & X2APIC_ENABLE)) {
               printk("Enabling x2apic\n");
               wrmsr(MSR_IA32_APICBASE, msr | X2APIC_ENABLE, 0);
       }
}

(FWIW the original Xen code in 17545:9fd00ff95068 looked a lot like this
too, b622e411eef8 just switched to wrmsrl and preserved the clearing
behaviour).

Perhaps there is some errata? Google didn't find one, but ...

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel