Re: [PATCH 2/2] arm64: topology: read CPPC FFH feedback counters in one operation

Re: [PATCH 2/2] arm64: topology: read CPPC FFH feedback counters in one operation

[Sumit Gupta]

On 10/04/26 15:11, Pengjie Zhang wrote:
> External email: Use caution opening links or attachments
>
>
> arm64 implements CPPC FFH feedback-counter reads using AMU counters.
> Because those counters must be sampled on the target CPU, reading the
> delivered and reference counters separately widens the observation window
> between them.
>
> Implement the paired FFH feedback-counter read hook on arm64 and sample
> both AMU counters together before decoding the requested CPC register
> values.
>
> Also factor the FFH bitfield extraction logic into a helper and reuse
> it from the existing single-counter FFH read path.
>
> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
> ---
>   arch/arm64/kernel/topology.c | 75 ++++++++++++++++++++++++++++++++----
>   1 file changed, 67 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index b32f13358fbb..b90a767b2a1f 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -50,6 +50,16 @@ struct amu_cntr_sample {
>          unsigned long   last_scale_update;
>   };
>
> +struct amu_ffh_ctrs {
> +       u64 corecnt;
> +       u64 constcnt;
> +};
> +
> +enum cpc_ffh_ctr_id {
> +       CPC_FFH_CTR_CORE  = 0x0,
> +       CPC_FFH_CTR_CONST = 0x1,
> +};
> +
>   static DEFINE_PER_CPU_SHARED_ALIGNED(struct amu_cntr_sample, cpu_amu_samples);
>
>   void update_freq_counters_refs(void)
> @@ -397,7 +407,7 @@ static void cpu_read_constcnt(void *val)
>   }
>
>   static inline
> -int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
> +int counters_read_on_cpu(int cpu, smp_call_func_t func, void *val)
>   {
>          /*
>           * Abort call on counterless CPU.
> @@ -447,24 +457,73 @@ bool cpc_ffh_supported(void)
>          return true;
>   }
>
> +static void amu_read_core_const_ctrs(void *val)
> +{
> +       struct amu_ffh_ctrs *ctrs = val;
> +
> +       cpu_read_constcnt(&ctrs->constcnt);
> +       cpu_read_corecnt(&ctrs->corecnt);
> +}

Any reason to flip the order?
Harmless as they are read back to back, but better to add a comment
if it's intentional.

Thanks,
Sumit
....

Re: [PATCH 0/2] CPPC: reduce FFH feedback-counter sampling skew on arm64

[Sumit Gupta]

On 10/04/26 15:11, Pengjie Zhang wrote:
> External email: Use caution opening links or attachments
>
>
> The legacy CPPC feedback-counter path reads the delivered and reference
> performance counters separately.
>
> On arm64 systems using AMU-backed CPPC FFH counters, each FFH read is
> served through a cross-CPU counter read helper. Reading the counters
> separately therefore widens the sampling window between them and can
> skew the delivered/reference ratio used by cpuinfo_cur_freq. Under heavy
> load, the skew is observable as transient values that may exceed the
> platform maximum, as discussed in [1] and [2].
>
> This series adds a small generic hook for architectures that can obtain
> both FFH feedback counters in one operation, while preserving the
> existing per-register read path as the fallback.
>
> Patch 1 adds the generic CPPC hook and uses it from cppc_get_perf_ctrs().
> Patch 2 implements the hook on arm64 by sampling both AMU counters in a
> single operation on the target CPU.
>
> [1] https://lore.kernel.org/all/20231025093847.3740104-4-zengheng4@huawei.com/
> [2] https://lore.kernel.org/all/20231212072617.14756-1-lihuisong@huawei.com/
>
> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
>
> Pengjie Zhang (2):
>    ACPI: CPPC: add paired FFH feedback-counter read hook
>    arm64: topology: read CPPC FFH feedback counters in one operation
>
>   arch/arm64/kernel/topology.c | 75 ++++++++++++++++++++++++++++++++----
>   drivers/acpi/cppc_acpi.c     | 58 +++++++++++++++++++++++++---
>   include/acpi/cppc_acpi.h     |  7 ++++
>   3 files changed, 127 insertions(+), 13 deletions(-)
>
> --


Hi Pengjie,

Thanks for the patches.
The series looks good to me, and it fixes the cpuinfo_cur_freq
spikes on an NVIDIA Vera (FFH) platform.

Just one nit on patch 2 (counter read order), noted there.

Tested-by: Sumit Gupta <sumitg@nvidia.com>
Reviewed-by: Sumit Gupta <sumitg@nvidia.com>

Thanks,
Sumit

Re: [PATCH 2/2] arm64: topology: read CPPC FFH feedback counters in one operation

[Pengjie Zhang]

On 6/29/2026 11:27 PM, Sumit Gupta wrote:
>
> On 10/04/26 15:11, Pengjie Zhang wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> arm64 implements CPPC FFH feedback-counter reads using AMU counters.
>> Because those counters must be sampled on the target CPU, reading the
>> delivered and reference counters separately widens the observation 
>> window
>> between them.
>>
>> Implement the paired FFH feedback-counter read hook on arm64 and sample
>> both AMU counters together before decoding the requested CPC register
>> values.
>>
>> Also factor the FFH bitfield extraction logic into a helper and reuse
>> it from the existing single-counter FFH read path.
>>
>> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
>> ---
>>   arch/arm64/kernel/topology.c | 75 ++++++++++++++++++++++++++++++++----
>>   1 file changed, 67 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
>> index b32f13358fbb..b90a767b2a1f 100644
>> --- a/arch/arm64/kernel/topology.c
>> +++ b/arch/arm64/kernel/topology.c
>> @@ -50,6 +50,16 @@ struct amu_cntr_sample {
>>          unsigned long   last_scale_update;
>>   };
>>
>> +struct amu_ffh_ctrs {
>> +       u64 corecnt;
>> +       u64 constcnt;
>> +};
>> +
>> +enum cpc_ffh_ctr_id {
>> +       CPC_FFH_CTR_CORE  = 0x0,
>> +       CPC_FFH_CTR_CONST = 0x1,
>> +};
>> +
>>   static DEFINE_PER_CPU_SHARED_ALIGNED(struct amu_cntr_sample, 
>> cpu_amu_samples);
>>
>>   void update_freq_counters_refs(void)
>> @@ -397,7 +407,7 @@ static void cpu_read_constcnt(void *val)
>>   }
>>
>>   static inline
>> -int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
>> +int counters_read_on_cpu(int cpu, smp_call_func_t func, void *val)
>>   {
>>          /*
>>           * Abort call on counterless CPU.
>> @@ -447,24 +457,73 @@ bool cpc_ffh_supported(void)
>>          return true;
>>   }
>>
>> +static void amu_read_core_const_ctrs(void *val)
>> +{
>> +       struct amu_ffh_ctrs *ctrs = val;
>> +
>> +       cpu_read_constcnt(&ctrs->constcnt);
>> +       cpu_read_corecnt(&ctrs->corecnt);
>> +}
>
> Any reason to flip the order?
> Harmless as they are read back to back, but better to add a comment
> if it's intentional.
>
> Thanks,
> Sumit
> ....
>
Hi Sumit，

Thanks for taking the time to review and test.
The cpu_read_constcnt() function includes the conditional check
this_cpu_has_cap(ARM64_WORKAROUND_2457168), which incurs a latency of
6–12 nanoseconds on the our platform.
If cpu_read_corecnt() is called prior to cpu_read_constcnt(), it will
widen the sampling interval between the corecnt and constcnt counter
readings. To address this, we have adjusted the call order:
cpu_read_constcnt() is executed first, followed by cpu_read_corecnt().
I will add comments later.

Thanks,
   Pengjie


>
>