Re: [PATCH] x86/acpi/boot: Do not register processors that cannot be onlined for x2apic

[Rafael J. Wysocki]

On Fri, Dec 30, 2022 at 2:23 PM Zhang, Rui <rui.zhang@xxxxxxxxx> wrote:
>
> Hi, Kishon,
>
> On Wed, 2022-12-28 at 11:45 +0000, Kishon Vijay Abraham I wrote:
> > Section 5.2.12.12 Processor Local x2APIC Structure in the ACPI v6.3
> > spec mandates that both "enabled" and "online capable" Local APIC
> > Flags
> > should be used to determine if the processor is usable or not.
>
> ACPI spec 6.4 is released, so better to refer to the latest ACPI spec,
> say,
> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#processor-local-x2apic-structure
> or
> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#local-apic-flags

ACPI 6.5 is out even.

> > However, Linux doesn't use the "online capable" flag for x2APIC to
> > determine if the processor is usable. As a result, cpu_possible_mask
> > has incorrect value and results in more memory getting allocated for
> > per_cpu variables than it is going to be used.
>
> Thanks for catching this. I had the same question when I was reading
> this piece of code recently.
>
> > Make sure Linux parses both "enabled" and "online capable" flags for
> > x2APIC to correctly determine if the processor is usable.
>
> A dumb question, the Local SAPIC structure also uses the Local APIC
> flags, and should we add the same check in acpi_parse_sapic()?

I'm not sure if this matters in practice, because SAPIC is only used
on IA64 anyway.

Tony, what do you think?

> > Fixes: 7237d3de78ff ("x86, ACPI: add support for x2apic ACPI
> > extensions")
>
> I'm not sure if this "Fixes" tag is accurate or not.
>
> Checking for the Local APIC flags was just added last year, by commit
> aa06e20f1be6 ("x86/ACPI: Don't add CPUs that are not online capable"),
> and the variable 'acpi_support_online_capable' used in this patch is
> also introduced by that commit. So, to me, this patch fixes a gap in aa
> 06e20f1be6, rather than the original x2apic support commit.

Agreed.