[RESEND][PATCH 0/3] ACPI: CPPC: Reduce .target() callback overhead

Christian Loehle posted 3 patches 1 month, 4 weeks ago
drivers/acpi/cppc_acpi.c | 135 +++++++++++++++++++++++++++++++--------
include/acpi/cppc_acpi.h |   5 +-
2 files changed, 113 insertions(+), 27 deletions(-)
[RESEND][PATCH 0/3] ACPI: CPPC: Reduce .target() callback overhead
Posted by Christian Loehle 1 month, 4 weeks ago
cppc-cpufreq reaches cppc_set_perf() from every target callback.  For
direct SystemMemory controls, that path currently does several steps
which are unnecessary once the immutable _CPC layout is known:

  - a full-width write first reads the access unit and merges the value.
  - every write takes the descriptor's RMW lock, even when its access unit
    is not shared with another _CPC entry.
  - cppc_set_perf() evaluates the same three PCC predicates at each phase
    of the transaction.

Remove those costs while retaining the existing conservative paths for
partial fields, overlapping or malformed access units, and PCC controls.

The series was tested on Arm Power-Orion O6 and AmpereOne systems using
cppc-cpufreq and schedutil.  An rt-app task pinned to one CPU ran for
500 us every 2 ms with uclamp.min=512, for 5000 periods per run.  schedutil
rate_limit_us was 1000.  cppc_cpufreq_fast_switch() latency was measured
for 100 ten-second runs.  Each sample is the mean callback latency within
one run, and the table reports the median and sample standard deviation
of those samples.

Orion O6               median       stdev       callbacks
baseline               5703 ns      307 ns       188017
complete series        5023 ns      240 ns       188209
==> 680 ns (11.9%) reduction

AmpereOne              median       stdev
baseline               2090 ns      157 ns
complete series      1907.5 ns      140 ns
==> 182.5 ns (8.7%) reduction

The cumulative intermediate results on the Orion O6 attribute roughly half
of the gain to each of the first two patches: avoiding the read reduced
the median by 284 ns (5.0%), and avoiding the lock reduced it by another
336 ns (6.2%).
Together they account for 620 ns of the 680 ns total reduction.

With the same arm64 configuration and GCC 11.4, caching the PCC predicate
also reduces cppc_set_perf() from 1124 to 884 bytes.  The generated
function has 60 fewer instructions, 17 fewer loads and 20 fewer branches.

This series is based on the CPPC fixes posted at (already queued):
https://lore.kernel.org/lkml/20260722093825.1030594-1-christian.loehle@arm.com/

and the separately posted fix still under review:
https://lore.kernel.org/lkml/20260724104042.1481804-1-christian.loehle@arm.com/

PS:
There's a final optimization that I actually wanted to make but decided to
split it out for now as it somewhat replicated Sumit's series:
Skipping redundant perf ctrl writes in cpufreq-cppc if registers are non-PCC
and !shared (because the values are unchanged, the
!autonomous-common-case), but that requires the driver to have a more
complex caching- and atomic-updating machinery in place, similar to
hotplug. As opposed to this series the optimization would be for the
microcontroller handling the CPPC requests, which may be shared across
many CPUs and therefore redundant requests can increase the dvfs
transition latency.
That patch will follow once Sumit's is queued:
https://lore.kernel.org/lkml/20260716153820.2007095-1-sumitg@nvidia.com/

[RESEND] for the new base-commit specifier below

Christian Loehle (3):
  ACPI: CPPC: Avoid unnecessary reads for full-width writes
  ACPI: CPPC: Avoid locking standalone full-width registers
  ACPI: CPPC: Evaluate performance-control PCC use once

 drivers/acpi/cppc_acpi.c | 135 +++++++++++++++++++++++++++++++--------
 include/acpi/cppc_acpi.h |   5 +-
 2 files changed, 113 insertions(+), 27 deletions(-)


