Re: [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io()
Zhongqiu Han <[email protected]>
| Newsgroups | org.kernel.vger.linux-pm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/13/2026 5:01 PM, lirongqing wrote: > From: Li RongQing <[email protected]> > > 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 <[email protected]> > --- > 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