[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <27c83d42e9cd95a1bfc34fa278315224a63cc062.camel@intel.com>
Date: Thu, 30 Mar 2023 01:10:07 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "Brown, Len" <len.brown@...el.com>, "pavel@....cz" <pavel@....cz>,
"mario.limonciello@....com" <mario.limonciello@....com>,
"rafael@...nel.org" <rafael@...nel.org>
CC: "x86@...nel.org" <x86@...nel.org>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"hpa@...or.com" <hpa@...or.com>,
"mingo@...hat.com" <mingo@...hat.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"bp@...en8.de" <bp@...en8.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"eric.devolder@...cle.com" <eric.devolder@...cle.com>
Subject: Re: [PATCH] x86/ACPI/boot: Use FADT version to check support for
online capable
On Wed, 2023-03-29 at 12:45 -0500, Mario Limonciello wrote:
> ACPI 6.3 introduced the online capable bit, and also introduced MADT
> version 5.
>
> This was used to distinguish whether the offset storing online
> capable
> could be used. However ACPI 6.2b has MADT version "45" which is for
> an errata version of the ACPI 6.2 spec.
I made a double check.
In https://uefi.org/sites/default/files/resources/ACPI_6_2.pdf,
MADT revision is 4.
In
https://uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf,
MADT revision is 45.
In
https://uefi.org/sites/default/files/resources/ACPI_6_2_B_final_Jan30.pdf
MADT revision is 5.
So you probably mean 6.2a has MADT revision "45" here?
> This means that the Linux code
> for detecting availability of MADT will mistakingly flag ACPI 6.2b as
> supporting online capable which is inaccurate as it's an ACPI 6.3
> feature.
>
> Instead use the FADT major and minor revision fields to distingush
> this.
>
> Reported-by: Eric DeVolder <eric.devolder@...cle.com>
> Reported-by: Borislav Petkob <bp@...en8.de>
> Fixes: aa06e20f1be6 ("x86/ACPI: Don't add CPUs that are not online
> capable")
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> arch/x86/kernel/acpi/boot.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/acpi/boot.c
> b/arch/x86/kernel/acpi/boot.c
> index 1c38174b5f01..e92e3292fef7 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -146,7 +146,10 @@ 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)
> +
> + if (acpi_gbl_FADT.header.revision > 6 ||
> + (acpi_gbl_FADT.header.revision == 6 &&
> + acpi_gbl_FADT.minor_revision >= 3))
> acpi_support_online_capable = true;
Better to have a comment here?
For me, it is hard to understand this by reading the code directly.
thanks,
rui
Powered by blists - more mailing lists