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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.09.2023 20:24, Andrew Cooper wrote:
> 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 wasn't aware that ./MAINTAINERS having

4. There must be no "open" objections.

was possible to overrule by any number of acks.

>>>  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.

And note how I offered a compromise by someone (Simon, you, Roger, or yet
someone else) sending a patch against SUPPORT.md. Yet that hasn't happened.
I therefore continue to be of the opinion that I can rightfully revert the
patch, based on process not having been followed and alternative actions
not having been taken.

Jan