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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.08.2023 11:21, Andrew Cooper wrote:
> On 07/08/2023 3:18 pm, Jan Beulich wrote:
>> On 07.08.2023 16:04, Andrew Cooper wrote:
>>> On 07/08/2023 2:17 pm, Jan Beulich wrote:
>>>> On 07.08.2023 14:55, Simon Gaiser wrote:
>>>>> Jan Beulich:
>>>>>> 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.
>>>>>> I'm afraid it doesn't become clear to me what problem you're trying to
>>>>>> solve.
>>>>> I want Xen to not think there are possible CPUs that actually never can
>>>>> be there.
>>>> Did you try using "maxcpus=" on the command line? If that doesn't work
>>>> well enough (perhaps because of causing undesirable log messages), maybe
>>>> we need some way to say "no CPU hotplug" on the command line.
>>> The ACPI tables are broken, and Xen's parsing of them is wrong.
>>>
>>> And irrespective - it's unreasonable to have users work around bugs in
>>> Xen by adding more command line.
>> Well, considering how rare CPU hotplug appears to be, such a new option
>> could default to "no hotplug".
> 
> ... or Xen could not be buggy and think there's a chance of hotplug on
> the 99% of servers where there isn't.

Why do you say "buggy" when there's no clear cut indication of whether
hotplug is possible, even not all so old ACPI versions?

>>>>> Without ignoring those dummy entries Xen thinks my NUC has 2 sockets and
>>>>> that there are 8 logical CPUs that are currently disabled but could be
>>>>> hotplugged.
>>>> Yet it's exactly this which ACPI is telling us here (with some vagueness,
>>>> which isn't easy to get around; see below).
>>>>
>>>>> I'm moderately sure that soldering in another CPU is not supported, even
>>>>> less so at runtime ;]
>>>> On your system. What about others, which are hotplug-capable?
>>> It is required that all APIC ID are stated *ahead of time*.
>> Would you mind pointing me at where this is stated?
> 
> In the spec, exactly where you'd expect to find them...
> 
> "OSPM does not expect the information provided in this table to be
> updated if the processor information changes during the lifespan of an
> OS boot."

I don't think this tells us anything about the ID not possibly changing.
It merely tells us that OSPM is not expected to parse this table again
(IOW firmware updating just this table isn't going to be enough). IDs
possibly changing is expressed by (a) the "if the processor information
changes", and (b) the next sentence, forbidding them to change while the
system is asleep: "While in the sleeping state, logical processors must
not be added or removed, nor can their ... ID or ... Flags change."

> Which is wordsmithing for "Some firmware was found to be modifying them
> and this was deemed to be illegal under the spec".

That's your reading of it; I certainly don't infer such from that
sentence.

Jan