Re: [PATCH 3/3] arm64: topology: Reduce the skew for AMU feedback counter sampling

[Christian Loehle <christian.loehle@arm.com>]

On 11/11/26 18:42, Beata Michalska wrote:
> On Wed, Nov 11, 2026 at 02:08:11PM +0000, Christian Loehle wrote:
>> On 11/11/26 12:39, Beata Michalska wrote:
>>> With the FFH support for CPPC, the AMU feedback counters, core and const
>>> counter, are being sampled to measure CPU performance. Checking for errata
>>> (which can affect both counters) between the reads delays the second
>>> sample and can skew the result.
>>>
>>> Take both samples in succession, then discard values from affected
>>> counters. This narrows the sampling interval used for CPPC feedback.
>>>
>>> Signed-off-by: Beata Michalska <beata.michalska@arm.com>
>>> ---
>>>  arch/arm64/kernel/topology.c | 76 +++++++++++++++++++++++-------------
>>>  1 file changed, 49 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
>>> index 66ac67bd025e..c968fa5b0c88 100644
>>> --- a/arch/arm64/kernel/topology.c
>>> +++ b/arch/arm64/kernel/topology.c
>>> @@ -381,29 +381,45 @@ enum cpc_ffh_ctr_id {
>>>  	CPC_FFH_CTR_CONST = 0x1,
>>>  };
>>>  
>>> -static void cpu_read_corecnt(void *val)
>>> +static inline void cpu_read_corecnt(void *val)
>>>  {
>>> -	/*
>>> -	 * A value of 0 can be returned if the current CPU does not support AMUs
>>> -	 * or if the counter is disabled for this CPU. A return value of 0 at
>>> -	 * counter read is properly handled as an error case by the users of the
>>> -	 * counter.
>>> -	 */
>>> -	*(u64 *)val = this_cpu_has_cap(ARM64_WORKAROUND_BROKEN_AMU_CORECNT) ?
>>> -		0UL : read_corecnt();
>>> +	*(u64 *)val = read_corecnt();
>>>  }
>>>  
>>> -static void cpu_read_constcnt(void *val)
>>> +static void cpu_try_read_corecnt(void *val)
>>>  {
>>> -	/*
>>> -	 * Return 0 if the current CPU is affected by a HW erratum.
>>> -	 * A value of 0 is also returned if the current CPU does not
>>> -	 * support AMUs or if the counter is disabled. A return
>>> -	 * value of 0 at counter read is properly handled as an error
>>> -	 * case by the users of the counter.
>>
>> Since you're moving this comment around, AFAICT it's not actually true?
>> cpu_has_amu_feat() is checked (which will return -EOPNOTSUPP) before
>> landing here, right?
>>
> Not entirely, no.
> `cpu_has_amu_feat()` verifies the support for AMUs, the extension itself
> (ID_AA64PFR0_EL1). Workarounds are applied on top of it.

Right, I was only referring to:
"A value of 0 is also returned if the current CPU does not support AMUs[...]"

> [snip]