[PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups

[PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

policy->freq_table is built with duplicate _PSS frequencies dropped, so it
no longer lines up positionally with perf->states[]; the original P-state
index is kept in freq_table[].driver_data. extract_io() and
get_cur_freq_on_cpu() indexed freq_table[] with a perf->states[] index
anyway, which can return the frequency of a wrong P-state.

Changes since v1:
- Reword patch 1: v1 described it as an out-of-bounds read, which is wrong;
  it is an index-mismatch / wrong-result bug, not a memory-safety issue.
- Add patch 2 for get_cur_freq_on_cpu().
- Add Fixes: tags (not exact, but reasonable backport targets).

Li RongQing (2):
  cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io()
  cpufreq: acpi-cpufreq: fix P-state index mismatch in
    get_cur_freq_on_cpu()

 drivers/cpufreq/acpi-cpufreq.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

-- 
2.9.4

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

[lirongqing]

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

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")
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;
 }
 
-- 
2.9.4

[PATCH v2 2/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() reads the cached frequency as
policy->freq_table[to_perf_data(data)->state], mixing two different index
spaces: perf->state indexes perf->states[], while policy->freq_table[] is
built with duplicate frequencies removed and stores the original P-state
index in freq_table[].driver_data.

Once any _PSS entry has been skipped the two arrays no longer line up, so
the cached frequency used to detect a "BIOS changed frequency behind our
back" event could be taken from the wrong table slot.

Look up the freq_table entry whose driver_data matches perf->state instead
of indexing freq_table[] with perf->state directly.

Fixes: 8cee1eed8e78 ("cpufreq: ACPI: Remove freq_table from acpi_cpufreq_data")
Reported-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 1abe9ab..61ede49c 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 cpufreq_frequency_table *pos;
 	struct acpi_cpufreq_data *data;
 	struct cpufreq_policy *policy;
 	unsigned int freq;
@@ -368,7 +369,13 @@ 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;
+	cached_freq = 0;
+	cpufreq_for_each_entry(pos, policy->freq_table)
+		if (pos->driver_data == to_perf_data(data)->state) {
+			cached_freq = pos->frequency;
+			break;
+		}
+
 	freq = extract_freq(policy, get_cur_val(cpumask_of(cpu), data));
 	if (freq != cached_freq) {
 		/*
-- 
2.9.4

Re: [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups

[Zhongqiu Han]

On 8/13/2026 5:01 PM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> policy->freq_table is built with duplicate _PSS frequencies dropped, so it
> no longer lines up positionally with perf->states[]; the original P-state
> index is kept in freq_table[].driver_data. extract_io() and
> get_cur_freq_on_cpu() indexed freq_table[] with a perf->states[] index
> anyway, which can return the frequency of a wrong P-state.
> 
> Changes since v1:
> - Reword patch 1: v1 described it as an out-of-bounds read, which is wrong;
>    it is an index-mismatch / wrong-result bug, not a memory-safety issue.
> - Add patch 2 for get_cur_freq_on_cpu().
> - Add Fixes: tags (not exact, but reasonable backport targets).

Hi RongQing,

The Fixes tag should point to the commit that introduced the bug.

> 
> Li RongQing (2):
>    cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io()
>    cpufreq: acpi-cpufreq: fix P-state index mismatch in
>      get_cur_freq_on_cpu()
> 
>   drivers/cpufreq/acpi-cpufreq.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 


-- 
Thx and BRs,
Zhongqiu Han

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

[Zhongqiu Han]

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

答复: [外部邮件] Re: [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io()

[Li,Rongqing]

> > 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().
> 

Thanks, I will send v3

[Li,Rongqing] 


> 
> > 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

Re: 答复: [外部邮件] Re: [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io()

[Zhongqiu Han]

On 8/18/2026 2:46 PM, Li,Rongqing wrote:
>>> 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().
>>
> 
> Thanks, I will send v3

Thanks, I will review the patch v2 2/2

> 
> [Li,Rongqing]
> 
> 
>>
>>> 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
> 


-- 
Thx and BRs,
Zhongqiu Han

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

[Zhongqiu Han]

On 8/13/2026 5:01 PM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> get_cur_freq_on_cpu() reads the cached frequency as
> policy->freq_table[to_perf_data(data)->state], mixing two different index
> spaces: perf->state indexes perf->states[], while policy->freq_table[] is
> built with duplicate frequencies removed and stores the original P-state
> index in freq_table[].driver_data.
> 
> Once any _PSS entry has been skipped the two arrays no longer line up, so
> the cached frequency used to detect a "BIOS changed frequency behind our
> back" event could be taken from the wrong table slot.

A further consequence of this index-space mismatch should be that,
depending on which entry is picked, the check either fails on every call
for P-states whose freq_table index differs from their _PSS index,
causing data->resume to force a redundant control-register rewrite on
every ->target(), or silently passes when the wrong slot happens to hold
the frequency the firmware actually moved the CPU to, causing
acpi_cpufreq_target() to short-circuit and leave the CPU running at a
frequency the core does not expect until a different P-state is
requested.

> 
> Look up the freq_table entry whose driver_data matches perf->state instead
> of indexing freq_table[] with perf->state directly.
> 
> Fixes: 8cee1eed8e78 ("cpufreq: ACPI: Remove freq_table from acpi_cpufreq_data")

The real Fixes tag should be e56a727b023d ("[CPUFREQ] Make acpi-cpufreq
more robust against BIOS freq changes behind our back.")

> Reported-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>   drivers/cpufreq/acpi-cpufreq.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 1abe9ab..61ede49c 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 cpufreq_frequency_table *pos;
>   	struct acpi_cpufreq_data *data;
>   	struct cpufreq_policy *policy;
>   	unsigned int freq;
> @@ -368,7 +369,13 @@ 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;

How about:
	struct acpi_processor_performance *perf = to_perf_data(data);
	...
	cached_freq = perf->states[perf->state].core_frequency * 1000;

get_cur_freq_on_cpu() is only called on ACPI_ADR_SPACE_FIXED_HARDWARE
platforms, and on such platforms perf->state is only assigned in the
following functions:

(1) acpi_cpufreq_target(): perf->state is then the index of the P-state
     last written to the hardware.
(2) acpi_cpufreq_fast_switch(): same as above (1).

(3) acpi_cpufreq_cpu_init(): this sets the initial value perf->state =
     0. In cpufreq_online(), after .init() has been called, .get() - i.e.
     get_cur_freq_on_cpu() - is called once.  The freq read at that point
     may be a leftover value from the hardware, but whether or not
     "if (freq != cached_freq)" holds, the only consequence is
     data->resume = 1, and data->resume has already been initialised to 1
     in .init() anyway.


> +	cached_freq = 0;
> +	cpufreq_for_each_entry(pos, policy->freq_table)
> +		if (pos->driver_data == to_perf_data(data)->state) {
> +			cached_freq = pos->frequency;
> +			break;
> +		}
> +
>   	freq = extract_freq(policy, get_cur_val(cpumask_of(cpu), data));
>   	if (freq != cached_freq) {
>   		/*


-- 
Thx and BRs,
Zhongqiu Han

答复: [外部邮件] Re: [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu()

[Li,Rongqing]

> -----邮件原件-----
> 发件人: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> 发送时间: 2026年8月18日 22:02
> 收件人: Li,Rongqing <lirongqing@baidu.com>; Rafael J . Wysocki
> <rafael@kernel.org>; Viresh Kumar <viresh.kumar@linaro.org>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> 抄送: zhongqiu.han@oss.qualcomm.com
> 主题: [外部邮件] Re: [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index
> mismatch in get_cur_freq_on_cpu()
> 
> On 8/13/2026 5:01 PM, lirongqing wrote:
> > From: Li RongQing <lirongqing@baidu.com>
> >
> > get_cur_freq_on_cpu() reads the cached frequency as
> > policy->freq_table[to_perf_data(data)->state], mixing two different
> > policy->index
> > spaces: perf->state indexes perf->states[], while policy->freq_table[]
> > is built with duplicate frequencies removed and stores the original
> > P-state index in freq_table[].driver_data.
> >
> > Once any _PSS entry has been skipped the two arrays no longer line up,
> > so the cached frequency used to detect a "BIOS changed frequency
> > behind our back" event could be taken from the wrong table slot.
> 
> A further consequence of this index-space mismatch should be that, depending
> on which entry is picked, the check either fails on every call for P-states whose
> freq_table index differs from their _PSS index, causing data->resume to force a
> redundant control-register rewrite on every ->target(), or silently passes when
> the wrong slot happens to hold the frequency the firmware actually moved the
> CPU to, causing
> acpi_cpufreq_target() to short-circuit and leave the CPU running at a frequency
> the core does not expect until a different P-state is requested.
> 
> >
> > Look up the freq_table entry whose driver_data matches perf->state
> > instead of indexing freq_table[] with perf->state directly.
> >
> > Fixes: 8cee1eed8e78 ("cpufreq: ACPI: Remove freq_table from
> > acpi_cpufreq_data")
> 
> The real Fixes tag should be e56a727b023d ("[CPUFREQ] Make acpi-cpufreq
> more robust against BIOS freq changes behind our back.")
> 
> > Reported-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >   drivers/cpufreq/acpi-cpufreq.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/acpi-cpufreq.c
> > b/drivers/cpufreq/acpi-cpufreq.c index 1abe9ab..61ede49c 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 cpufreq_frequency_table *pos;
> >   	struct acpi_cpufreq_data *data;
> >   	struct cpufreq_policy *policy;
> >   	unsigned int freq;
> > @@ -368,7 +369,13 @@ 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;
> 
> How about:
> 	struct acpi_processor_performance *perf = to_perf_data(data);
> 	...
> 	cached_freq = perf->states[perf->state].core_frequency * 1000;
> 

Thanks, that's cleaner and I've adopted it in v3.

Using perf->state to index perf->states[] (its native index space) is exactly right: it avoids the freq_table[] index-space mismatch at the root, and perf->states[perf->state].core_frequency * 1000 is the same value that gets stored into freq_table[] at init time, so the freq != cached_freq resync check is unchanged.

It also removes a corner case in my earlier freq_table-walk version: if perf->state points at a P-state that was deduplicated out of freq_table[], the walk finds no match and leaves cached_freq at 0, misfiring the check. Indexing perf->states[] directly avoids that.

Will send as part of v3.

[Li,Rongqing] 


> get_cur_freq_on_cpu() is only called on ACPI_ADR_SPACE_FIXED_HARDWARE
> platforms, and on such platforms perf->state is only assigned in the following
> functions:
> 
> (1) acpi_cpufreq_target(): perf->state is then the index of the P-state
>      last written to the hardware.
> (2) acpi_cpufreq_fast_switch(): same as above (1).
> 
> (3) acpi_cpufreq_cpu_init(): this sets the initial value perf->state =
>      0. In cpufreq_online(), after .init() has been called, .get() - i.e.
>      get_cur_freq_on_cpu() - is called once.  The freq read at that point
>      may be a leftover value from the hardware, but whether or not
>      "if (freq != cached_freq)" holds, the only consequence is
>      data->resume = 1, and data->resume has already been initialised to 1
>      in .init() anyway.
> 
> 
> > +	cached_freq = 0;
> > +	cpufreq_for_each_entry(pos, policy->freq_table)
> > +		if (pos->driver_data == to_perf_data(data)->state) {
> > +			cached_freq = pos->frequency;
> > +			break;
> > +		}
> > +
> >   	freq = extract_freq(policy, get_cur_val(cpumask_of(cpu), data));
> >   	if (freq != cached_freq) {
> >   		/*
> 
> 
> --
> Thx and BRs,
> Zhongqiu Han