From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: APIC MSRs query Date: Tue, 17 May 2011 15:08:10 +0100 Message-ID: <4DD2814A.3050008@citrix.com> References: <4DD27760.9020706@citrix.com> <4DD297B90200007800041A32@vpn.id2.novell.com> <1305640796.20907.58.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1305640796.20907.58.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Campbell Cc: xen devel , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 17/05/11 14:59, Ian Campbell wrote: > On Tue, 2011-05-17 at 14:43 +0100, Jan Beulich wrote: >>>>> On 17.05.11 at 15:25, Andrew Cooper 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. > I couldn't find any errata which is why I asked here. Bits [63:36] are reserved so should be WriteAsZero - it is possible that whoever put it into Linux just wanted to zero the top bits and either missed the top 4 bits or decided that they would never be set. -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com