linux-kernel - Re: [PATCH 11/11] pmdomain: core: Leave powered-on genpds on until ->sync

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: <CAGETcx_hQRr1hRQD0vyAN9bhZRx+763zfHUG0oBi0sqFUi85pA@mail.gmail.com>
Date: Thu, 17 Apr 2025 17:50:45 -0700
From: Saravana Kannan <saravanak@...gle.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Stephen Boyd <sboyd@...nel.org>, linux-pm@...r.kernel.org, 
	"Rafael J . Wysocki" <rafael@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Michael Grzeschik <m.grzeschik@...gutronix.de>, Bjorn Andersson <andersson@...nel.org>, 
	Abel Vesa <abel.vesa@...aro.org>, Devarsh Thakkar <devarsht@...v0571a.ent.ti.com>, 
	Peng Fan <peng.fan@....nxp.com>, Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, 
	Johan Hovold <johan@...nel.org>, Maulik Shah <maulik.shah@....qualcomm.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 11/11] pmdomain: core: Leave powered-on genpds on until ->sync_state()

On Thu, Apr 17, 2025 at 7:25 AM Ulf Hansson <ulf.hansson@...aro.org> wrote:
>
> Powering-off a genpd that was on during boot, before all of its consumer
> devices have been probed, is certainly prone to problems.
>
> Let's fix these problems by preventing these genpds from being powered-off
> until ->sync_state(). Note that, this only works for OF based platform as
> ->sync_state() are relying on fw_devlink.

For non-OF platforms, this will still wait until late_initcall(). So,
there's at least SOME protection. We could potentially even move that
to happen after deferred probe timeout expires.

-Saravana

>
> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> ---
>  drivers/pmdomain/core.c   | 12 +++++++++++-
>  include/linux/pm_domain.h |  1 +
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 695d7d9e5582..a8c56f7a7ba0 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -212,6 +212,12 @@ static inline bool irq_safe_dev_in_sleep_domain(struct device *dev,
>         return ret;
>  }
>
> +#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
> +static bool genpd_may_stay_on(bool on) { return on; }
> +#else
> +static bool genpd_may_stay_on(bool on) { return false; }
> +#endif
> +
>  static int genpd_runtime_suspend(struct device *dev);
>
>  /*
> @@ -933,11 +939,12 @@ static void genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
>          * The domain is already in the "power off" state.
>          * System suspend is in progress.
>          * The domain is configured as always on.
> +        * The domain was on at boot and still need to stay on.
>          * The domain has a subdomain being powered on.
>          */
>         if (!genpd_status_on(genpd) || genpd->prepared_count > 0 ||
>             genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd) ||
> -           atomic_read(&genpd->sd_count) > 0)
> +           genpd->stay_on || atomic_read(&genpd->sd_count) > 0)
>                 return;
>
>         /*
> @@ -2374,6 +2381,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
>         INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
>         atomic_set(&genpd->sd_count, 0);
>         genpd->status = is_off ? GENPD_STATE_OFF : GENPD_STATE_ON;
> +       genpd->stay_on = genpd_may_stay_on(!is_off);
>         genpd->sync_state = GENPD_SYNC_STATE_OFF;
>         genpd->device_count = 0;
>         genpd->provider = NULL;
> @@ -2640,6 +2648,7 @@ void of_genpd_sync_state(struct device *dev)
>         list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
>                 if (genpd->provider == &np->fwnode) {
>                         genpd_lock(genpd);
> +                       genpd->stay_on = false;
>                         genpd_power_off(genpd, false, 0);
>                         genpd_unlock(genpd);
>                 }
> @@ -3486,6 +3495,7 @@ static void genpd_provider_sync_state(struct device *dev)
>
>         case GENPD_SYNC_STATE_SIMPLE:
>                 genpd_lock(genpd);
> +               genpd->stay_on = false;
>                 genpd_power_off(genpd, false, 0);
>                 genpd_unlock(genpd);
>                 break;
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 2185ee9e4f7c..c5358cccacad 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -193,6 +193,7 @@ struct generic_pm_domain {
>         unsigned int performance_state; /* Aggregated max performance state */
>         cpumask_var_t cpus;             /* A cpumask of the attached CPUs */
>         bool synced_poweroff;           /* A consumer needs a synced poweroff */
> +       bool stay_on;                   /* Stay powered-on during boot. */
>         enum genpd_sync_state sync_state; /* How sync_state is managed. */
>         int (*power_off)(struct generic_pm_domain *domain);
>         int (*power_on)(struct generic_pm_domain *domain);
> --
> 2.43.0
>