From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH][retry 2][2/2] new platform hypervisor call to get APICIDs Date: Tue, 04 Mar 2008 08:59:02 +0000 Message-ID: <47CD1D66.76E4.0078.0@novell.com> References: <200803031540.28151.mark.langsdorf@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <200803031540.28151.mark.langsdorf@amd.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Mark Langsdorf Cc: xen-devel@lists.xensource.com, Chris Lalancette List-Id: xen-devel@lists.xenproject.org In addition to Keir's comment, this >--- a/drivers/xen/core/smpboot.c Mon Mar 03 13:36:57 2008 +0000 >+++ b/drivers/xen/core/smpboot.c Mon Mar 03 14:40:26 2008 -0600 >@@ -254,21 +254,39 @@ static void __cpuinit cpu_initialize_con > spin_unlock(&ctxt_lock); > } >=20 >+int dom0_vcpus_pinned =3D 1; > void __init smp_prepare_cpus(unsigned int max_cpus) > { > unsigned int cpu; > struct task_struct *idle; >+ int apicid; >+ u8 pinned_apicids[NR_CPUS]; is introducing a latent bug. If you follow Keir's suggestion, it'll be = gone. If there's a reason to keep it obtaining all IDs at once, you'll need to convert this to a static, __initdata array. The other thing is that you absolutely shouldn't be limiting the APIC IDs at the Xen interface to 8 bits. With the x2APIC, they'll not be bounded to that smaller range anymore. Finally, I'm worried about the way you handle dom0_vcpus_pinned: By its name, I would assume that if I (later, for whatever reason) use it elsewhere, I can be certain it means what it says. However, at present, the hypercall succeeding does not imply that VCPUs are indeed pinned. Further, under !is_initial_xen_domain() you shouldn't set it, nor should you have a reason to even attempt the hypercall. Jan