[PATCH v3 1/2] ACPICA: Add support for MADT online enabled bit

[PATCH v3 1/2] ACPICA: Add support for MADT online enabled bit

[Mario Limonciello]

The online enabled bit on newer ACPI implmentations will indicate
whether the CPU is hotpluggable.

Link: http://github.com/acpica/acpica/pull/708/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 include/acpi/actbl2.h | 1 +
 1 file changed, 1 insertion(+)

Changes from v2:
 * Split into a separate patch from the acpica pull request

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 2069ac38a4e2..fae45e383987 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -808,6 +808,7 @@ struct acpi_madt_multiproc_wakeup_mailbox {
 /* MADT Local APIC flags */
 
 #define ACPI_MADT_ENABLED           (1)	/* 00: Processor is usable if set */
+#define ACPI_MADT_ONLINE_CAPABLE    (2)	/* 01: System HW supports enabling processor at runtime */
 
 /* MADT MPS INTI flags (inti_flags) */
 
-- 
2.25.1

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

[Mario Limonciello]

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@amd.com>
---
 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

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@amd.com> 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@amd.com>

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
>

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

[Limonciello, Mario]

On 9/13/2021 12:23, Rafael J. Wysocki wrote:
> On Wed, Sep 8, 2021 at 11:41 PM Mario Limonciello
> <mario.limonciello@amd.com> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fuefi.org%2Fhtmlspecs%2FACPI_Spec_6_4_html%2F05_ACPI_Software_Programming_Model%2FACPI_Software_Programming_Model.html%3F%23local-apic-flags&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C081016240ab94e72822d08d976db3d0a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637671506287828485%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=0vIs%2BDryYFVrtT1DvhU6Ke1BDJe%2BsIhbxPu94NeXRBo%3D&amp;reserved=0
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> 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.
> 

Sounds good, appreciate the diligence.

> 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
>>