Re: [PATCH 2/3] cpufreq: Use hardware feedback for cpuinfo_avg_freq

[Beata Michalska <beata.michalska@arm.com>]

Hello Chuyi,

On Wed, Sep 16, 2026 at 10:26:22PM +0800, Chuyi Zhou wrote:
> cpuinfo_avg_freq is documented as a frequency derived from hardware
> feedback, but it uses arch_freq_get_on_cpu(), which supplies a fallback
> on x86 when the cached sample is unavailable. Userspace cannot distinguish
> that value from a measurement. With amd-pstate-epp, for example, the
> fallback is policy->min even under the performance policy. A fallback
> based on the requested frequency also does not establish the frequency
> at which the hardware actually ran.
> 
> Use arch_freq_get_avg() for cpuinfo_avg_freq reads and support detection.
> On x86, unavailable samples then return -EAGAIN instead of a fallback
> frequency, and the attribute is omitted when APERF/MPERF is unsupported.
> 
> Provide a weak default returning -EOPNOTSUPP and reuse the existing ARM64
> AMU implementation. Preserve the behavior of ARM64, scaling_cur_freq
> and /proc/cpuinfo.
> 
> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
> ---
>  Documentation/admin-guide/pm/cpufreq.rst | 11 +++++++++--
>  arch/arm64/kernel/topology.c             |  7 ++++++-
>  drivers/cpufreq/cpufreq.c                | 19 +++++++++++++++++--
>  3 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/admin-guide/pm/cpufreq.rst b/Documentation/admin-guide/pm/cpufreq.rst
> index 34baf20cc202..3057829b01bd 100644
> --- a/Documentation/admin-guide/pm/cpufreq.rst
> +++ b/Documentation/admin-guide/pm/cpufreq.rst
> @@ -255,12 +255,19 @@ are the following:
>  
>          This is expected to be based on the frequency the hardware actually runs
>          at and, as such, might require specialised hardware support (such as AMU
> -        extension on ARM). If one cannot be determined, this attribute should
> -        not be present.
> +        extension on ARM or APERF/MPERF on x86). This attribute is not present
> +        when hardware feedback is unsupported.
>  
>          Note that failed attempt to retrieve current frequency for a given
>          CPU(s) will result in an appropriate error, i.e.: EAGAIN for CPU that
>          remains idle (raised on ARM).
> +        The attribute remains present during temporary sampling gaps.
That's bit ambigous - what are 'temporary sampling gaps' ?
Sampling did not take place, samples were outdated or somewhat invalid ?
Also, isn't that implied by the sentence above.
> +
> +        On x86, reads use cached APERF/MPERF samples without waking the target
Is there a case when the target is woken up for this particular attribute
readings ?
> +        CPU to collect new samples. An expired sample or a zero MPERF delta
> +        results in ``EAGAIN`` instead of a fallback to a policy or reference
> +        frequency. A CPU that has just entered idle can still have a usable
> +        sample, while a busy CPU excluded from periodic sampling can lack one.
>  
>  ``cpuinfo_max_freq``
>  	Maximum possible operating frequency the CPUs belonging to this policy
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index d28438f8b83f..939d3e4ce5d2 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -181,7 +181,7 @@ void arch_cpu_idle_enter(void)
>  
>  #define AMU_SAMPLE_EXP_MS	20
>  
> -int arch_freq_get_on_cpu(int cpu)
> +int arch_freq_get_avg(int cpu)
>  {
>  	struct amu_cntr_sample *amu_sample;
>  	unsigned int start_cpu = cpu;
> @@ -250,6 +250,11 @@ int arch_freq_get_on_cpu(int cpu)
>  	return freq;
>  }
>  
> +int arch_freq_get_on_cpu(int cpu)
> +{
> +	return arch_freq_get_avg(cpu);
> +}
> +
I do not think this is needed.

---
BR
Beata
>  static void amu_fie_setup(const struct cpumask *cpus)
>  {
>  	int cpu;
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 0d0df986fa3d..21bec6b7f538 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -705,9 +705,24 @@ __weak int arch_freq_get_on_cpu(int cpu)
>  	return -EOPNOTSUPP;
>  }
>  
> +/**
> + * arch_freq_get_avg() - Get an average frequency from hardware feedback
> + * @cpu: CPU to read.
> + *
> + * Provide cpuinfo_avg_freq with an average operating frequency derived
> + * from recent hardware feedback for @cpu or its frequency domain.
> + *
> + * Return: Frequency in kHz, -EOPNOTSUPP if feedback is unsupported for the
> + * CPU or policy under the current configuration.
> + */
> +__weak int arch_freq_get_avg(int cpu)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static inline bool cpufreq_avg_freq_supported(struct cpufreq_policy *policy)
>  {
> -	return arch_freq_get_on_cpu(policy->cpu) != -EOPNOTSUPP;
> +	return arch_freq_get_avg(policy->cpu) != -EOPNOTSUPP;
>  }
>  
>  static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
> @@ -769,7 +784,7 @@ static ssize_t show_cpuinfo_cur_freq(struct cpufreq_policy *policy,
>  static ssize_t show_cpuinfo_avg_freq(struct cpufreq_policy *policy,
>  				     char *buf)
>  {
> -	int avg_freq = arch_freq_get_on_cpu(policy->cpu);
> +	int avg_freq = arch_freq_get_avg(policy->cpu);
>  
>  	if (avg_freq > 0)
>  		return sysfs_emit(buf, "%u\n", avg_freq);
> -- 
> 2.20.1