[PATCH RFC v3 01/21] ACPI: Only enumerate enabled (or functional) devices

[Russell King (Oracle)]

On Mon, Jan 29, 2024 at 04:05:42PM +0100, Rafael J. Wysocki wrote:
> On Mon, Jan 29, 2024 at 3:55 PM Russell King (Oracle)
> <linux at armlinux.org.uk> 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 at armlinux.org.uk> 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.]

>From ACPI v6.5 (bit 3 is our "status.functional":

 _STA may return bit 0 clear (not present) with bit [3] set (device is
 functional). This case is used to indicate a valid device for which no
 device driver should be loaded (for example, a bridge device.) Children
 of this device may be present and valid. OSPM should continue
 enumeration below a device whose _STA returns this bit combination.

So, for this case, acpi_dev_ready_for_enumeration() returning true for
this case is correct, since we're supposed to enumerate it and child
devices.

It's probably also worth pointing out that in the above table, the two
combinations with P=0 E=1 goes against the spec, but are included for
completness.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!