On Tue, 9 Jan 2024 16:13:21 +0000 "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote: > On Tue, Jan 09, 2024 at 05:05:15PM +0100, Rafael J. Wysocki wrote: > > On Tue, Jan 9, 2024 at 4:49 PM Russell King (Oracle) > > <linux@xxxxxxxxxxxxxxx> wrote: > > > > > > On Mon, Dec 18, 2023 at 09:17:34PM +0100, Rafael J. Wysocki wrote: > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@xxxxxxxxxxxxxxx> wrote: > > > > > > > > > > From: James Morse <james.morse@xxxxxxx> > > > > > > > > > > ACPI has two ways of describing processors in the DSDT. From ACPI v6.5, > > > > > 5.2.12: > > > > > > > > > > "Starting with ACPI Specification 6.3, the use of the Processor() object > > > > > was deprecated. Only legacy systems should continue with this usage. On > > > > > the Itanium architecture only, a _UID is provided for the Processor() > > > > > that is a string object. This usage of _UID is also deprecated since it > > > > > can preclude an OSPM from being able to match a processor to a > > > > > non-enumerable device, such as those defined in the MADT. From ACPI > > > > > Specification 6.3 onward, all processor objects for all architectures > > > > > except Itanium must now use Device() objects with an _HID of ACPI0007, > > > > > and use only integer _UID values." > > > > > > > > > > Also see https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#declaring-processors > > > > > > > > > > Duplicate descriptions are not allowed, the ACPI processor driver already > > > > > parses the UID from both devices and containers. acpi_processor_get_info() > > > > > returns an error if the UID exists twice in the DSDT. > > > > > > > > I'm not really sure how the above is related to the actual patch. > > > > > > > > > The missing probe for CPUs described as packages > > > > > > > > It is unclear what exactly is meant by "CPUs described as packages". > > > > > > > > From the patch, it looks like those would be Processor() objects > > > > defined under a processor container device. > > > > > > > > > creates a problem for > > > > > moving the cpu_register() calls into the acpi_processor driver, as CPUs > > > > > described like this don't get registered, leading to errors from other > > > > > subsystems when they try to add new sysfs entries to the CPU node. > > > > > (e.g. topology_sysfs_init()'s use of topology_add_dev() via cpuhp) > > > > > > > > > > To fix this, parse the processor container and call acpi_processor_add() > > > > > for each processor that is discovered like this. > > > > > > > > Discovered like what? > > > > > > > > > The processor container > > > > > handler is added with acpi_scan_add_handler(), so no detach call will > > > > > arrive. > > > > > > > > The above requires clarification too. > > > > > > The above comments... yea. As I didn't write the commit description, but > > > James did, and James has basically vanished, I don't think these can be > > > answered, short of rewriting the entire commit message, with me spending > > > a lot of time with the ACPI specification trying to get the terminology > > > right - because at lot of the above on the face of it seems to be things > > > to do with wrong terminology being used. > > > > > > I wasn't expecting this level of issues with this patch set, and I now > > > feel completely out of my depth with this series. I'm wondering whether > > > I should even continue with it, since I don't have the ACPI knowledge > > > to address a lot of these comments. > > > > Well, sorry about this. > > > > I met James at the LPC last year, so he seems to be still around, in > > some way at least.. > > On the previous posting, I wanted James to comment on some of the > feedback from Jonathan, and despite explicitly asking, there has been > nothing but radio silence ever since James' last post of this series. > > So, I now deem this work to be completely dead in the water, and not > going to happen - not unless others can input on your comments. > I'll take another pass at this and see which comments I can resolve. Will need a few additional test setups so may take a few days. So far I've established that QEMU uses Processor for x86 and ACPI0007 for arm64. Goody, at least that simplifies testing the various options. Jonathan
