All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hanjun Guo <guohanjun@huawei.com>
To: Mario Limonciello <mario.limonciello@amd.com>,
	Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Alex Deucher <alexander.deucher@amd.com>,
	Huang Rui <ray.huang@amd.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Robert Moore <robert.moore@intel.com>,
	Erik Kaneda <erik.kaneda@intel.com>,
	"open list:SUSPEND TO RAM" <linux-pm@vger.kernel.org>,
	"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
	<linux-kernel@vger.kernel.org>,
	"open list:ACPI" <linux-acpi@vger.kernel.org>,
	"open list:ACPI COMPONENT ARCHITECTURE (ACPICA)"
	<devel@acpica.org>
Subject: Re: [PATCH] x86/acpi: Don't add CPUs that are not online capable
Date: Fri, 13 Aug 2021 14:26:56 +0800	[thread overview]
Message-ID: <a671969e-526f-cdc0-6c77-0eb2d940ec5c@huawei.com> (raw)
In-Reply-To: <20210812051657.28605-1-mario.limonciello@amd.com>

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


WARNING: multiple messages have this Message-ID (diff)
From: Hanjun Guo <guohanjun at huawei.com>
To: devel@acpica.org
Subject: [Devel] Re: [PATCH] x86/acpi: Don't add CPUs that are not online capable
Date: Fri, 13 Aug 2021 06:27:06 +0000	[thread overview]
Message-ID: <a671969e-526f-cdc0-6c77-0eb2d940ec5c@huawei.com> (raw)
In-Reply-To: 20210812051657.28605-1-mario.limonciello@amd.com

[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]

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(a)amd.com>
> Reviewed-by: Huang Rui <ray.huang(a)amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello(a)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

  reply	other threads:[~2021-08-13  6:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]
2021-08-13  6:27   ` [Devel] " Hanjun Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a671969e-526f-cdc0-6c77-0eb2d940ec5c@huawei.com \
    --to=guohanjun@huawei.com \
    --cc=alexander.deucher@amd.com \
    --cc=bp@alien8.de \
    --cc=devel@acpica.org \
    --cc=erik.kaneda@intel.com \
    --cc=hpa@zytor.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=mingo@redhat.com \
    --cc=pavel@ucw.cz \
    --cc=ray.huang@amd.com \
    --cc=rjw@rjwysocki.net \
    --cc=robert.moore@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.