base-commit: 075b74841bd0065a3bda3440873c747938e69b68
-- 
2.34.1
Re: [RESEND][PATCH 0/3] ACPI: CPPC: Reduce .target() callback overhead
Posted by Christian Loehle 1 month, 4 weeks ago
On 8/3/26 22:05, Christian Loehle wrote:
> cppc-cpufreq reaches cppc_set_perf() from every target callback.  For
> direct SystemMemory controls, that path currently does several steps
> which are unnecessary once the immutable _CPC layout is known:
> 
>   - a full-width write first reads the access unit and merges the value.
>   - every write takes the descriptor's RMW lock, even when its access unit
>     is not shared with another _CPC entry.
>   - cppc_set_perf() evaluates the same three PCC predicates at each phase
>     of the transaction.
> 
> Remove those costs while retaining the existing conservative paths for
> partial fields, overlapping or malformed access units, and PCC controls.
> 
> The series was tested on Arm Power-Orion O6 and AmpereOne systems using
> cppc-cpufreq and schedutil.  An rt-app task pinned to one CPU ran for
> 500 us every 2 ms with uclamp.min=512, for 5000 periods per run.  schedutil
> rate_limit_us was 1000.  cppc_cpufreq_fast_switch() latency was measured
> for 100 ten-second runs.  Each sample is the mean callback latency within
> one run, and the table reports the median and sample standard deviation
> of those samples.
> 
> Orion O6               median       stdev       callbacks
> baseline               5703 ns      307 ns       188017
> complete series        5023 ns      240 ns       188209
> ==> 680 ns (11.9%) reduction
> 
> AmpereOne              median       stdev
> baseline               2090 ns      157 ns
> complete series      1907.5 ns      140 ns
> ==> 182.5 ns (8.7%) reduction
> 
> The cumulative intermediate results on the Orion O6 attribute roughly half
> of the gain to each of the first two patches: avoiding the read reduced
> the median by 284 ns (5.0%), and avoiding the lock reduced it by another
> 336 ns (6.2%).
> Together they account for 620 ns of the 680 ns total reduction.
> 
> With the same arm64 configuration and GCC 11.4, caching the PCC predicate
> also reduces cppc_set_perf() from 1124 to 884 bytes.  The generated
> function has 60 fewer instructions, 17 fewer loads and 20 fewer branches.
> 
> This series is based on the CPPC fixes posted at (already queued):
> https://lore.kernel.org/lkml/20260722093825.1030594-1-christian.loehle@arm.com/
> 
> and the separately posted fix still under review:
> https://lore.kernel.org/lkml/20260724104042.1481804-1-christian.loehle@arm.com/
> 
> PS:
> There's a final optimization that I actually wanted to make but decided to
> split it out for now as it somewhat replicated Sumit's series:
> Skipping redundant perf ctrl writes in cpufreq-cppc if registers are non-PCC
> and !shared (because the values are unchanged, the
> !autonomous-common-case), but that requires the driver to have a more
> complex caching- and atomic-updating machinery in place, similar to
> hotplug. As opposed to this series the optimization would be for the
> microcontroller handling the CPPC requests, which may be shared across
> many CPUs and therefore redundant requests can increase the dvfs
> transition latency.
> That patch will follow once Sumit's is queued:
> https://lore.kernel.org/lkml/20260716153820.2007095-1-sumitg@nvidia.com/
> 
> [RESEND] for the new base-commit specifier below
> 
> Christian Loehle (3):
>   ACPI: CPPC: Avoid unnecessary reads for full-width writes
>   ACPI: CPPC: Avoid locking standalone full-width registers
>   ACPI: CPPC: Evaluate performance-control PCC use once
> 
>  drivers/acpi/cppc_acpi.c | 135 +++++++++++++++++++++++++++++++--------
>  include/acpi/cppc_acpi.h |   5 +-
>  2 files changed, 113 insertions(+), 27 deletions(-)
> 
> 
> base-commit: 075b74841bd0065a3bda3440873c747938e69b68

Nice, so that worked:
https://sashiko.dev/#/patchset/20260803210527.1285229-1-christian.loehle@arm.com
Seems no findings on $SUBJECT but the rest of the comments it has
look legit to me? I'll go take another look tomorrow:

