* [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 [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, linux-kernel > > 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, linux-kernel 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 [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, linux-kernel > -----邮件原件----- > 发件人: 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®