Re: Patch "x86/acpi: Ignore invalid x2APIC entries" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2023-12-08 at 05:06 -0500, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
> 
>     x86/acpi: Ignore invalid x2APIC entries
> 
> to the 6.1-stable tree which can be found at:
>    
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      x86-acpi-ignore-invalid-x2apic-entries.patch
> and it can be found in the queue-6.1 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable
> tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
> 
> 
Hi,

This patch causes regressions on two platforms, and a fix has been
posted at
https://lore.kernel.org/all/20231210143925.38722-1-rui.zhang@xxxxxxxxx/

And I agree with Andres that this should not be a -stable material.
https://lore.kernel.org/all/20231206070423.wp7cxxnwfe3lidm3@xxxxxxxxxxxxxxxxxx/

thanks,
rui

> 
> commit 4c7717939f49024466540344aa3463a934461ec2
> Author: Zhang Rui <rui.zhang@xxxxxxxxx>
> Date:   Mon Jul 3 00:28:02 2023 +0800
> 
>     x86/acpi: Ignore invalid x2APIC entries
>     
>     [ Upstream commit ec9aedb2aa1ab7ac420c00b31f5edc5be15ec167 ]
>     
>     Currently, the kernel enumerates the possible CPUs by parsing
> both ACPI
>     MADT Local APIC entries and x2APIC entries. So CPUs with "valid"
> APIC IDs,
>     even if they have duplicated APIC IDs in Local APIC and x2APIC,
> are always
>     enumerated.
>     
>     Below is what ACPI MADT Local APIC and x2APIC describes on an
>     Ivebridge-EP system,
>     
>     [02Ch 0044   1]                Subtable Type : 00 [Processor
> Local APIC]
>     [02Fh 0047   1]                Local Apic ID : 00
>     ...
>     [164h 0356   1]                Subtable Type : 00 [Processor
> Local APIC]
>     [167h 0359   1]                Local Apic ID : 39
>     [16Ch 0364   1]                Subtable Type : 00 [Processor
> Local APIC]
>     [16Fh 0367   1]                Local Apic ID : FF
>     ...
>     [3ECh 1004   1]                Subtable Type : 09 [Processor
> Local x2APIC]
>     [3F0h 1008   4]                Processor x2Apic ID : 00000000
>     ...
>     [B5Ch 2908   1]                Subtable Type : 09 [Processor
> Local x2APIC]
>     [B60h 2912   4]                Processor x2Apic ID : 00000077
>     
>     As a result, kernel shows "smpboot: Allowing 168 CPUs, 120
> hotplug CPUs".
>     And this wastes significant amount of memory for the per-cpu
> data.
>     Plus this also breaks
> https://lore.kernel.org/all/87edm36qqb.ffs@tglx/,
>     because __max_logical_packages is over-estimated by the APIC IDs
> in
>     the x2APIC entries.
>     
>     According to
> https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#processor-local-x2apic-structure
> :
>     
>       "[Compatibility note] On some legacy OSes, Logical processors
> with APIC
>        ID values less than 255 (whether in XAPIC or X2APIC mode) must
> use the
>        Processor Local APIC structure to convey their APIC
> information to OSPM,
>        and those processors must be declared in the DSDT using the
> Processor()
>        keyword. Logical processors with APIC ID values 255 and
> greater must use
>        the Processor Local x2APIC structure and be declared using the
> Device()
>        keyword."
>     
>     Therefore prevent the registration of x2APIC entries with an APIC
> ID less
>     than 255 if the local APIC table enumerates valid APIC IDs.
>     
>     [ tglx: Simplify the logic ]
>     
>     Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
>     Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>     Tested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
>     Link:
> https://lore.kernel.org/r/20230702162802.344176-1-rui.zhang@xxxxxxxxx
>     Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> 
> diff --git a/arch/x86/kernel/acpi/boot.c
> b/arch/x86/kernel/acpi/boot.c
> index 2252340b2133e..14af7fbdc6b5e 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 has_lapic_cpus __initdata;
>  static bool acpi_support_online_capable;
>  #endif
>  
> @@ -235,6 +236,14 @@ acpi_parse_x2apic(union acpi_subtable_headers
> *header, const unsigned long end)
>         if (!acpi_is_processor_usable(processor->lapic_flags))
>                 return 0;
>  
> +       /*
> +        * According to
> https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#processor-local-x2apic-structure
> +        * when MADT provides both valid LAPIC and x2APIC entries,
> the APIC ID
> +        * in x2APIC must be equal or greater than 0xff.
> +        */
> +       if (has_lapic_cpus && apic_id < 0xff)
> +               return 0;
> +
>         /*
>          * We need to register disabled CPU as well to permit
>          * counting disabled CPUs. This allows us to size
> @@ -1114,10 +1123,7 @@ static int __init
> early_acpi_parse_madt_lapic_addr_ovr(void)
>  
>  static int __init acpi_parse_madt_lapic_entries(void)
>  {
> -       int count;
> -       int x2count = 0;
> -       int ret;
> -       struct acpi_subtable_proc madt_proc[2];
> +       int count, x2count = 0;
>  
>         if (!boot_cpu_has(X86_FEATURE_APIC))
>                 return -ENODEV;
> @@ -1126,21 +1132,11 @@ static int __init
> acpi_parse_madt_lapic_entries(void)
>                                       acpi_parse_sapic,
> MAX_LOCAL_APIC);
>  
>         if (!count) {
> -               memset(madt_proc, 0, sizeof(madt_proc));
> -               madt_proc[0].id = ACPI_MADT_TYPE_LOCAL_APIC;
> -               madt_proc[0].handler = acpi_parse_lapic;
> -               madt_proc[1].id = ACPI_MADT_TYPE_LOCAL_X2APIC;
> -               madt_proc[1].handler = acpi_parse_x2apic;
> -               ret = acpi_table_parse_entries_array(ACPI_SIG_MADT,
> -                               sizeof(struct acpi_table_madt),
> -                               madt_proc, ARRAY_SIZE(madt_proc),
> MAX_LOCAL_APIC);
> -               if (ret < 0) {
> -                       pr_err("Error parsing LAPIC/X2APIC
> entries\n");
> -                       return ret;
> -               }
> -
> -               count = madt_proc[0].count;
> -               x2count = madt_proc[1].count;
> +               count =
> acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_APIC,
> +                                       acpi_parse_lapic,
> MAX_LOCAL_APIC);
> +               has_lapic_cpus = count > 0;
> +               x2count =
> acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_X2APIC,
> +                                       acpi_parse_x2apic,
> MAX_LOCAL_APIC);
>         }
>         if (!count && !x2count) {
>                 pr_err("No LAPIC entries present\n");





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux