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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.09.2023 22:49, 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. 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.
> 
> I noticed that there is no entry for physical CPU hotplug support in
> SUPPORT.md today. Should we assume that it is not supported already as
> it is not listed as supported?

Hmm, I see

## Host hardware support

### Physical CPU Hotplug

    Status, x86: Supported

pretty close to the top of the file.

> Specifically, would it be a good idea to add a sentence at the top of
> the file saying that anything not explicitly listed is not supported?

Iirc that was the plan to do for 4.18, but then we need to be sure that
things don't unintentionally become unsupported. I've no clear idea how
this plan was meant to be carried out, though.

Jan