Re: [patch 00/53] x86/topology: The final installment

Peter Zijlstra
2023-08-08T22:11:51Z

On Tue, Aug 08, 2023 at 10:41:51PM +0200, Thomas Gleixner wrote:

> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -171,6 +171,8 @@ static bool __init acpi_is_processor_usa
> return false;
> }
>
> +static bool has_lapic_cpus;
> +
> static int __init
> acpi_parse_x2apic(union acpi_subtable_headers *header, const unsigned long end)
> {
> @@ -241,6 +243,14 @@ acpi_parse_lapic(union acpi_subtable_hea
> return 0;
>
> /*
> + * According to https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#processor-local-x2apic-structure
> + * when MADT provides both valid LAPIC and x2APIC entries, the APIC ID
> + * in x2APIC must be equal or greater than 0xff.
> + */
> + if (has_lapic_cpus && processor->id < 0xff)
> + return 0;
> +
> + /*
> * We need to register disabled CPU as well to permit
> * counting disabled CPUs. This allows us to size
> * cpus_possible_map more accurately, to permit

It works better if you move this hunk into acpi_parse_x2apic() instead.
Then I can indeed confirm it works as advertised -- also having one of
them afflicted ivb-ep machines.

Tested-by: Peter Zijlstra (Intel) <peterz@infradead.org>

---
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 088323ed6179..f6cff99d6087 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -171,6 +171,8 @@ static bool __init acpi_is_processor_usable(u32 lapic_flags)
return false;
}

+static bool has_lapic_cpus;
+
static int __init
acpi_parse_x2apic(union acpi_subtable_headers *header, const unsigned long end)
{
@@ -199,6 +201,14 @@ acpi_parse_x2apic(union acpi_subtable_headers *header, const unsigned long end)
if (!acpi_is_processor_usable(processor->lapic_flags))
return 0;

+ /*
+ * According to https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#processor-local-x2apic-structure
+ * when MADT provides both valid LAPIC and x2APIC entries, the APIC ID
+ * in x2APIC must be equal or greater than 0xff.
+ */
+ if (has_lapic_cpus && apic_id < 0xff)
+ return 0;
+
/*
* We need to register disabled CPU as well to permit
* counting disabled CPUs. This allows us to size
@@ -1072,10 +1083,8 @@ static int __init early_acpi_parse_madt_lapic_addr_ovr(void)

static int __init acpi_parse_madt_lapic_entries(void)
{
- int count;
int x2count = 0;
- int ret;
- struct acpi_subtable_proc madt_proc[2];
+ int count;

if (!boot_cpu_has(X86_FEATURE_APIC))
return -ENODEV;
@@ -1084,21 +1093,12 @@ static int __init acpi_parse_madt_lapic_entries(void)
acpi_parse_sapic, MAX_LOCAL_APIC);

if (!count) {
- 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;
- ret = acpi_table_parse_entries_array(ACPI_SIG_MADT,
- sizeof(struct acpi_table_madt),
- madt_proc, ARRAY_SIZE(madt_proc), MAX_LOCAL_APIC);
- if (ret < 0) {
- pr_err("Error parsing LAPIC/X2APIC entries\n");
- return ret;
- }
-
- count = madt_proc[0].count;
- x2count = madt_proc[1].count;
+ count = acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_APIC,
+ acpi_parse_lapic, MAX_LOCAL_APIC);
+ if (count > 0)
+ has_lapic_cpus = true;
+ x2count = acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_X2APIC,
+ acpi_parse_x2apic, MAX_LOCAL_APIC);
}
if (!count && !x2count) {
pr_err("No LAPIC entries present\n");