Re: [PATCH v3 4/5] cpuidle: psci: Initialize the PM domains in powered off state for OSI

[Ulf Hansson]

On Fri, Sep 11, 2026 at 6:34 PM Dhruva G <goledhruva@xxxxxxxxx> wrote:
>
> On 07-09-2026 16:46, Ulf Hansson wrote:
> > At the point when the PM domain and the topology are registered through the
> > genpd subsystem, it's not really known whether corresponding CPUs are
> > online and thus if the PM domain should be initialized as powered on or
> > not. Instead this information becomes available when the CPU devices gets
> > attached to their respective PM domain through dt_idle_attach_cpu().
> >
> > This is a problem when using PSCI OS-initiated mode, as we may end up with
> > a PM domain that has the genpd's status indicating it to be powered on,
> > while it in fact may not be the case. In the less severe scenario, this
> > leads to selecting a shallower domain idle state for the PM domain than
> > necessary. A more critical problem is when a non-CPU device shares the PM
> > domain, leading to their corresponding drivers not being able to trust the
> > status of it.
> >
> > Let's fix these problems by initializing the state for the genpd's to be
> > powered off and in the deepest possible domain idle state, when using
> > OS-initiated mode. The support for ->sync_state() is maintained by setting
> > the GENPD_FLAG_POWER_UNKNOWN for the genpds in question.
> >
> > Reported-by: Maulik Shah <maulik.shah@xxxxxxxxxxxxxxxx>
> > Link: https://lore.kernel.org/all/20260811-domain_off_ss3-v1-0-6a0a0fc023f5@xxxxxxxxxxxxxxxx/
> > Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> > Tested-by: Yuanfang Zhang <yuanfang.zhang@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxxxxxxxx>
> > ---
> >
> > Changes in v3:
> >       - None.
> > Changes in v2:
> >       - Fix a bug in the call to pm_genpd_init().
> >
> > ---
> >  drivers/cpuidle/cpuidle-psci-domain.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c
> > index b9e4ad7d43a3..4d8c63d329c2 100644
> > --- a/drivers/cpuidle/cpuidle-psci-domain.c
> > +++ b/drivers/cpuidle/cpuidle-psci-domain.c
> > @@ -68,7 +68,8 @@ static int psci_pd_init(struct device_node *np, bool use_osi)
> >        */
> >       if (use_osi) {
> >               pd->power_off = psci_pd_power_off;
> > -             pd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
> > +             pd->flags |= GENPD_FLAG_ACTIVE_WAKEUP | GENPD_FLAG_POWER_UNKNOWN;
> > +             pd->state_idx = pd->state_count ? pd->state_count - 1 : 0;
> >               if (IS_ENABLED(CONFIG_PREEMPT_RT))
> >                       pd->flags |= GENPD_FLAG_RPM_ALWAYS_ON;
> >       } else {
> > @@ -78,7 +79,7 @@ static int psci_pd_init(struct device_node *np, bool use_osi)
> >       /* Use governor for CPU PM domains if it has some states to manage. */
> >       pd_gov = pd->states ? &pm_domain_cpu_gov : NULL;
> >
> > -     ret = pm_genpd_init(pd, pd_gov, false);
> > +     ret = pm_genpd_init(pd, pd_gov, use_osi);
>
> If CONFIG_PREEMPT_RT=y,
>
> psci_pd_init(use_osi=true)
>   -> sets GENPD_FLAG_POWER_UNKNOWN
>   -> sets GENPD_FLAG_RPM_ALWAYS_ON
>   -> pm_genpd_init(..., is_off=true)
>   -> genpd->status = GENPD_STATE_OFF
>   -> RPM_ALWAYS_ON + OFF is rejected (pmdomain/core.c: pm_genpd_init()
>                                       rejects an RPM_ALWAYS_ON domain whose initial state is OFF)
>   -> return -EINVAL
>
> Therefore, on a PREEMPT_RT platform using OSI and hierarchical PSCI domains, psci_cpuidle_domain_probe()
> should fail while initializing the first domain. The genpd providers are then unavailable, so
> dt_idle_attach_cpu() fails during PSCI cpuidle initialization and the driver rolls back its CPU
> registrations.
>
> I don't have a device on me to test this path, perhaps one of the QC devices + RT config
> can reproduce this?

No need to test this on HW, it's certainly a problem. I also noticed
that Sashiko pointed out this as well.

>
> Should PREEMPT_RT case instead initialize these domains as ON?

I don't think so. Instead, I think we can simply drop
GENPD_FLAG_RPM_ALWAYS_ON. The main reason to use it was mostly for
optimization reasons.

Since we don't assign psci_enter_domain_idle_state() to the
corresponding ->enter() callback for the idle states, when PREEMPT_RT
is set, this should still be fine without GENPD_FLAG_RPM_ALWAYS_ON.

I will give this a try and see how it plays out, cooking a v4.

>
> This might be safer here atleast:
>
> ret = pm_genpd_init(pd, pd_gov,
>                     use_osi && !IS_ENABLED(CONFIG_PREEMPT_RT));
>
> Regards,
> Dhruva

Again, thanks for reviewing!

Kind regards
Uffe