Re: [PATCH] cpuidle: psci: Assign domain callbacks to all CPU idle states

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The genpd governor bypasses the cpuidle governor's short-sleep predictions for shallow states, forcing CPUs into deep sleep and violating exit latency requirements.
--

commit 377e8584faa669a41142f336e4973cde237d354b
Author: Kevin Hilman (TI) <khilman@baylibre.com>

cpuidle: psci: Assign domain callbacks to all CPU idle states

This commit assigns domain-aware callbacks to all non-WFI CPU idle states.
This ensures the domain-idle-state logic and cluster-level states can be
triggered regardless of the selected CPU idle state.

> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
> index dcf20ea5ef5e3..db9aa57c51f55 100644
> --- a/drivers/cpuidle/cpuidle-psci.c
> +++ b/drivers/cpuidle/cpuidle-psci.c

[ ... ]

> @@ -261,14 +263,22 @@ static int psci_dt_cpu_init_topology(struct cpuidle_driver *drv,
>  	psci_cpuidle_use_syscore = true;
>  
>  	/*
> -	 * Using the deepest state for the CPU to trigger a potential selection
> -	 * of a shared state for the domain, assumes the domain states are all
> -	 * deeper states. On PREEMPT_RT the hierarchical topology is limited to
> -	 * s2ram and s2idle.
> +	 * Assign the domain-aware callbacks to all CPU idle states so that the
> +	 * domain-idle-state logic is triggered regardless of which CPU idle
> +	 * state is selected.
> +	 *
> +	 * For s2idle, enter_s2idle uses dev_pm_genpd_suspend() which is safe
> +	 * on PREEMPT_RT. find_deepest_state() will pick the deepest state
> +	 * whose exit latency fits within the active QoS constraint.
> +	 *
> +	 * For the normal idle path, enter uses pm_runtime_put_sync_suspend()
> +	 * which may sleep and is therefore not used on PREEMPT_RT.
>  	 */
> -	drv->states[state_count - 1].enter_s2idle = psci_enter_s2idle_domain_idle_state;
> -	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> -		drv->states[state_count - 1].enter = psci_enter_domain_idle_state;
> +	for (i = 1; i < state_count; i++) {
> +		drv->states[i].enter_s2idle = psci_enter_s2idle_domain_idle_state;
> +		if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> +			drv->states[i].enter = psci_enter_domain_idle_state;
> +	}

[Severity: High]
Does assigning these domain callbacks to shallow states bypass the cpuidle
governor's short-sleep predictions?

If the cpuidle governor predicts a short sleep (such as from frequent
interrupts) and selects a shallow CPU idle state, 
psci_enter_domain_idle_state() calls pm_runtime_put_sync_suspend(). 
This drops the CPU PM domain refcount and triggers the genpd governor.

The genpd governor calculates idle duration strictly based on next_hrtimer,
ignoring the cpuidle governor's statistical short-sleep prediction. If the
next timer is distant, the genpd governor will select a deep cluster state
and update psci_domain_state.state.

When this happens, __psci_enter_domain_idle_state() unconditionally
overrides the CPU's chosen shallow state parameter with the deeper
composite state:

	ds = this_cpu_ptr(&psci_domain_state);
	if (ds->state)
		state = ds->state;

	trace_psci_domain_idle_enter(dev->cpu, state, s2idle);
	ret = psci_cpu_suspend_enter(state) ? -1 : idx;

Could this force the CPU into deep sleep, causing exit latency constraint
violations when the expected short-term interrupt arrives?

Additionally, since __psci_enter_domain_idle_state() returns the shallow
state idx, will this incorrectly attribute deep-sleep latency and residency
to the shallow state and corrupt cpuidle statistics?

>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-topic-lpm-psci-domain-callbacks-v1-1-4a47d95c9c79@baylibre.com?part=1