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

[Thomas Gleixner <tglx@xxxxxxxxxxxxx>]

On Tue, Aug 29 2023 at 16:25, Roger Pau Monné wrote:
> On Sun, Aug 27, 2023 at 05:44:15PM +0200, Thomas Gleixner wrote:
>> The APIC/X2APIC description of MADT specifies flags:
>> 
>> Enabled              If this bit is set the processor is ready for use. If
>>              this bit is clear and the Online Capable bit is set,
>>              system hardware supports enabling this processor during
>>              OS runtime. If this bit is clear and the Online Capable
>>              bit is also clear, this processor is unusable, and OSPM
>>              shall ignore the contents of the Processor Local APIC
>>              Structure.
>> 
>> Online Capable       The information conveyed by this bit depends on the
>>              value of the Enabled bit. If the Enabled bit is set,
>>              this bit is reserved and must be zero. Otherwise, if
>>              this this bit is set, system hardware supports enabling
>>              this processor during OS runtime.
>
> Sadly this flag is only present starting with MADT v5.

Correct. The difference between pre v5 MADT and v5+ is that the latter
allows the OS to make more informed decisions, but the lack of this flag
does not make the claim that randomly assigning APIC IDs after the
initial parsing is a valid approach any more correct. Why?

Simply because the other relationships vs. processor UIDs and SRAT/SLIT
are not magically going away due to the lack of that flag.

>> Otherwise you'd end up with a CPU hotplugged which is outside of the
>> resource space allocated during init.
>
> It's my understating that ACPI UIDs 0xff and 0xfffffff for local ACPI
> and x2APIC structures respectively are invalid, as that's the
> broadcast value used by the local (x2)APIC NMI Structures.

Correct. These IDs are invalid independent of any flag value. 

> I think Jan's point (if I understood correctly) is that Processor or
> Device objects can have a _MAT method that returns updated MADT
> entries, and some ACPI implementations might modify the original
> entries on the MADT and return them from that method when CPU
> hotplug takes place.  The spec notes that "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." so that the
> MADT doesn't need to be updated when CPU hotplug happens, but I don't
> see that sentence as preventing the MADT to be updated if CPU hotplug
> takes place, it's just not required.

Right. But if you read carefully what I wrote then you will figure out
that any randomly made up APIC ID post MADT enumeration cannot work.

> I don't see anywhere in the spec that states that APIC IDs 0xff and
> 0xffffffff are invalid and entries using those IDs should be ignored,
> but I do think that any system that supports CPU hotplug better has
> those IDs defined since boot.  Also it seems vendors have started
> relying on using 0xff and 0xffffffff APIC IDs to signal non-present
> CPUs, and Linux has been ignoring such entries for quite some time
> already  without reported issues.

There is no requirement for the ACPI spec to state this simply because
these APIC IDs are invalid to address a processor at the architectural
level. ACPI does not care about architectural restrictions unless really
required, e.g. like the LAPIC vs. X2APIC exclusiveness.

Thanks,

        tglx