[PATCH v5 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch between perf->states[] and freq_table[]

[PATCH v5 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch between perf->states[] and freq_table[]

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

policy->freq_table is built by filtering out _PSS entries that are not
lower in frequency than the previous one, and the original ACPI P-state
index of each remaining entry is kept in freq_table[].driver_data.  The
index spaces of perf->states[] and freq_table[] therefore differ, and
mixing them up leads to the frequency of one P-state being attributed to
another.

Patch 1/2 is unchanged from v4 and fixes the cached frequency used by
get_cur_freq_on_cpu() to detect a firmware frequency change behind the
OS's back.

Patch 2/2 is new in v5.  As Rafael pointed out, Sashiko observed that
the extract_msr() fallback interferes with 1/2 and can render it
ineffective: when the status read from the hardware matches no
freq_table entry, extract_msr() returns freq_table[0].frequency instead
of reporting the status as unknown.  If the OS is already at the P-state
corresponding to freq_table[0], a firmware switch to a filtered-out
P-state then compares equal to the cached frequency, data->resume is
left clear and the next ->target() call does not rewrite the control
register.  So 1/2 alone does not make the detection work in that case,
and the two patches are posted together rather than separately as I had
suggested earlier.

v4: https://lkml.iu.edu/2609.1/07369.html

Changes since v4:
 - Drop "cpufreq: acpi-cpufreq: fix P-state index mismatch in
   extract_io()", which has been applied.  This series is based on top
   of it.
 - Add 2/2 to stop extract_msr() from masking firmware P-state changes,
   as reported by Sashiko, and note the dependency in 1/2's changelog.
 - 1/2: no functional change; changelog extended to say that it fixes
   only the cached side of the comparison, and a duplicate
   Signed-off-by was removed.

Li RongQing (2):
  cpufreq: acpi-cpufreq: fix P-state index mismatch in
    get_cur_freq_on_cpu()
  cpufreq: acpi-cpufreq: don't mask firmware P-state changes

 drivers/cpufreq/acpi-cpufreq.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
2.9.4

[PATCH v5 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu()

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

get_cur_freq_on_cpu() uses perf->state, which is an index into
perf->states[], to index policy->freq_table[]. However, freq_table[]
is built by filtering _PSS entries that are not lower in frequency
than the previous one, so its index space no longer matches
perf->states[]. The original P-state index for each remaining
freq_table entry is stored in driver_data.

Once an entry has been skipped, using perf->state as an index into
freq_table[] can therefore select the frequency of a different
P-state.

The reported current frequency itself remains correct because it is
obtained from extract_freq(). The mismatch only affects the cached
frequency used by get_cur_freq_on_cpu() to detect a firmware frequency
change behind our back.

If the wrong table entry contains a frequency different from the one
the CPU is actually running at, the check falsely detects a frequency
change and sets data->resume. The next ->target() call then performs a
redundant control-register write even if the requested P-state is
already the current P-state.

Conversely, if the wrong table entry happens to contain the frequency
to which firmware has changed the CPU, the frequency change is missed
and data->resume remains clear. A subsequent ->target() call for the
P-state that the cpufreq core believes to be current can then
short-circuit without rewriting the control register, leaving the CPU
at the firmware-selected frequency until a different P-state is
requested.

Fix this by taking the cached frequency directly from
perf->states[perf->state].core_frequency. perf->state and
perf->states[] use the same P-state index space, and converting
core_frequency to kHz yields the same value stored in the corresponding
freq_table entry during initialization.

This fixes the cached side of the comparison only. extract_msr() still
reports freq_table[0].frequency when the status read from the hardware
matches no freq_table entry, which defeats the comparison when the OS
is already at the P-state corresponding to freq_table[0]. That is
addressed by the next patch.

Fixes: e56a727b023d ("[CPUFREQ] Make acpi-cpufreq more robust against BIOS freq changes behind our back.")
Reported-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
Suggested-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index f64fa37..2d7d147 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -353,6 +353,7 @@ static u32 get_cur_val(const struct cpumask *mask, struct acpi_cpufreq_data *dat
 
 static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
 {
+	struct acpi_processor_performance *perf;
 	struct acpi_cpufreq_data *data;
 	struct cpufreq_policy *policy;
 	unsigned int freq;
@@ -368,7 +369,9 @@ static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
 	if (unlikely(!data || !policy->freq_table))
 		return 0;
 
-	cached_freq = policy->freq_table[to_perf_data(data)->state].frequency;
+	perf = to_perf_data(data);
+	cached_freq = perf->states[perf->state].core_frequency * 1000;
+
 	freq = extract_freq(policy, get_cur_val(cpumask_of(cpu), data));
 	if (freq != cached_freq) {
 		/*
-- 
2.9.4

[PATCH v5 2/2] cpufreq: acpi-cpufreq: don't mask firmware P-state changes

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

get_cur_freq_on_cpu() detects a firmware frequency change behind the
OS's back by comparing the frequency decoded from the hardware with the
frequency cached by the cpufreq core. The previous patch fixes the
cached side of that comparison; this patch fixes the decoded side.

policy->freq_table is built by filtering out _PSS entries that are not
lower in frequency than the previous one, so firmware may autonomously
switch the CPU to one of the filtered-out P-states. The status of such
a P-state matches no freq_table entry, and extract_msr() then falls
back to freq_table[0].frequency, i.e. the highest frequency, instead of
reporting that the status is unknown.

As a result the comparison is defeated exactly when the OS is already
at the P-state corresponding to freq_table[0], which is particularly likely 
under the performance governor: freq equals cached_freq, data->resume is
left clear, and the next ->target() call short-circuits without
rewriting the control register, leaving the CPU at the firmware-selected
P-state. Fixing the index mismatch alone therefore does not make the
detection work in that case.

Return 0 from extract_msr() when the status cannot be matched so that
the fallback no longer hides the firmware change, and treat a zero
result as a firmware frequency change in get_cur_freq_on_cpu(). Keep
returning the cached frequency to the cpufreq core in that case: a zero
return value there would be indistinguishable from a read failure and
would feed 0 into the cpufreq statistics. This also covers the
ACPI_ADR_SPACE_SYSTEM_IO path, where extract_io() already returns 0 for
an unmatched status.

check_freqs() is unaffected: it compares against a frequency taken from
freq_table, which is never 0, so an unknown status keeps the
verification loop from declaring success.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260910111329.2220-1-lirongqing@baidu.com
Fixes: e56a727b023d ("[CPUFREQ] Make acpi-cpufreq more robust against BIOS freq changes behind our back.")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 2d7d147..1e7dc1c 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -225,7 +225,7 @@ static unsigned extract_msr(struct cpufreq_policy *policy, u32 msr)
 	cpufreq_for_each_entry(pos, policy->freq_table)
 		if (msr == perf->states[pos->driver_data].status)
 			return pos->frequency;
-	return policy->freq_table[0].frequency;
+	return 0;
 }
 
 static unsigned extract_freq(struct cpufreq_policy *policy, u32 val)
@@ -373,12 +373,14 @@ static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
 	cached_freq = perf->states[perf->state].core_frequency * 1000;
 
 	freq = extract_freq(policy, get_cur_val(cpumask_of(cpu), data));
-	if (freq != cached_freq) {
+	if (!freq || freq != cached_freq) {
 		/*
 		 * The dreaded BIOS frequency change behind our back.
 		 * Force set the frequency on next target call.
 		 */
 		data->resume = 1;
+		if (!freq)
+			freq = cached_freq;
 	}
 
 	pr_debug("cur freq = %u\n", freq);
-- 
2.9.4

Re: [PATCH v5 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch between perf->states[] and freq_table[]

[Rafael J. Wysocki (Intel)]

On Fri, Nov 6, 2026 at 7:40 PM lirongqing <lirongqing@baidu.com> wrote:
>
> From: Li RongQing <lirongqing@baidu.com>
>
> policy->freq_table is built by filtering out _PSS entries that are not
> lower in frequency than the previous one, and the original ACPI P-state
> index of each remaining entry is kept in freq_table[].driver_data.  The
> index spaces of perf->states[] and freq_table[] therefore differ, and
> mixing them up leads to the frequency of one P-state being attributed to
> another.
>
> Patch 1/2 is unchanged from v4 and fixes the cached frequency used by
> get_cur_freq_on_cpu() to detect a firmware frequency change behind the
> OS's back.
>
> Patch 2/2 is new in v5.  As Rafael pointed out, Sashiko observed that
> the extract_msr() fallback interferes with 1/2 and can render it
> ineffective: when the status read from the hardware matches no
> freq_table entry, extract_msr() returns freq_table[0].frequency instead
> of reporting the status as unknown.  If the OS is already at the P-state
> corresponding to freq_table[0], a firmware switch to a filtered-out
> P-state then compares equal to the cached frequency, data->resume is
> left clear and the next ->target() call does not rewrite the control
> register.  So 1/2 alone does not make the detection work in that case,
> and the two patches are posted together rather than separately as I had
> suggested earlier.
>
> v4: https://lkml.iu.edu/2609.1/07369.html
>
> Changes since v4:
>  - Drop "cpufreq: acpi-cpufreq: fix P-state index mismatch in
>    extract_io()", which has been applied.  This series is based on top
>    of it.
>  - Add 2/2 to stop extract_msr() from masking firmware P-state changes,
>    as reported by Sashiko, and note the dependency in 1/2's changelog.

So Sashiko has feedback on this one now:

https://sashiko.dev/#/message/20261106184008.1728-2-lirongqing%40baidu.com

>  - 1/2: no functional change; changelog extended to say that it fixes
>    only the cached side of the comparison, and a duplicate
>    Signed-off-by was removed.
>
> Li RongQing (2):
>   cpufreq: acpi-cpufreq: fix P-state index mismatch in
>     get_cur_freq_on_cpu()
>   cpufreq: acpi-cpufreq: don't mask firmware P-state changes
>
>  drivers/cpufreq/acpi-cpufreq.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> --
> 2.9.4
>

答复: [外部邮件] Re: [PATCH v5 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch between perf->states[] and freq_table[]

[Li,Rongqing]

> > Patch 1/2 is unchanged from v4 and fixes the cached frequency used by
> > get_cur_freq_on_cpu() to detect a firmware frequency change behind the
> > OS's back.
> >
> > Patch 2/2 is new in v5.  As Rafael pointed out, Sashiko observed that
> > the extract_msr() fallback interferes with 1/2 and can render it
> > ineffective: when the status read from the hardware matches no
> > freq_table entry, extract_msr() returns freq_table[0].frequency
> > instead of reporting the status as unknown.  If the OS is already at
> > the P-state corresponding to freq_table[0], a firmware switch to a
> > filtered-out P-state then compares equal to the cached frequency,
> > data->resume is left clear and the next ->target() call does not
> > rewrite the control register.  So 1/2 alone does not make the
> > detection work in that case, and the two patches are posted together
> > rather than separately as I had suggested earlier.
> >
> > v4: https://lkml.iu.edu/2609.1/07369.html
> >
> > Changes since v4:
> >  - Drop "cpufreq: acpi-cpufreq: fix P-state index mismatch in
> >    extract_io()", which has been applied.  This series is based on top
> >    of it.
> >  - Add 2/2 to stop extract_msr() from masking firmware P-state changes,
> >    as reported by Sashiko, and note the dependency in 1/2's changelog.
> 
> So Sashiko has feedback on this one now:
> 
> https://sashiko.dev/#/message/20261106184008.1728-2-lirongqing%40baidu.
> com

This feedback is reasonable, I will send new version

thanks

[Li,Rongqing] 


> 
> >  - 1/2: no functional change; changelog extended to say that it fixes
> >    only the cached side of the comparison, and a duplicate
> >    Signed-off-by was removed.
> >
> > Li RongQing (2):
> >   cpufreq: acpi-cpufreq: fix P-state index mismatch in
> >     get_cur_freq_on_cpu()
> >   cpufreq: acpi-cpufreq: don't mask firmware P-state changes
> >
> >  drivers/cpufreq/acpi-cpufreq.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > --
> > 2.9.4
> >