On Sun, Aug 27, 2023 at 05:44:15PM +0200, Thomas Gleixner wrote: > On Wed, Aug 23 2023 at 14:56, Jan Beulich wrote: > > On 23.08.2023 11:21, Andrew Cooper wrote: > >> 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. > > 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. > This is also related to SRAT which defines the proximity of memory to > processors at boot time with a similar set of flags. > > Also 8.4 says: > > Each processor in the system must be declared in the ACPI namespace in > the \_SB scope. .... A Device definition for a processor is declared > using the ACPI0007 hardware identifier (HID). Processor configuration > information is provided exclusively by objects in the processor > device’s object list. > > When the platform uses the APIC interrupt model, UID object values > under a processor device are used to associate processor devices with > entries in the MADT. > > > MADT is the authoritative table for processor enumeration, whether > present or not. This is required because that's the only way to size > resources, which depend on the possible maximum topology. > > 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. 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. 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. Overall I'm inclined to say we should take this change, as it avoids flagging a lot of systems as CPU hotplug capable when they are not. Acked-by: Roger Pau Monné <[email protected]> I think Andrew had some minor comments. I wonder whether the check should be done after the entry has been printed if `opt_cpu_info` is set, but seeing as the ONLINE_CAPABLE is also done before possibly printing the entry I guess it's fine. Thanks, Roger.
