Re: [XEN PATCH] x86/ACPI: Ignore entries with invalid APIC IDs when parsing MADT

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/09/2023 9:49 pm, Stefano Stabellini wrote:
> On Fri, 1 Sep 2023, Jan Beulich wrote:
>> On 07.08.2023 11:38, Simon Gaiser wrote:
>>> It seems some firmwares put dummy entries in the ACPI MADT table for non
>>> existing processors. On my NUC11TNHi5 those have the invalid APIC ID
>>> 0xff. Linux already has code to handle those cases both in
>>> acpi_parse_lapic [1] as well as in acpi_parse_x2apic [2]. So add the
>>> same check to Xen.
>>>
>>> Note that on some older (2nd gen Core i) laptop of mine I also saw dummy
>>> entries with a valid APIC ID. Linux would still ignore those because
>>> they have !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE. But in Xen
>>> this check is only active for madt_revision >= 5. But since this version
>>> check seems to be intentionally I leave that alone.
>>>
>>> Link: 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3bf1dbe64b62a2058dd1944c00990df203e8e7a
>>>  # [1]
>>> Link: 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=10daf10ab154e31237a8c07242be3063fb6a9bf4
>>>  # [2]
>>> Signed-off-by: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
>> This patch was committed with unaddressed review comments.

Count the number of x86 maintainer tags on the patch, and see that they
were given after discussion, not before.

You're outvoted 2:1 by Xen x86 maintainers alone, and more than 2:1 if
you include the x86 maintainers from other projects who participated in
the discussion.


I'm not willing to let Xen keep on malfunctioning on millions of systems
just because you think an unfinished spec is more correct that practical
reality which invalidates it.

>>  The normal action
>> in such a case would be to revert, expecting a v2 to arrive. One alternative
>> here would be a timely incremental patch submission. Another alternative,
>> considering in particular Thomas's most recent reply, would be to properly
>> downgrade CPU hotplug support in SUPPORT.md (with a corresponding entry in
>> CHANGELOG.md).
> I am in favor of downgrading physical CPU hotplug support in
> SUPPORT.md.

SUPPORT.md does look bogus and wants correcting, but it is laughable to
claim that this ever worked, let alone that it's supported.

Intel got half way through specifying ACPI CPU Hotplug, and abandoned it
as a technology because they couldn't get it to work.  Hence the feature
has never been tested.

Furthermore, cursory testing that Thomas did for the Linux topology work
demonstrates that it is broken anyway for reasons unrelated to ACPI parsing.

Even furthermore, it's an area of the Xen / dom0 boundary which is
fundamentally broken for non-PV cases, and undocumented for the PV case,
hence why it's broken in Linux.


Physical CPU Hotplug does not pass the bar for being anything more than
experimental.  It's absolutely not tech-preview level because the only
demo it has had in an environment (admittedly virtual) which does
implement the spec in a usable way demonstrates that it doesn't function.

The fact no-one has noticed until now shows that the feature isn't used,
which comes back around full circle to the fact that Intel never made it
work and never shipped it.

~Andrew