1. Using per-CPU cpc_desc->rmw_lock for SYSTEM_MEMORY CPPC control register that
may be shared.
2. MASK_VAL_WRITE() truncation on 32bit architectures.
3. acpi_cppc_processor_exit() calls kfree(cpc_ptr) unconditionally (UAF with sysfs?)
4. cppc_set_reg_val_in_pcc() calls cpc_write() before down_write(&pcc_ss_data->pcc_lock)
5 & 6 are sanitization of values from FW.
Re: [RESEND][PATCH 0/3] ACPI: CPPC: Reduce .target() callback overhead
Posted by Christian Loehle 1 month, 3 weeks ago
On 8/3/26 23:28, Christian Loehle wrote:
> On 8/3/26 22:05, Christian Loehle wrote:
>> cppc-cpufreq reaches cppc_set_perf() from every target callback.  For
>> direct SystemMemory controls, that path currently does several steps
>> which are unnecessary once the immutable _CPC layout is known:
>>
>>   - a full-width write first reads the access unit and merges the value.
>>   - every write takes the descriptor's RMW lock, even when its access unit
>>     is not shared with another _CPC entry.
>>   - cppc_set_perf() evaluates the same three PCC predicates at each phase
>>     of the transaction.
>>
>> Remove those costs while retaining the existing conservative paths for
>> partial fields, overlapping or malformed access units, and PCC controls.
>>
>> The series was tested on Arm Power-Orion O6 and AmpereOne systems using
>> cppc-cpufreq and schedutil.  An rt-app task pinned to one CPU ran for
>> 500 us every 2 ms with uclamp.min=512, for 5000 periods per run.  schedutil
>> rate_limit_us was 1000.  cppc_cpufreq_fast_switch() latency was measured
>> for 100 ten-second runs.  Each sample is the mean callback latency within
>> one run, and the table reports the median and sample standard deviation
>> of those samples.
>>
>> Orion O6               median       stdev       callbacks
>> baseline               5703 ns      307 ns       188017
>> complete series        5023 ns      240 ns       188209
>> ==> 680 ns (11.9%) reduction
>>
>> AmpereOne              median       stdev
>> baseline               2090 ns      157 ns
>> complete series      1907.5 ns      140 ns
>> ==> 182.5 ns (8.7%) reduction
>>
>> The cumulative intermediate results on the Orion O6 attribute roughly half
>> of the gain to each of the first two patches: avoiding the read reduced
>> the median by 284 ns (5.0%), and avoiding the lock reduced it by another
>> 336 ns (6.2%).
>> Together they account for 620 ns of the 680 ns total reduction.
>>
>> With the same arm64 configuration and GCC 11.4, caching the PCC predicate
>> also reduces cppc_set_perf() from 1124 to 884 bytes.  The generated
>> function has 60 fewer instructions, 17 fewer loads and 20 fewer branches.
>>
>> This series is based on the CPPC fixes posted at (already queued):
>> https://lore.kernel.org/lkml/20260722093825.1030594-1-christian.loehle@arm.com/
>>
>> and the separately posted fix still under review:
>> https://lore.kernel.org/lkml/20260724104042.1481804-1-christian.loehle@arm.com/
>>
>> PS:
>> There's a final optimization that I actually wanted to make but decided to
>> split it out for now as it somewhat replicated Sumit's series:
>> Skipping redundant perf ctrl writes in cpufreq-cppc if registers are non-PCC
>> and !shared (because the values are unchanged, the
>> !autonomous-common-case), but that requires the driver to have a more
>> complex caching- and atomic-updating machinery in place, similar to
>> hotplug. As opposed to this series the optimization would be for the
>> microcontroller handling the CPPC requests, which may be shared across
>> many CPUs and therefore redundant requests can increase the dvfs
>> transition latency.
>> That patch will follow once Sumit's is queued:
>> https://lore.kernel.org/lkml/20260716153820.2007095-1-sumitg@nvidia.com/
>>
>> [RESEND] for the new base-commit specifier below
>>
>> Christian Loehle (3):
>>   ACPI: CPPC: Avoid unnecessary reads for full-width writes
>>   ACPI: CPPC: Avoid locking standalone full-width registers
>>   ACPI: CPPC: Evaluate performance-control PCC use once
>>
>>  drivers/acpi/cppc_acpi.c | 135 +++++++++++++++++++++++++++++++--------
>>  include/acpi/cppc_acpi.h |   5 +-
>>  2 files changed, 113 insertions(+), 27 deletions(-)
>>
>>
>> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> 
> Nice, so that worked:
> https://sashiko.dev/#/patchset/20260803210527.1285229-1-christian.loehle@arm.com
> Seems no findings on $SUBJECT but the rest of the comments it has
> look legit to me? I'll go take another look tomorrow:

So I have patches for these all and will post them in a minute.

> 
> 1. Using per-CPU cpc_desc->rmw_lock for SYSTEM_MEMORY CPPC control register that
> may be shared.

