* [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups
@ 2026-08-13 9:01 lirongqing
2026-08-13 9:01 ` [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() lirongqing
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: lirongqing @ 2026-08-13 9:01 UTC (permalink / raw)
To: Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-kernel
Cc: zhongqiu.han, Li RongQing
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() 2026-08-13 9:01 [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups lirongqing @ 2026-08-13 9:01 ` lirongqing 2026-08-17 13:33 ` Zhongqiu Han 2026-08-13 9:01 ` [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu() lirongqing 2026-08-17 12:45 ` [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups Zhongqiu Han 2 siblings, 1 reply; 9+ messages in thread From: lirongqing @ 2026-08-13 9:01 UTC (permalink / raw) To: Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-kernel Cc: zhongqiu.han, Li RongQing 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() 2026-08-13 9:01 ` [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() lirongqing @ 2026-08-17 13:33 ` Zhongqiu Han 2026-08-18 6:46 ` 答复: [外部邮件] " Li,Rongqing 0 siblings, 1 reply; 9+ messages in thread From: Zhongqiu Han @ 2026-08-17 13:33 UTC (permalink / raw) To: lirongqing, Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-kernel Cc: 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: [外部邮件] Re: [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() 2026-08-17 13:33 ` Zhongqiu Han @ 2026-08-18 6:46 ` Li,Rongqing 2026-08-18 8:38 ` Zhongqiu Han 0 siblings, 1 reply; 9+ messages in thread From: Li,Rongqing @ 2026-08-18 6:46 UTC (permalink / raw) To: Zhongqiu Han, Rafael J . Wysocki, Viresh Kumar, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 答复: [外部邮件] Re: [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() 2026-08-18 6:46 ` 答复: [外部邮件] " Li,Rongqing @ 2026-08-18 8:38 ` Zhongqiu Han 0 siblings, 0 replies; 9+ messages in thread From: Zhongqiu Han @ 2026-08-18 8:38 UTC (permalink / raw) To: Li,Rongqing, Rafael J . Wysocki, Viresh Kumar, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Cc: 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu() 2026-08-13 9:01 [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups lirongqing 2026-08-13 9:01 ` [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() lirongqing @ 2026-08-13 9:01 ` lirongqing 2026-08-18 14:02 ` Zhongqiu Han 2026-08-17 12:45 ` [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups Zhongqiu Han 2 siblings, 1 reply; 9+ messages in thread From: lirongqing @ 2026-08-13 9:01 UTC (permalink / raw) To: Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-kernel Cc: zhongqiu.han, Li RongQing 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu() 2026-08-13 9:01 ` [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu() lirongqing @ 2026-08-18 14:02 ` Zhongqiu Han 2026-08-19 11:21 ` 答复: [外部邮件] " Li,Rongqing 0 siblings, 1 reply; 9+ messages in thread From: Zhongqiu Han @ 2026-08-18 14:02 UTC (permalink / raw) To: lirongqing, Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-kernel Cc: 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: [外部邮件] Re: [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu() 2026-08-18 14:02 ` Zhongqiu Han @ 2026-08-19 11:21 ` Li,Rongqing 0 siblings, 0 replies; 9+ messages in thread From: Li,Rongqing @ 2026-08-19 11:21 UTC (permalink / raw) To: Zhongqiu Han, Rafael J . Wysocki, Viresh Kumar, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org > -----邮件原件----- > 发件人: 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups 2026-08-13 9:01 [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups lirongqing 2026-08-13 9:01 ` [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() lirongqing 2026-08-13 9:01 ` [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu() lirongqing @ 2026-08-17 12:45 ` Zhongqiu Han 2 siblings, 0 replies; 9+ messages in thread From: Zhongqiu Han @ 2026-08-17 12:45 UTC (permalink / raw) To: lirongqing, Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-kernel Cc: 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-19 11:23 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-13 9:01 [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups lirongqing 2026-08-13 9:01 ` [PATCH v2 1/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in extract_io() lirongqing 2026-08-17 13:33 ` Zhongqiu Han 2026-08-18 6:46 ` 答复: [外部邮件] " Li,Rongqing 2026-08-18 8:38 ` Zhongqiu Han 2026-08-13 9:01 ` [PATCH v2 2/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in get_cur_freq_on_cpu() lirongqing 2026-08-18 14:02 ` Zhongqiu Han 2026-08-19 11:21 ` 答复: [外部邮件] " Li,Rongqing 2026-08-17 12:45 ` [PATCH v2 0/2] cpufreq: acpi-cpufreq: fix P-state index mismatch in freq lookups Zhongqiu Han
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.