linux-kernel - Re: [PATCH] rpm: pm: enable PM_RPM

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: <2023103133-kelp-copartner-8e9c@gregkh>
Date:   Tue, 31 Oct 2023 10:48:13 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Guan-Yu Lin <guanyulin@...gle.com>
Cc:     rafael@...nel.org, len.brown@...el.com, pavel@....cz,
        stern@...land.harvard.edu, heikki.krogerus@...ux.intel.com,
        mkl@...gutronix.de, hadess@...ess.net, mailhol.vincent@...adoo.fr,
        ivan.orlov0322@...il.com, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        pumahsu@...gle.com, raychi@...gle.com, albertccwang@...gle.com
Subject: Re: [PATCH] rpm: pm: enable PM_RPM_EXCEPTION config flag

On Tue, Oct 31, 2023 at 05:38:55PM +0800, Guan-Yu Lin wrote:
> Introducing PM_RPM_EXCEPTION config flag, which may alter the priority
> between system power management and runtime power management. In
> suspend-to-idle flow, PM core will suspend all devices to avoid device
> interact with the system. However, chances are devices might be used by
> other systems rather than a single system. In this case, PM core shouldn't
> suspend the devices. One may use PM_RPM_EXCEPTION config flag to mark
> such exception, and determine the power state of a device with runtime
> power management rather than system power management.
> 
> Signed-off-by: Guan-Yu Lin <guanyulin@...gle.com>
> ---
>  drivers/usb/core/generic.c |  6 ++++++
>  drivers/usb/core/usb.h     | 16 ++++++++++++++++
>  kernel/power/Kconfig       |  8 ++++++++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
> index 740342a2812a..bb0dfcfc9764 100644
> --- a/drivers/usb/core/generic.c
> +++ b/drivers/usb/core/generic.c
> @@ -266,6 +266,9 @@ int usb_generic_driver_suspend(struct usb_device *udev, pm_message_t msg)
>  {
>  	int rc;
>  
> +	if (usb_runtime_pm_exception(udev))
> +		return 0;
> +
>  	/* Normal USB devices suspend through their upstream port.
>  	 * Root hubs don't have upstream ports to suspend,
>  	 * so we have to shut down their downstream HC-to-USB
> @@ -294,6 +297,9 @@ int usb_generic_driver_resume(struct usb_device *udev, pm_message_t msg)
>  {
>  	int rc;
>  
> +	if (usb_runtime_pm_exception(udev))
> +		return 0;
> +
>  	/* Normal USB devices resume/reset through their upstream port.
>  	 * Root hubs don't have upstream ports to resume or reset,
>  	 * so we have to start up their downstream HC-to-USB
> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> index 60363153fc3f..14a054f814a2 100644
> --- a/drivers/usb/core/usb.h
> +++ b/drivers/usb/core/usb.h
> @@ -90,6 +90,22 @@ extern void usb_major_cleanup(void);
>  extern int usb_device_supports_lpm(struct usb_device *udev);
>  extern int usb_port_disable(struct usb_device *udev);
>  
> +#ifdef	CONFIG_PM_RPM_EXCEPTION
> +
> +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> +{
> +	return atomic_read(&udev->dev.power.usage_count);
> +}
> +
> +#else
> +
> +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> +{
> +	return 0;
> +}
> +
> +#endif
> +
>  #ifdef	CONFIG_PM
>  
>  extern int usb_suspend(struct device *dev, pm_message_t msg);
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 4b31629c5be4..beba7a0f3947 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -193,6 +193,14 @@ config PM
>  	  responsible for the actual handling of device suspend requests and
>  	  wake-up events.
>  
> +config PM_RPM_EXCEPTION
> +	bool "Prioritize Runtime Power Management more than Power Management"
> +	default n

The default is always 'n' so no need to specify it.

> +	help
> +	Provides a way to prioritize Runtime Power Management more than Power
> +	Management. This way system can suspnd with maintaining specific
> +	components in operation.

This really doesn't give me a good description of why someone would ever
want to enable this at all.

And why does this have to be a build option?  That feels very heavy, why
not make it changable at runtime?

If this is a build option, how are you going to get all the distros and
all of the Android/ChromeOS systems in the world to enable it?

thanks,

greg k-h