LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] x86/acpi: Don't add CPUs that are not online capable
@ 2021-08-12  5:16 Mario Limonciello
  2021-08-13  6:26 ` Hanjun Guo
  0 siblings, 1 reply; 2+ messages in thread
From: Mario Limonciello @ 2021-08-12  5:16 UTC (permalink / raw)
  To: Borislav Petkov, Ingo Molnar, Thomas Gleixner
  Cc: Mario Limonciello, Alex Deucher, Huang Rui, Rafael J. Wysocki,
	Len Brown, Pavel Machek,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Robert Moore, Erik Kaneda,
	open list:SUSPEND TO RAM,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	open list:ACPI, open list:ACPI COMPONENT ARCHITECTURE (ACPICA)

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.

The ACPI spec is clear that the Online Capable bit in the
MADT should be 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
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/x86/kernel/acpi/boot.c | 6 ++++++
 include/acpi/actbl2.h       | 1 +
 2 files changed, 7 insertions(+)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e55e0c1fad8c..eeb10b27d6de 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -239,6 +239,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 (!(processor->lapic_flags & ACPI_MADT_ENABLED)) {
+		if (!(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
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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] x86/acpi: Don't add CPUs that are not online capable
  2021-08-12  5:16 [PATCH] x86/acpi: Don't add CPUs that are not online capable Mario Limonciello
@ 2021-08-13  6:26 ` Hanjun Guo
  0 siblings, 0 replies; 2+ messages in thread
From: Hanjun Guo @ 2021-08-13  6:26 UTC (permalink / raw)
  To: Mario Limonciello, Borislav Petkov, Ingo Molnar, Thomas Gleixner
  Cc: Alex Deucher, Huang Rui, Rafael J. Wysocki, Len Brown,
	Pavel Machek, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Robert Moore, Erik Kaneda,
	open list:SUSPEND TO RAM,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	open list:ACPI, open list:ACPI COMPONENT ARCHITECTURE (ACPICA)

On 2021/8/12 13:16, Mario Limonciello 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.
> 
> The ACPI spec is clear that the Online Capable bit in the
> MADT should be used to determine whether or not a CPU is hotplug
> capable when the enabled bit is not set.

This was introduced in ACPI 6.3 spec, which means ACPI 6.2 and
earlier versions don't include the "Online Capable bit".

> 
> Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html?#local-apic-flags
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   arch/x86/kernel/acpi/boot.c | 6 ++++++
>   include/acpi/actbl2.h       | 1 +
>   2 files changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index e55e0c1fad8c..eeb10b27d6de 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -239,6 +239,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 (!(processor->lapic_flags & ACPI_MADT_ENABLED)) {
> +		if (!(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
> +			return 0;
> +	}

For firmware using ACPI 6.2 and early versions, the
ACPI_MADT_ONLINE_CAPABLE bit is reserved as zero, so if
we set CPU as disabled, the code here will always return
0 in those firmwares.

> +
>   	/*
>   	 * We need to register disabled CPU as well to permit
>   	 * counting disabled CPUs. This allows us to size

So we will not register the disabled CPU and will break
CPU hotplug features.

I think we need to consider the compatibility with old versions
of firmware.

Thanks
Hanjun


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-08-13  6:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12  5:16 [PATCH] x86/acpi: Don't add CPUs that are not online capable Mario Limonciello
2021-08-13  6:26 ` Hanjun Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).