linux-kernel - Re: [PATCH v2] PM: domains: Prevent power off for parent unless child is in deepest state

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 for Android: free password hash cracker in your pocket

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

Message-ID: <a869b705-c10e-ed0b-4119-35ef0a028311@gmail.com>
Date:   Fri, 18 Feb 2022 02:11:51 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org
Cc:     Kevin Hilman <khilman@...nel.org>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Dong Aisheng <aisheng.dong@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] PM: domains: Prevent power off for parent unless child
 is in deepest state

17.02.2022 15:49, Ulf Hansson пишет:
> A PM domain managed by genpd may support multiple idlestates (power-off
> states). During genpd_power_off() a genpd governor may be asked to select
> one of the idlestates based upon the dev PM QoS constraints, for example.
> 
> However, there is a problem with the behaviour around this in genpd. More
> precisely, a parent-domain is allowed to be powered off, no matter of what
> idlestate that has been selected for the child-domain.
> 
> For the stm32mp1 platform from STMicro, this behaviour doesn't play well.
> Instead, the parent-domain must not be powered off, unless the deepest
> idlestate has been selected for the child-domain. As the current behaviour
> in genpd is quite questionable anyway, let's simply change it into what is
> needed by the stm32mp1 platform.
> 
> If it surprisingly turns out that other platforms may need a different
> behaviour from genpd, then we will have to revisit this to find a way to
> make it configurable.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> ---
> 
> Changes in v2:
> 	- Clarified commit message - based upon discussions with Dmitry.
> 	- Updated a comment in the code, suggested by Dmitry.
> 
> ---
>  drivers/base/power/domain.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 5db704f02e71..c87588c21700 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -636,6 +636,18 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
>  			atomic_read(&genpd->sd_count) > 0)
>  		return -EBUSY;
>  
> +	/*
> +	 * The children must be in their deepest (powered-off) states to allow
> +	 * the parent to be powered off. Note that, there's no need for
> +	 * additional locking, as powering on a child, requires the parent's
> +	 * lock to be acquired first.
> +	 */
> +	list_for_each_entry(link, &genpd->parent_links, parent_node) {
> +		struct generic_pm_domain *child = link->child;
> +		if (child->state_idx < child->state_count - 1)
> +			return -EBUSY;
> +	}
> +
>  	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
>  		enum pm_qos_flags_status stat;
>  
> @@ -1073,6 +1085,13 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
>  	    || atomic_read(&genpd->sd_count) > 0)
>  		return;
>  
> +	/* Check that the children are in their deepest (powered-off) state. */
> +	list_for_each_entry(link, &genpd->parent_links, parent_node) {
> +		struct generic_pm_domain *child = link->child;
> +		if (child->state_idx < child->state_count - 1)
> +			return;
> +	}
> +
>  	/* Choose the deepest state when suspending */
>  	genpd->state_idx = genpd->state_count - 1;
>  	if (_genpd_power_off(genpd, false))

Thank you, looks good. Although, this should be v3.

Reviewed-by: Dmitry Osipenko <digetx@...il.com>