Re: [Qemu-devel] [SeaBIOS] [seabios PATCH 2/2] allow CPUs to have non-contiguous Local APIC IDs

[Eduardo Habkost]

On Thu, Jul 19, 2012 at 09:12:54PM -0400, Kevin O'Connor wrote:
> On Thu, Jul 19, 2012 at 05:52:42PM -0300, Eduardo Habkost wrote:
> > Extract Local APIC IDs directly from the CPUs, and instead of check for
> > "i < CountCPUs", check if the APIC ID was present on boot, when building
> > ACPI tables and the MP-Table.
> > 
> > This keeps ACPI Processor ID == APIC ID, but allows the
> > hardware<->Seabios interface be completely APIC-ID based, and Seabios
> > may change the way ACPI Processor IDs are chosen in the future.
> > 
> > As we currently Seabios support only xAPIC and not x2APIC, the list of
> > present-on-boot APIC IDs is a 256-bit bitmap. If one day Seabios starts
> > to support x2APIC, the data structure used to enumerate the APIC IDs
> > will have to be changed.
> 
> Is this SeaBIOS change dependent on a particular version of QEMU?

No, it's the opposite: QEMU needs this change first, to be able to fix
some CPU socket/core/thread topology bugs caused by incorrect APIC IDs.

For hardware that has contiguous APIC IDs (that's the case for existing
QEMU versions), the OS-visible behavior and resulting tables should be
exactly the same.

> 
> [...]
> > --- a/src/smp.c
> > +++ b/src/smp.c
> > @@ -36,6 +36,8 @@ wrmsr_smp(u32 index, u64 val)
> >  
> >  u32 CountCPUs VAR16VISIBLE;
> >  u32 MaxCountCPUs VAR16VISIBLE;
> > +// 256 bits for the found APIC IDs
> > +u32 FoundAPICIDs[256/32] VAR16VISIBLE;
> >  extern void smp_ap_boot_code(void);
> >  ASM16(
> >      "  .global smp_ap_boot_code\n"
> > @@ -59,6 +61,12 @@ ASM16(
> >      "  jmp 1b\n"
> >      "2:\n"
> >  
> > +    // get apic ID on EBX, set bit on FoundAPICIDs
> > +    "  mov $1, %eax\n"
> > +    "  cpuid\n"
> > +    "  shrl $24, %ebx\n"
> > +    "  lock bts %ebx, FoundAPICIDs\n"
> 
> Because of the insanity that is 16bit mode, please use explicit size
> suffixes on assembler instructions (ie, "movl" and "btsl").

I'll do. Thanks for the feedback.

-- 
Eduardo