The solution here sucks unfortunately.
Obviously expanding the per-CPU rmw_lock to per-policy is reasonable, but unfortunately
that doesn't prevent something like:
Policy 0 Desired: address X, bits 7:0,  access size 32
Policy 1 Desired: address X, bits 15:8, access size 32
which would be a compliant GAS description and would require a global lock for rmw.

And of course even worse, now that Desired is write-only rmw isn't possible at all
anymore.
I'm leaning to just reject these edge-cases that hopefully don't actually exist
(and require _CPC rev4 to be full access-unit width)
Opinions?

> 2. MASK_VAL_WRITE() truncation on 32bit architectures.
> 3. acpi_cppc_processor_exit() calls kfree(cpc_ptr) unconditionally (UAF with sysfs?)
> 4. cppc_set_reg_val_in_pcc() calls cpc_write() before down_write(&pcc_ss_data->pcc_lock)
> 5 & 6 are sanitization of values from FW. 
>
Re: [RESEND][PATCH 0/3] ACPI: CPPC: Reduce .target() callback overhead
Posted by Rafael J. Wysocki (Intel) 1 month, 3 weeks ago
On Thu, Aug 6, 2026 at 12:05 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 8/3/26 23:28, Christian Loehle wrote:
> > On 8/3/26 22:05, Christian Loehle wrote:
> >> cppc-cpufreq reaches cppc_set_perf() from every target callback.  For
> >> direct SystemMemory controls, that path currently does several steps
> >> which are unnecessary once the immutable _CPC layout is known:
> >>
> >>   - a full-width write first reads the access unit and merges the value.
> >>   - every write takes the descriptor's RMW lock, even when its access unit
> >>     is not shared with another _CPC entry.
> >>   - cppc_set_perf() evaluates the same three PCC predicates at each phase
> >>     of the transaction.
> >>
> >> Remove those costs while retaining the existing conservative paths for
> >> partial fields, overlapping or malformed access units, and PCC controls.
> >>
> >> The series was tested on Arm Power-Orion O6 and AmpereOne systems using
> >> cppc-cpufreq and schedutil.  An rt-app task pinned to one CPU ran for
> >> 500 us every 2 ms with uclamp.min=512, for 5000 periods per run.  schedutil
> >> rate_limit_us was 1000.  cppc_cpufreq_fast_switch() latency was measured
> >> for 100 ten-second runs.  Each sample is the mean callback latency within
> >> one run, and the table reports the median and sample standard deviation
> >> of those samples.
> >>
> >> Orion O6               median       stdev       callbacks
> >> baseline               5703 ns      307 ns       188017
> >> complete series        5023 ns      240 ns       188209
> >> ==> 680 ns (11.9%) reduction
> >>
> >> AmpereOne              median       stdev
> >> baseline               2090 ns      157 ns
> >> complete series      1907.5 ns      140 ns
> >> ==> 182.5 ns (8.7%) reduction
> >>
> >> The cumulative intermediate results on the Orion O6 attribute roughly half
> >> of the gain to each of the first two patches: avoiding the read reduced
> >> the median by 284 ns (5.0%), and avoiding the lock reduced it by another
> >> 336 ns (6.2%).
> >> Together they account for 620 ns of the 680 ns total reduction.
> >>
> >> With the same arm64 configuration and GCC 11.4, caching the PCC predicate
> >> also reduces cppc_set_perf() from 1124 to 884 bytes.  The generated
> >> function has 60 fewer instructions, 17 fewer loads and 20 fewer branches.
> >>
> >> This series is based on the CPPC fixes posted at (already queued):
> >> https://lore.kernel.org/lkml/20260722093825.1030594-1-christian.loehle@arm.com/
> >>
> >> and the separately posted fix still under review:
> >> https://lore.kernel.org/lkml/20260724104042.1481804-1-christian.loehle@arm.com/
> >>
> >> PS:
> >> There's a final optimization that I actually wanted to make but decided to
> >> split it out for now as it somewhat replicated Sumit's series:
> >> Skipping redundant perf ctrl writes in cpufreq-cppc if registers are non-PCC
> >> and !shared (because the values are unchanged, the
> >> !autonomous-common-case), but that requires the driver to have a more
> >> complex caching- and atomic-updating machinery in place, similar to
> >> hotplug. As opposed to this series the optimization would be for the
> >> microcontroller handling the CPPC requests, which may be shared across
> >> many CPUs and therefore redundant requests can increase the dvfs
> >> transition latency.
> >> That patch will follow once Sumit's is queued:
> >> https://lore.kernel.org/lkml/20260716153820.2007095-1-sumitg@nvidia.com/
> >>
> >> [RESEND] for the new base-commit specifier below
> >>
> >> Christian Loehle (3):
> >>   ACPI: CPPC: Avoid unnecessary reads for full-width writes
> >>   ACPI: CPPC: Avoid locking standalone full-width registers
> >>   ACPI: CPPC: Evaluate performance-control PCC use once
> >>
> >>  drivers/acpi/cppc_acpi.c | 135 +++++++++++++++++++++++++++++++--------
> >>  include/acpi/cppc_acpi.h |   5 +-
> >>  2 files changed, 113 insertions(+), 27 deletions(-)
> >>
> >>
> >> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> >
> > Nice, so that worked:
> > https://sashiko.dev/#/patchset/20260803210527.1285229-1-christian.loehle@arm.com
> > Seems no findings on $SUBJECT but the rest of the comments it has
> > look legit to me? I'll go take another look tomorrow:
>
> So I have patches for these all and will post them in a minute.
>
> >
> > 1. Using per-CPU cpc_desc->rmw_lock for SYSTEM_MEMORY CPPC control register that
> > may be shared.
>
> The solution here sucks unfortunately.
> Obviously expanding the per-CPU rmw_lock to per-policy is reasonable, but unfortunately
> that doesn't prevent something like:
> Policy 0 Desired: address X, bits 7:0,  access size 32
> Policy 1 Desired: address X, bits 15:8, access size 32
> which would be a compliant GAS description and would require a global lock for rmw.
>
> And of course even worse, now that Desired is write-only rmw isn't possible at all
> anymore.
> I'm leaning to just reject these edge-cases that hopefully don't actually exist
> (and require _CPC rev4 to be full access-unit width)
> Opinions?

