Re: [Xen-devel] [PATCH][retry 2][2/2] new platform hypervisor call to get APICIDs

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

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);
> }
> 
>+int dom0_vcpus_pinned = 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


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