Re: [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io()

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

On 8/13/2026 5:01 PM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> When policy->freq_table is built in acpi_cpufreq_cpu_init(), entries
> with duplicate frequencies are skipped. The original P-state index for
> each remaining entry is stored in freq_table[].driver_data, so the index
> space of freq_table no longer matches perf->states[].
> 
> extract_io() walks perf->states[] with index i and uses the same i to
> index policy->freq_table[i]. This causes two problems when duplicate
> frequencies exist:
> 
>    - Returning the frequency of the wrong P-state
>    - Returning 0 from a zeroed tail entry, or even CPUFREQ_TABLE_END
>      (~1u) reported as 0xfffffffe kHz

AFAICT, It may be worth expanding the changelog to cover the
consequences described below , particularly the resulting divergence
between the hardware state and the cpufreq core's view of the current
frequency:

Furthermore, extract_io() is only reachable on ACPI_ADR_SPACE_SYSTEM_IO
platforms, where cpufreq_driver->get is not installed, so its only
caller in practice is check_freqs(), i.e. only when the
acpi_pstate_strict module parameter is set.

There, the mismatched lookup makes the frequency comparison fail even
though drv_write() has already switched the hardware to the requested
P-state.  So check_freqs() sleeps through all 100 iterations - at least
~1 ms of usleep_range() plus 100 cross-CPU calls and I/O port reads, all
with policy->rwsem held - and ->target_index() returns -EAGAIN. perf
->state is therefore left at its previous value while the hardware sits
at the new one.

The core then restores policy->cur to the old frequency, and
because __cpufreq_driver_target() returns early when the requested
frequency equals policy->cur, the driver is not called again for it - so
the control register is not rewritten and the CPU is left running at a
frequency the core does not know about.


> 
> Fix it by walking policy->freq_table with cpufreq_for_each_entry() and
> using perf->states[pos->driver_data].status, aligning with extract_msr().
> 
> Fixes: 8cee1eed8e78 ("cpufreq: ACPI: Remove freq_table from acpi_cpufreq_data")

The real tag should be fe27cb358835 ("[CPUFREQ][2/8] acpi:
reorganize code to make MSR support addition easier")

That commit added both the entry-skipping loop with the
freq_table[].index (now .driver_data) back-pointer, which is what makes
the two indices diverge, and the faulty lookup itself -- back then in a
function called extract_freq(). dde9f7ba60ad ("[CPUFREQ][3/8] acpi
cpufreq: Pull in MSR based transition support") merely renamed it to
extract_io().


> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>   drivers/cpufreq/acpi-cpufreq.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 21639d9..1abe9ab 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -197,14 +197,13 @@ static unsigned extract_io(struct cpufreq_policy *policy, u32 value)
>   {
>   	struct acpi_cpufreq_data *data = policy->driver_data;
>   	struct acpi_processor_performance *perf;
> -	int i;
> +	struct cpufreq_frequency_table *pos;
>   
>   	perf = to_perf_data(data);
>   
> -	for (i = 0; i < perf->state_count; i++) {
> -		if (value == perf->states[i].status)
> -			return policy->freq_table[i].frequency;
> -	}
> +	cpufreq_for_each_entry(pos, policy->freq_table)
> +		if (value == perf->states[pos->driver_data].status)
> +			return pos->frequency;
>   	return 0;
>   }
>   


-- 
Thx and BRs,
Zhongqiu Han