On Mon, Jan 29, 2024 at 3:55 PM Russell King (Oracle) <linux@xxxxxxxxxxxxxxx> wrote: > > Hi Jonathan, > > On Fri, Jan 12, 2024 at 11:52:05AM +0000, Jonathan Cameron wrote: > > On Thu, 11 Jan 2024 10:26:15 +0000 > > "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote: > > > @@ -2381,16 +2388,38 @@ EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies); > > > * acpi_dev_ready_for_enumeration - Check if the ACPI device is ready for enumeration > > > * @device: Pointer to the &struct acpi_device to check > > > * > > > - * Check if the device is present and has no unmet dependencies. > > > + * Check if the device is functional or enabled and has no unmet dependencies. > > > * > > > - * Return true if the device is ready for enumeratino. Otherwise, return false. > > > + * Return true if the device is ready for enumeration. Otherwise, return false. > > > */ > > > bool acpi_dev_ready_for_enumeration(const struct acpi_device *device) > > > { > > > if (device->flags.honor_deps && device->dep_unmet) > > > return false; > > > > > > - return acpi_device_is_present(device); > > > + /* > > > + * ACPI 6.5's 6.3.7 "_STA (Device Status)" allows firmware to return > > > + * (!present && functional) for certain types of devices that should be > > > + * enumerated. Note that the enabled bit should not be set unless the > > > + * present bit is set. > > > + * > > > + * However, limit this only to processor devices to reduce possible > > > + * regressions with firmware. > > > + */ > > > + if (device->status.functional) > > > + return true; > > I have a report from within Oracle that this causes testing failures > with QEMU using -smp cpus=2,maxcpus=4. I think it needs to be: > > if (!device->status.present) > return device->status.functional; > > if (device->status.enabled) > return true; > > return !acpi_device_is_processor(device); The above is fine by me. > So we can better understand the history here, let's list it as a > truth table. P=present, F=functional, E=enabled, Orig=how the code > is in mainline, James=James' original proposal, Rafael=the proposed > replacement but seems to be buggy, Rmk=the fixed version that passes > tests: > > P F E Orig James Rafael Rmk > 0 0 0 0 0 0 0 > 0 0 1 0 0 0 0 > 0 1 0 1 1 1 1 > 0 1 1 1 0 1 1 > 1 0 0 1 0 !processor !processor > 1 0 1 1 1 1 1 > 1 1 0 1 0 1 !processor > 1 1 1 1 1 1 1 > > Any objections to this? So AFAIAC it can return false if not enabled, but present and functional. [Side note: I'm wondering what "functional" means then, but whatever.]