linux-kernel - Re: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190826102907.GP31967@paasikivi.fi.intel.com>
Date:   Mon, 26 Aug 2019 13:29:07 +0300
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
        "Rafael J. Wysocki" <rjw@...ysocki.net>
Subject: Re: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control
 power at probe time

Hi Greg,

Thanks for the comments.

On Mon, Aug 26, 2019 at 10:46:34AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Aug 26, 2019 at 11:31:08AM +0300, Sakari Ailus wrote:
> > Allow drivers and firmware tell ACPI that there's no need to power on a
> > device for probe. This requires both a hint from the firmware as well as
> > an indication from a driver to leave the device off.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > ---
> >  drivers/acpi/device_pm.c | 15 +++++++++++++--
> >  include/linux/device.h   |  7 +++++++
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> > index f616b16c1f0be..adcdf78ce4de8 100644
> > --- a/drivers/acpi/device_pm.c
> > +++ b/drivers/acpi/device_pm.c
> > @@ -1276,7 +1276,12 @@ static void acpi_dev_pm_detach(struct device *dev, bool power_off)
> >  	if (adev && dev->pm_domain == &acpi_general_pm_domain) {
> >  		dev_pm_domain_set(dev, NULL);
> >  		acpi_remove_pm_notifier(adev);
> > -		if (power_off) {
> > +		if (power_off
> > +#ifdef CONFIG_PM
> > +		    && !(dev->driver->probe_low_power &&
> > +			 device_property_present(dev, "probe-low-power"))
> > +#endif
> 
> As proof of the "only a bus-specific thing", why is probe_low_power even
> needed?  Why not just always trigger off of this crazy device_property?
> That makes the driver changes less.

That's an option, too, but firmware having this property for a device the
driver of which doesn't expect it will fail to power on the device for
probe. This leaves some room for unexpected failures that admittedly are
easy to fix, but could be harder to debug.

> 
> Also, is this #ifdef really needed?

I thought it was but it seems if CONFIG_PM is disabled,
dev_pm_domain_attach() has a nop implementation. So I agree it is not.

-- 
Regards,

Sakari Ailus
sakari.ailus@...ux.intel.com