Fail initialization on insane platform setups I'd say.

There's no promise that Linux will support compliant setups that are
too much pain to deal with.
Re: [RESEND][PATCH 0/3] ACPI: CPPC: Reduce .target() callback overhead
Posted by Christian Loehle 1 month, 3 weeks ago
On 8/6/26 11:36, Rafael J. Wysocki (Intel) wrote:
> On Thu, Aug 6, 2026 at 12:05 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
>>
>> On 8/3/26 23:28, Christian Loehle wrote:
>>> On 8/3/26 22:05, Christian Loehle wrote:
>>>> cppc-cpufreq reaches cppc_set_perf() from every target callback.  For
>>>> direct SystemMemory controls, that path currently does several steps
>>>> which are unnecessary once the immutable _CPC layout is known:
>>>>
>>>>   - a full-width write first reads the access unit and merges the value.
>>>>   - every write takes the descriptor's RMW lock, even when its access unit
>>>>     is not shared with another _CPC entry.
>>>>   - cppc_set_perf() evaluates the same three PCC predicates at each phase
>>>>     of the transaction.
>>>>
>>>> Remove those costs while retaining the existing conservative paths for
>>>> partial fields, overlapping or malformed access units, and PCC controls.
>>>>
>>>> The series was tested on Arm Power-Orion O6 and AmpereOne systems using
>>>> cppc-cpufreq and schedutil.  An rt-app task pinned to one CPU ran for
>>>> 500 us every 2 ms with uclamp.min=512, for 5000 periods per run.  schedutil
>>>> rate_limit_us was 1000.  cppc_cpufreq_fast_switch() latency was measured
>>>> for 100 ten-second runs.  Each sample is the mean callback latency within
>>>> one run, and the table reports the median and sample standard deviation
>>>> of those samples.
>>>>
>>>> Orion O6               median       stdev       callbacks
>>>> baseline               5703 ns      307 ns       188017
>>>> complete series        5023 ns      240 ns       188209
>>>> ==> 680 ns (11.9%) reduction
>>>>
>>>> AmpereOne              median       stdev
>>>> baseline               2090 ns      157 ns
>>>> complete series      1907.5 ns      140 ns
>>>> ==> 182.5 ns (8.7%) reduction
>>>>
>>>> The cumulative intermediate results on the Orion O6 attribute roughly half
>>>> of the gain to each of the first two patches: avoiding the read reduced
>>>> the median by 284 ns (5.0%), and avoiding the lock reduced it by another
>>>> 336 ns (6.2%).
>>>> Together they account for 620 ns of the 680 ns total reduction.
>>>>
>>>> With the same arm64 configuration and GCC 11.4, caching the PCC predicate
>>>> also reduces cppc_set_perf() from 1124 to 884 bytes.  The generated
>>>> function has 60 fewer instructions, 17 fewer loads and 20 fewer branches.
>>>>
>>>> This series is based on the CPPC fixes posted at (already queued):
>>>> https://lore.kernel.org/lkml/20260722093825.1030594-1-christian.loehle@arm.com/
>>>>
>>>> and the separately posted fix still under review:
>>>> https://lore.kernel.org/lkml/20260724104042.1481804-1-christian.loehle@arm.com/
>>>>
>>>> PS:
>>>> There's a final optimization that I actually wanted to make but decided to
>>>> split it out for now as it somewhat replicated Sumit's series:
>>>> Skipping redundant perf ctrl writes in cpufreq-cppc if registers are non-PCC
>>>> and !shared (because the values are unchanged, the
>>>> !autonomous-common-case), but that requires the driver to have a more
>>>> complex caching- and atomic-updating machinery in place, similar to
>>>> hotplug. As opposed to this series the optimization would be for the
>>>> microcontroller handling the CPPC requests, which may be shared across
>>>> many CPUs and therefore redundant requests can increase the dvfs
>>>> transition latency.
>>>> That patch will follow once Sumit's is queued:
>>>> https://lore.kernel.org/lkml/20260716153820.2007095-1-sumitg@nvidia.com/
>>>>
>>>> [RESEND] for the new base-commit specifier below
>>>>
>>>> Christian Loehle (3):
>>>>   ACPI: CPPC: Avoid unnecessary reads for full-width writes
>>>>   ACPI: CPPC: Avoid locking standalone full-width registers
>>>>   ACPI: CPPC: Evaluate performance-control PCC use once
>>>>
>>>>  drivers/acpi/cppc_acpi.c | 135 +++++++++++++++++++++++++++++++--------
>>>>  include/acpi/cppc_acpi.h |   5 +-
>>>>  2 files changed, 113 insertions(+), 27 deletions(-)
>>>>
>>>>
>>>> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
>>>
>>> Nice, so that worked:
>>> https://sashiko.dev/#/patchset/20260803210527.1285229-1-christian.loehle@arm.com
>>> Seems no findings on $SUBJECT but the rest of the comments it has
>>> look legit to me? I'll go take another look tomorrow:
>>
>> So I have patches for these all and will post them in a minute.
>>
>>>
>>> 1. Using per-CPU cpc_desc->rmw_lock for SYSTEM_MEMORY CPPC control register that
>>> may be shared.
>>
>> The solution here sucks unfortunately.
>> Obviously expanding the per-CPU rmw_lock to per-policy is reasonable, but unfortunately
>> that doesn't prevent something like:
>> Policy 0 Desired: address X, bits 7:0,  access size 32
>> Policy 1 Desired: address X, bits 15:8, access size 32
>> which would be a compliant GAS description and would require a global lock for rmw.
>>
>> And of course even worse, now that Desired is write-only rmw isn't possible at all
>> anymore.
>> I'm leaning to just reject these edge-cases that hopefully don't actually exist
>> (and require _CPC rev4 to be full access-unit width)
>> Opinions?
> 
> Fail initialization on insane platform setups I'd say.
> 
> There's no promise that Linux will support compliant setups that are
> too much pain to deal with.

