x86, acpi: Handle xapic/x2apic entries in MADT at same time
PATCH 1/1 x86, acpi: Handle xapic/x2apic entries in MADT at same time
One system have mixing xapic and x2apic entries in MADT and SRAT. BIOS guys insist that ACPI 4.0 SPEC said so, if apic id < 255, even the cpus are with x2apic mode pre-enabled, still need to use xapic entries instead of x2apic entries.
on 8 socket system with x2apic pre-enabled, will get out of order sequence:
CPU0: socket0, core0, thread0.
CPU1 - CPU 40: socket 4 - socket 7, thread 0
CPU41 - CPU 80: socket 4 - socket 7, thread 1
CPU81 - CPU 119: socket 0 - socket 3, thread 0
CPU120 - CPU 159: socket 0 - socket 3, thread 1
so max_cpus=80 will not get all thread0 now.
Need to handle every entry in MADT at same time with xapic and x2apic. so we can honor sequence in MADT.
We can use max_cpus= command line to use thread0 in every core, because recent MADT always have all thread0 at first. Also it could make the cpu to node mapping more sane.
after patch will get
CPU0 - CPU 79: socket 0 - socket 7, thread 0
CPU80 - CPU 159: socket 0 - socket 7, thread 1
-v2: update some comments, and change to pass array pointer.
--- arch/x86/kernel/acpi/boot.c | 30 ++++++++++++++++------- drivers/acpi/numa.c | 16 +++++++++--- drivers/acpi/tables.c | 57 +++++++++++++++++++++++++++++++++----------- include/linux/acpi.h | 9 ++++++ 4 files changed, 86 insertions(+), 26 deletions(-)
arch/x86/kernel/acpi/boot.c +21 -9
@@ -883,6 +883,7 @@ static int __init acpi_parse_madt_lapic_ {int count;int x2count =0 ; +struct acpi_subtable_proc madt_proc[2 ];if (!cpu_has_apic)return -ENODEV ;
@@ -907,10 +908,16 @@ static int __init acpi_parse_madt_lapic_ acpi_parse_sapic, MAX_LOCAL_APIC);if (!count) { - x2count =acpi_table_parse_madt (ACPI_MADT_TYPE_LOCAL_X2APIC , - acpi_parse_x2apic,MAX_LOCAL_APIC ); - count =acpi_table_parse_madt (ACPI_MADT_TYPE_LOCAL_APIC , - acpi_parse_lapic,MAX_LOCAL_APIC ); +memset (madt_proc,0 ,sizeof (madt_proc)); + madt_proc[0 ].id =ACPI_MADT_TYPE_LOCAL_APIC ; + madt_proc[0 ].handler = acpi_parse_lapic; + madt_proc[1 ].id =ACPI_MADT_TYPE_LOCAL_X2APIC ; + madt_proc[1 ].handler = acpi_parse_x2apic; +acpi_table_parse_entries_array (ACPI_SIG_MADT , +sizeof (struct acpi_table_madt ), + madt_proc,2 ,MAX_LOCAL_APIC ); + count = madt_proc[0 ].count; + x2count = madt_proc[1 ].count; }if (!count && !x2count) { printk(KERN_ERR PREFIX "No LAPIC entries present\n");
@@ -922,11 +929,16 @@ static int __init acpi_parse_madt_lapic_return count; } - x2count = -acpi_table_parse_madt (ACPI_MADT_TYPE_LOCAL_X2APIC_NMI , - acpi_parse_x2apic_nmi,0 ); - count = -acpi_table_parse_madt (ACPI_MADT_TYPE_LOCAL_APIC_NMI , acpi_parse_lapic_nmi,0 ); +memset (madt_proc,0 ,sizeof (madt_proc)); + madt_proc[0 ].id =ACPI_MADT_TYPE_LOCAL_APIC_NMI ; + madt_proc[0 ].handler = acpi_parse_lapic_nmi; + madt_proc[1 ].id =ACPI_MADT_TYPE_LOCAL_X2APIC_NMI ; + madt_proc[1 ].handler = acpi_parse_x2apic_nmi; +acpi_table_parse_entries_array (ACPI_SIG_MADT , +sizeof (struct acpi_table_madt ), + madt_proc,2 ,0 ); + count = madt_proc[0 ].count; + x2count = madt_proc[1 ].count;if (count <0 || x2count <0 ) { printk(KERN_ERR PREFIX "Error parsing LAPIC NMI entry\n");/* TBD: Cleanup to allow fallback to MPS */
drivers/acpi/numa.c +12 -4
@@ -284,10 +284,18 @@ int __init acpi_numa_init(void)/* SRAT: Static Resource Affinity Table */ if (!acpi_table_parse (ACPI_SIG_SRAT , acpi_parse_srat)) { -acpi_table_parse_srat (ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY , - acpi_parse_x2apic_affinity,0 ); -acpi_table_parse_srat (ACPI_SRAT_TYPE_CPU_AFFINITY , - acpi_parse_processor_affinity,0 ); +struct acpi_subtable_proc srat_proc[2 ]; + +memset (srat_proc,0 ,sizeof (srat_proc)); + srat_proc[0 ].id =ACPI_SRAT_TYPE_CPU_AFFINITY ; + srat_proc[0 ].handler = acpi_parse_processor_affinity; + srat_proc[1 ].id =ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY ; + srat_proc[1 ].handler = acpi_parse_x2apic_affinity; + +acpi_table_parse_entries_array (ACPI_SIG_SRAT , +sizeof (struct acpi_table_srat ), + srat_proc,2 ,0 ); + ret =acpi_table_parse_srat (ACPI_SRAT_TYPE_MEMORY_AFFINITY , acpi_parse_memory_affinity,NR_NODE_MEMBLKS );
drivers/acpi/tables.c +44 -13
97 lines not on this page · raw message
include/linux/acpi.h +9 -0
@@ -76,6 +76,12 @@ typedef int (*acpi_table_handler) (structypedef int (*acpi_table_entry_handler ) (struct acpi_subtable_header *header,const unsigned long end); +struct acpi_subtable_proc { +int id; +acpi_table_entry_handler handler; +int count; +}; +char *__acpi_map_table (unsigned long phys_addr,unsigned long size);void __acpi_unmap_table (char *map,unsigned long size);int early_acpi_boot_init (void );
@@ -86,6 +92,9 @@ int acpi_numa_init (void);int acpi_table_init (void );int acpi_table_parse (char *id,acpi_table_handler handler); +int acpi_table_parse_entries_array (char *id,unsigned long table_size, +struct acpi_subtable_proc *proc,int proc_num, +unsigned int max_entries);int __init acpi_table_parse_entries (char *id,unsigned long table_size,int entry_id,acpi_table_entry_handler handler,unsigned int max_entries);int acpi_table_parse_madt (enum acpi_madt_type id,acpi_table_entry_handler handler,unsigned int max_entries);
Quoted 4 lines · Yinghai Lu
One system have mixing xapic and x2apic entries in MADT and SRAT. BIOS guys insist that ACPI 4.0 SPEC said so, if apic id < 255, even the cpus are with x2apic mode pre-enabled, still need to use xapic entries instead of x2apic entries.
true. thats what the spec says.
Quoted 11 lines
on 8 socket system with x2apic pre-enabled, will get out of order sequence:
CPU0: socket0, core0, thread0.
CPU1 - CPU 40: socket 4 - socket 7, thread 0
CPU41 - CPU 80: socket 4 - socket 7, thread 1
CPU81 - CPU 119: socket 0 - socket 3, thread 0
CPU120 - CPU 159: socket 0 - socket 3, thread 1so max_cpus=80 will not get all thread0 now.
Need to handle every entry in MADT at same time with xapic and x2apic. so we can honor sequence in MADT.
hmm few things.
So is your bios listing like this:
xapic entries for all thread-0
x2apic entries for all thread-0
xapic entries for all thread-1
x2apic entries for all thread-1
What happens if some other bios lists like:
all xapic entries
followed by all x2apic entries
or
all x2apic entries
followed by all x2apic entries
I am not sure if there is a strict recommendation by Intel bios writers guide to list all thread-0 first followed by thread-1. Even if there is a strict recommendation like that, I am sure there will be some bios's that don't follow the rules.
Anyways two things.
a) I don't want to complicate the code too much and still doesn't get the desired behavior when some bios has a different silly ordering.
b) Also perhaps we need to make it more explicit. Like instead of relying on the "maxcpus=n/2" to boot all thread-0's, we should say it more explicitly like perhaps nosmt or noht (we used to have this kind of option before). Or is it even worth to bother about this, given that we can boot all the cpu's and offline the SMT siblings.
Quoted 10 lines
We can use max_cpus= command line to use thread0 in every core, because recent MADT always have all thread0 at first. Also it could make the cpu to node mapping more sane.
after patch will get
CPU0 - CPU 79: socket 0 - socket 7, thread 0
CPU80 - CPU 159: socket 0 - socket 7, thread 1-v2: update some comments, and change to pass array pointer.
thanks,
suresh
Quoted 7 lines · Suresh Siddha
Quoted 4 lines · Yinghai Lu
One system have mixing xapic and x2apic entries in MADT and SRAT. BIOS guys insist that ACPI 4.0 SPEC said so, if apic id < 255, even the cpus are with x2apic mode pre-enabled, still need to use xapic entries instead of x2apic entries.
true. thats what the spec says.
I was thinking if cpus are with x2apic pre-enabled, we should have x2apic entries for them.
Quoted 13 lines
Quoted 8 lines
on 8 socket system with x2apic pre-enabled, will get out of order sequence:
CPU0: socket0, core0, thread0.
CPU1 - CPU 40: socket 4 - socket 7, thread 0
CPU41 - CPU 80: socket 4 - socket 7, thread 1
CPU81 - CPU 119: socket 0 - socket 3, thread 0
CPU120 - CPU 159: socket 0 - socket 3, thread 1Quoted 4 lines
so max_cpus=80 will not get all thread0 now.
Need to handle every entry in MADT at same time with xapic and x2apic. so we can honor sequence in MADT.
Quoted 8 lines
hmm few things.
So is your bios listing like this:
xapic entries for all thread-0
x2apic entries for all thread-0
xapic entries for all thread-1
x2apic entries for all thread-1
Yes. that is required by BIOS write guide, it says all thread 0 should come first.
Quoted 5 lines
What happens if some other bios lists like:
all xapic entries
followed by all x2apic entries
assume should get
CPU0: socket0, core0, thread0.
CPU1 - CPU 80: socket 4 - socket 7, thread 0 and thread 1 CPU81 - CPU 159: socket 0 - socket 3, thread 0 and thread 1
Quoted 5 lines
or
all x2apic entries
followed by all x2apic entries
all x2apic ?
Quoted 5 lines
I am not sure if there is a strict recommendation by Intel bios writers guide to list all thread-0 first followed by thread-1. Even if there is a strict recommendation like that, I am sure there will be some bios's that don't follow the rules.
our bios follow the BWG guide.
Quoted 5 lines
Anyways two things.
a) I don't want to complicate the code too much and still doesn't get the desired behavior when some bios has a different silly ordering.
we have right ordering according to ACPI 4.0 spec and BWG
also i think that code is not that complicated.
Quoted 6 lines
b) Also perhaps we need to make it more explicit. Like instead of relying on the "maxcpus=n/2" to boot all thread-0's, we should say it more explicitly like perhaps nosmt or noht (we used to have this kind of option before). Or is it even worth to bother about this, given that we can boot all the cpu's and offline the SMT siblings.
interesting, if we have nosmt or noht, we still need to boot that cpu to use cpuid to
find out thread idx?
Thanks
Yinghai Lu
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 28 lines · Yinghai Lu
One system have mixing xapic and x2apic entries in MADT and SRAT. BIOS guys insist that ACPI 4.0 SPEC said so, if apic id < 255, even the cpus are with x2apic mode pre-enabled, still need to use xapic entries instead of x2apic entries.
on 8 socket system with x2apic pre-enabled, will get out of order sequence:
CPU0: socket0, core0, thread0.
CPU1 - CPU 40: socket 4 - socket 7, thread 0
CPU41 - CPU 80: socket 4 - socket 7, thread 1
CPU81 - CPU 119: socket 0 - socket 3, thread 0
CPU120 - CPU 159: socket 0 - socket 3, thread 1so max_cpus=80 will not get all thread0 now.
Need to handle every entry in MADT at same time with xapic and x2apic. so we can honor sequence in MADT.
We can use max_cpus= command line to use thread0 in every core, because recent MADT always have all thread0 at first. Also it could make the cpu to node mapping more sane.
after patch will get
CPU0 - CPU 79: socket 0 - socket 7, thread 0
CPU80 - CPU 159: socket 0 - socket 7, thread 1-v2: update some comments, and change to pass array pointer.
Quoted 8 lines
Signed-off-by Yinghai Lu <[email protected]>--- arch/x86/kernel/acpi/boot.c | 30 ++++++++++++++++------- drivers/acpi/numa.c | 16 +++++++++--- drivers/acpi/tables.c | 57 +++++++++++++++++++++++++++++++++----------- include/linux/acpi.h | 9 ++++++ 4 files changed, 86 insertions(+), 26 deletions(-)
Seems useful - we can queue it up and can get it tested in the x86 tree if Len acks the principle and the ACPI approach.
Thanks,
Ingo