Re: [PATCH v3 2/2] x86/acpi: Don't add CPUs that are not online capable

[Rafael J. Wysocki]

On Wed, Sep 8, 2021 at 11:41 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
>
> A number of systems are showing "hotplug capable" CPUs when they
> are not really hotpluggable.  This is because the MADT has extra
> CPU entries to support different CPUs that may be inserted into
> the socket with different numbers of cores.
>
> Starting with ACPI 6.3 the spec has an Online Capable bit in the
> MADT used to determine whether or not a CPU is hotplug capable
> when the enabled bit is not set.
>
> Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html?#local-apic-flags
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>

I've added the patches in this series to my queue, but given what this
one does, I think that it's a bit risky, because it exposes the kernel
to a new category of possible platform firmware bugs.

For this reason, I'd rather queue it up as 5.16 material (and of
course x86 reviewer comments are welcome).

> ---
>  arch/x86/kernel/acpi/boot.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> Changes from v1 -> v2:
>  * Make the change only apply on ACPI 6.3 or later
> Changes from v2 -> v3:
>  * Make acpi_support_online_capable static and only valid if CONFIG_X86_LOCAL_APIC is defined
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index e55e0c1fad8c..d915f01b582b 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -62,6 +62,7 @@ int acpi_fix_pin2_polarity __initdata;
>
>  #ifdef CONFIG_X86_LOCAL_APIC
>  static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
> +static bool acpi_support_online_capable;
>  #endif
>
>  #ifdef CONFIG_X86_IO_APIC
> @@ -138,6 +139,8 @@ static int __init acpi_parse_madt(struct acpi_table_header *table)
>
>                 pr_debug("Local APIC address 0x%08x\n", madt->address);
>         }
> +       if (madt->header.revision >= 5)
> +               acpi_support_online_capable = true;
>
>         default_acpi_madt_oem_check(madt->header.oem_id,
>                                     madt->header.oem_table_id);
> @@ -239,6 +242,12 @@ acpi_parse_lapic(union acpi_subtable_headers * header, const unsigned long end)
>         if (processor->id == 0xff)
>                 return 0;
>
> +       /* don't register processors that can not be onlined */
> +       if (acpi_support_online_capable &&
> +           !(processor->lapic_flags & ACPI_MADT_ENABLED) &&
> +           !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
> +               return 0;
> +
>         /*
>          * We need to register disabled CPU as well to permit
>          * counting disabled CPUs. This allows us to size
> --
> 2.25.1
>