Ok done, I would delay posting until the your bleeding-edge hits linux-next, in the
hopes of being able to supply a valid base-commit for Sashiko.
Re: [RESEND][PATCH 0/3] ACPI: CPPC: Reduce .target() callback overhead
Posted by Rafael J. Wysocki (Intel) 1 month, 3 weeks ago
On Tue, Aug 4, 2026 at 12:28 AM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 8/3/26 22:05, Christian Loehle wrote:
> > cppc-cpufreq reaches cppc_set_perf() from every target callback.  For
> > direct SystemMemory controls, that path currently does several steps
> > which are unnecessary once the immutable _CPC layout is known:
> >
> >   - a full-width write first reads the access unit and merges the value.
> >   - every write takes the descriptor's RMW lock, even when its access unit
> >     is not shared with another _CPC entry.
> >   - cppc_set_perf() evaluates the same three PCC predicates at each phase
> >     of the transaction.
> >
> > Remove those costs while retaining the existing conservative paths for
> > partial fields, overlapping or malformed access units, and PCC controls.
> >
> > The series was tested on Arm Power-Orion O6 and AmpereOne systems using
> > cppc-cpufreq and schedutil.  An rt-app task pinned to one CPU ran for
> > 500 us every 2 ms with uclamp.min=512, for 5000 periods per run.  schedutil
> > rate_limit_us was 1000.  cppc_cpufreq_fast_switch() latency was measured
> > for 100 ten-second runs.  Each sample is the mean callback latency within
> > one run, and the table reports the median and sample standard deviation
> > of those samples.
> >
> > Orion O6               median       stdev       callbacks
> > baseline               5703 ns      307 ns       188017
> > complete series        5023 ns      240 ns       188209
> > ==> 680 ns (11.9%) reduction
> >
> > AmpereOne              median       stdev
> > baseline               2090 ns      157 ns
> > complete series      1907.5 ns      140 ns
> > ==> 182.5 ns (8.7%) reduction
> >
> > The cumulative intermediate results on the Orion O6 attribute roughly half
> > of the gain to each of the first two patches: avoiding the read reduced
> > the median by 284 ns (5.0%), and avoiding the lock reduced it by another
> > 336 ns (6.2%).
> > Together they account for 620 ns of the 680 ns total reduction.
> >
> > With the same arm64 configuration and GCC 11.4, caching the PCC predicate
> > also reduces cppc_set_perf() from 1124 to 884 bytes.  The generated
> > function has 60 fewer instructions, 17 fewer loads and 20 fewer branches.
> >
> > This series is based on the CPPC fixes posted at (already queued):
> > https://lore.kernel.org/lkml/20260722093825.1030594-1-christian.loehle@arm.com/
> >
> > and the separately posted fix still under review:
> > https://lore.kernel.org/lkml/20260724104042.1481804-1-christian.loehle@arm.com/
> >
> > PS:
> > There's a final optimization that I actually wanted to make but decided to
> > split it out for now as it somewhat replicated Sumit's series:
> > Skipping redundant perf ctrl writes in cpufreq-cppc if registers are non-PCC
> > and !shared (because the values are unchanged, the
> > !autonomous-common-case), but that requires the driver to have a more
> > complex caching- and atomic-updating machinery in place, similar to
> > hotplug. As opposed to this series the optimization would be for the
> > microcontroller handling the CPPC requests, which may be shared across
> > many CPUs and therefore redundant requests can increase the dvfs
> > transition latency.
> > That patch will follow once Sumit's is queued:
> > https://lore.kernel.org/lkml/20260716153820.2007095-1-sumitg@nvidia.com/
> >
> > [RESEND] for the new base-commit specifier below
> >
> > Christian Loehle (3):
> >   ACPI: CPPC: Avoid unnecessary reads for full-width writes
> >   ACPI: CPPC: Avoid locking standalone full-width registers
> >   ACPI: CPPC: Evaluate performance-control PCC use once
> >
> >  drivers/acpi/cppc_acpi.c | 135 +++++++++++++++++++++++++++++++--------
> >  include/acpi/cppc_acpi.h |   5 +-
> >  2 files changed, 113 insertions(+), 27 deletions(-)
> >
> >
> > base-commit: 075b74841bd0065a3bda3440873c747938e69b68
>
> Nice, so that worked:
> https://sashiko.dev/#/patchset/20260803210527.1285229-1-christian.loehle@arm.com
> Seems no findings on $SUBJECT

Yes, so applied as 7.3 material.

> but the rest of the comments it has look legit to me?

Yes, but please address those on top of the $subject series, thanks!

> I'll go take another look tomorrow:
>
> 1. Using per-CPU cpc_desc->rmw_lock for SYSTEM_MEMORY CPPC control register that
> may be shared.
> 2. MASK_VAL_WRITE() truncation on 32bit architectures.
> 3. acpi_cppc_processor_exit() calls kfree(cpc_ptr) unconditionally (UAF with sysfs?)
> 4. cppc_set_reg_val_in_pcc() calls cpc_write() before down_write(&pcc_ss_data->pcc_lock)
> 5 & 6 are sanitization of values from FW.

Sounds good to me.