Messages in this thread | | | From | "Zhang, Rui" <> | Subject | Re: [PATCH] x86/ACPI/boot: Use FADT version to check support for online capable | Date | Thu, 30 Mar 2023 01:10:07 +0000 |
| |
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@oracle.com> > Reported-by: Borislav Petkob <bp@alien8.de> > Fixes: aa06e20f1be6 ("x86/ACPI: Don't add CPUs that are not online > capable") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.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
| |