On Wed, Jul 25, 2012 at 03:42:21PM -0300, Eduardo Habkost wrote: > On Mon, Jul 23, 2012 at 03:20:14PM +0300, Gleb Natapov wrote: > > On Fri, Jul 20, 2012 at 02:04:50PM -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 not depend > > > on any other kind of "CPU identifier". This way, SeaBIOS may change the > > > way ACPI Processor IDs are chosen in the future. > > > > > > As currently SeaBIOS supports 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 (but this is an internal implementation detail, > > > not visible to the OS or on any hardware<=>SeaBIOS interface). > > > > > > For current QEMU versions (that always make the APIC IDs contiguous), > > > the OS-visible behavior and resulting ACPI tables should be exactly the > > > same. This patch will simply allow QEMU to start setting non-contiguous > > > APIC IDs (that is a requirement for some sockets/cores/threads topology > > > settings). > > > > > > Changes v1 -> v2: > > > - Use size suffixes on all asm instructions on smp.c > > > - New patch description > > > > > > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > > > --- > > > src/acpi-dsdt.dsl | 4 +++- > > > src/acpi.c | 9 +++++---- > > > src/mptable.c | 2 +- > > > src/smp.c | 17 +++++++++++++++++ > > > src/util.h | 1 + > > > 5 files changed, 27 insertions(+), 6 deletions(-) > > > > > > diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl > > > index 2060686..72dc7d8 100644 > > > --- a/src/acpi-dsdt.dsl > > > +++ b/src/acpi-dsdt.dsl > > > @@ -676,6 +676,7 @@ DefinitionBlock ( > > > /* Methods called by run-time generated SSDT Processor objects */ > > > Method (CPMA, 1, NotSerialized) { > > > // _MAT method - create an madt apic buffer > > > + // Arg0 = Processor ID = Local APIC ID > > > // Local0 = CPON flag for this cpu > > > Store(DerefOf(Index(CPON, Arg0)), Local0) > > > // Local1 = Buffer (in madt apic form) to return > > > @@ -688,6 +689,7 @@ DefinitionBlock ( > > > } > > > Method (CPST, 1, NotSerialized) { > > > // _STA method - return ON status of cpu > > > + // Arg0 = Processor ID = Local APIC ID > > > // Local0 = CPON flag for this cpu > > > Store(DerefOf(Index(CPON, Arg0)), Local0) > > > If (Local0) { Return(0xF) } Else { Return(0x0) } > > > @@ -708,7 +710,7 @@ DefinitionBlock ( > > > Store (PRS, Local5) > > > // Local2 = last read byte from bitmap > > > Store (Zero, Local2) > > > - // Local0 = cpuid iterator > > > + // Local0 = Processor ID / APIC ID iterator > > > Store (Zero, Local0) > > > While (LLess(Local0, SizeOf(CPON))) { > > > // Local1 = CPON flag for this cpu > > > diff --git a/src/acpi.c b/src/acpi.c > > > index da3bc57..39b7172 100644 > > > --- a/src/acpi.c > > > +++ b/src/acpi.c > > > @@ -327,7 +327,7 @@ build_madt(void) > > > apic->length = sizeof(*apic); > > > apic->processor_id = i; > > > apic->local_apic_id = i; > > > - if (i < CountCPUs) > > > + if (apic_id_is_present(apic->local_apic_id)) > > > apic->flags = cpu_to_le32(1); > > > else > > > apic->flags = cpu_to_le32(0); > > > @@ -445,6 +445,7 @@ build_ssdt(void) > > > } > > > > > > // build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, > > > Arg1)} ...}" > > > + // Arg0 = Processor ID = APIC ID > > > *(ssdt_ptr++) = 0x14; // MethodOp > > > ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2); > > > *(ssdt_ptr++) = 'N'; > > > @@ -477,7 +478,7 @@ build_ssdt(void) > > > ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2); > > > *(ssdt_ptr++) = acpi_cpus; > > > for (i=0; i<acpi_cpus; i++) > > > - *(ssdt_ptr++) = (i < CountCPUs) ? 0x01 : 0x00; > > > + *(ssdt_ptr++) = (apic_id_is_present(i)) ? 0x01 : 0x00; > > > > > > // store pci io windows: start, end, length > > > // this way we don't have to do the math in the dsdt > > > @@ -656,10 +657,10 @@ build_srat(void) > > > core->proximity_lo = curnode; > > > memset(core->proximity_hi, 0, 3); > > > core->local_sapic_eid = 0; > > > - if (i < CountCPUs) > > > + if (apic_id_is_present(i)) > > > core->flags = cpu_to_le32(1); > > > else > > > - core->flags = 0; > > > + core->flags = cpu_to_le32(0); > > > core++; > > > } > > > > > > diff --git a/src/mptable.c b/src/mptable.c > > > index 103f462..9406f98 100644 > > > --- a/src/mptable.c > > > +++ b/src/mptable.c > > > @@ -59,7 +59,7 @@ mptable_init(void) > > > cpu->apicid = i; > > > cpu->apicver = apic_version; > > > /* cpu flags: enabled, bootstrap cpu */ > > > - cpu->cpuflag = ((i<CountCPUs) ? 0x01 : 0x00) | ((i==0) ? 0x02 : > > > 0x00); > > > + cpu->cpuflag = (apic_id_is_present(i) ? 0x01 : 0x00) | ((i==0) ? > > > 0x02 : 0x00); > > > cpu->cpusignature = cpuid_signature; > > > cpu->featureflag = cpuid_features; > > > cpu++; > > > diff --git a/src/smp.c b/src/smp.c > > > index 8c077a1..3c36f8c 100644 > > > --- 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 > > > + " movl $1, %eax\n" > > > + " cpuid\n" > > > + " shrl $24, %ebx\n" > > > + " lock btsl %ebx, FoundAPICIDs\n" > > > + > > > // Increment the cpu counter > > > " lock incl CountCPUs\n" > > You can get rid of CountCPUs by calculating FoundAPICIDs bitmap weight. > > I was going to do that after you suggested, but then I saw this: > > while (cmos_smp_count + 1 != readl(&CountCPUs)) > yield(); > > It's possible to replace the atomic read of CountCPUs with the bitmap weight > calculation on the loop, but: is it really worth it? > Why not? This eliminates one more global state.
-- Gleb.