[PATCH 0/4] arm64: cppc: add FFH support using AMUs

[PATCH 0/4] arm64: cppc: add FFH support using AMUs

[Ionela Voinescu]

Hi guys,

This series adds support for CPPC's delivered and reference performance
counters through the FFH methods by using the AMU equivalent core and
constant cycle counters.

This support is added in patch 4/4, while the first 3 patches generalise
the existing AMU counter read and validation functionality to be reused
for this usecase.

The specification that drove this implementation can be found at [1],
chapter 3.2.

The code was tested on a Armv8-A Base Platform FVP: Architecture Envelope
Model [2] with the following _CPC entry for all CPUs:

Name(_CPC, Package()
{
  23, // NumEntries
  3, // Revision
  100, // Highest Performance - Fixed 100MHz
  100, // Nominal Performance - Fixed 100MHz
  1, // Lowest Nonlinear Performance
  1, // Lowest Performance
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Guaranteed Performance Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Desired Perf Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Minimum Performance Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Maximum Performance Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Performance Red. Tolerance Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Time Window Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Counter Wraparound Time
  ResourceTemplate(){Register(FFixedHW, 0x40, 0, 1, 0x4)}, // Reference Performance Counter Register
  ResourceTemplate(){Register(FFixedHW, 0x40, 0, 0, 0x4)}, // Delivered Performance Counter Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Performance Ltd Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // CPPC Enable Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Autonomous Selection Enable
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Autonomous Activity Window Register
  ResourceTemplate(){Register(SystemMemory, 0, 0, 0, 0)}, // Energy Performance Preference Register
  100, // Reference Performance - Fixed 100MHz
  1, // Lowest Frequency
  100, // Nominal Frequency - Fixed 100MHz
})

The following configuration is necessary for Activity Monitors use:

   cluster0.has_arm_v8-4=1
   cluster1.has_arm_v8-4=1
   cluster0.has_amu=1
   cluster1.has_amu=1

To be noted:
 - The FVP has fixed core and constant frequency of 100MHz
 - The kernel I used for testing had some extra debug information as you
   can see below:

$ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_cur_freq
[   23.850590] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (2402*100)/2402=100.
100000
[   23.851246] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (2402*100)/2402=100.
100000
[   23.851826] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (2873*100)/2872=100.
100000
[   23.852326] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (2402*100)/2402=100.
100000
[   23.852747] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (2309*100)/2309=100.
100000
[   23.853228] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (1333*100)/1333=100.
100000
[   23.854097] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (18762*100)/20003=93.
93000
[   23.854890] CPPC: Delivered perf (core_cnt*ref_perf/const_cnt): (20047*100)/20051=99.
99000

[1] https://documentation-service.arm.com/static/5f106ad60daa596235e80081
[2] https://developer.arm.com/tools-and-software/simulation-models/fixed-virtual-platforms

Thanks,
Ionela.

Ionela Voinescu (4):
  arm64: cpufeature: restructure AMU feedback function
  arm64: wrap and generalise counter read functions
  arm64: split counter validation function
  arm64: implement CPPC FFH support using AMUs

 arch/arm64/include/asm/cpufeature.h |   6 +-
 arch/arm64/kernel/cpufeature.c      |  15 ++-
 arch/arm64/kernel/topology.c        | 147 ++++++++++++++++++++++------
 3 files changed, 129 insertions(+), 39 deletions(-)


base-commit: 3a00d3dfd4b68b208ecd5405e676d06c8ad6bb63
-- 
2.17.1

[PATCH 1/4] arm64: cpufeature: restructure AMU feedback function

[Ionela Voinescu]

The current cpu_has_amu_feat() function only returns whether a single
provided CPU has support for Activity Monitors (AMUs).

Replace that function by cpus_with_amu_counters() that returns a pointer
to a cpumask with all CPUs that support AMUs. This way the user has more
freedom in regards to either checking many CPUs at once or selecting any
supporting CPU, through the use of cpumask operations.

Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/cpufeature.h |  6 ++----
 arch/arm64/kernel/cpufeature.c      | 11 ++++++++---
 arch/arm64/kernel/topology.c        |  3 ++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 89b4f0142c28..387b28ab270c 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -758,10 +758,8 @@ static inline bool cpu_has_hw_af(void)
 						ID_AA64MMFR1_HADBS_SHIFT);
 }
 
-#ifdef CONFIG_ARM64_AMU_EXTN
-/* Check whether the cpu supports the Activity Monitors Unit (AMU) */
-extern bool cpu_has_amu_feat(int cpu);
-#endif
+/* Return cpumask with CPUs that support the Activity Monitors Unit (AMU) */
+extern const struct cpumask *cpus_with_amu_counters(void);
 
 static inline unsigned int get_vmid_bits(u64 mmfr1)
 {
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 6424584be01e..15a376689b2f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1516,9 +1516,9 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
  */
 static struct cpumask amu_cpus __read_mostly;
 
-bool cpu_has_amu_feat(int cpu)
+const struct cpumask *cpus_with_amu_counters(void)
 {
-	return cpumask_test_cpu(cpu, &amu_cpus);
+	return (const struct cpumask *)&amu_cpus;
 }
 
 /* Initialize the use of AMU counters for frequency invariance */
@@ -1552,7 +1552,12 @@ static bool has_amu(const struct arm64_cpu_capabilities *cap,
 
 	return true;
 }
-#endif
+#else
+const struct cpumask *cpus_with_amu_counters(void)
+{
+	return NULL;
+}
+#endif /* CONFIG_ARM64_AMU_EXTN */
 
 #ifdef CONFIG_ARM64_VHE
 static bool runs_at_el2(const struct arm64_cpu_capabilities *entry, int __unused)
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 0801a0f3c156..2ef440938282 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -142,9 +142,10 @@ void init_cpu_freq_invariance_counters(void)
 
 static int validate_cpu_freq_invariance_counters(int cpu)
 {
+	const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters();
 	u64 max_freq_hz, ratio;
 
-	if (!cpu_has_amu_feat(cpu)) {
+	if (!cnt_cpu_mask || !cpumask_test_cpu(cpu, cnt_cpu_mask)) {
 		pr_debug("CPU%d: counters are not supported.\n", cpu);
 		return -EINVAL;
 	}
-- 
2.17.1

[PATCH 2/4] arm64: wrap and generalise counter read functions

[Ionela Voinescu]

In preparation for other uses of Activity Monitors (AMU) cycle counters,
place counter read functionality in generic functions that can reused:
read_corecnt() and read_constcnt().

As a result, implement update_freq_counters_refs() to replace
init_cpu_freq_invariance_counters() and both initialise and update
the per-cpu reference variables.

Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/cpufeature.c |  4 ++--
 arch/arm64/kernel/topology.c   | 36 ++++++++++++++++++++++------------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 15a376689b2f..40d7a4c52558 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1522,7 +1522,7 @@ const struct cpumask *cpus_with_amu_counters(void)
 }
 
 /* Initialize the use of AMU counters for frequency invariance */
-extern void init_cpu_freq_invariance_counters(void);
+extern void update_freq_counters_refs(void);
 
 static void cpu_amu_enable(struct arm64_cpu_capabilities const *cap)
 {
@@ -1530,7 +1530,7 @@ static void cpu_amu_enable(struct arm64_cpu_capabilities const *cap)
 		pr_info("detected CPU%d: Activity Monitors Unit (AMU)\n",
 			smp_processor_id());
 		cpumask_set_cpu(smp_processor_id(), &amu_cpus);
-		init_cpu_freq_invariance_counters();
+		update_freq_counters_refs();
 	}
 }
 
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 2ef440938282..1241087e92c8 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -121,23 +121,33 @@ int __init parse_acpi_topology(void)
 }
 #endif
 
-#ifdef CONFIG_ARM64_AMU_EXTN
+#define COUNTER_READ_STORE(NAME, VAL) \
+static inline u64 read_##NAME(void) \
+{ \
+	return VAL; \
+} \
+static inline void store_##NAME(void *val) \
+{ \
+	*(u64 *)val = read_##NAME(); \
+}
 
-#undef pr_fmt
-#define pr_fmt(fmt) "AMU: " fmt
+#ifdef CONFIG_ARM64_AMU_EXTN
+COUNTER_READ_STORE(corecnt, read_sysreg_s(SYS_AMEVCNTR0_CORE_EL0));
+COUNTER_READ_STORE(constcnt, read_sysreg_s(SYS_AMEVCNTR0_CONST_EL0));
+#else
+COUNTER_READ_STORE(corecnt, 0);
+COUNTER_READ_STORE(constcnt, 0);
+#endif
 
 static DEFINE_PER_CPU_READ_MOSTLY(unsigned long, arch_max_freq_scale);
 static DEFINE_PER_CPU(u64, arch_const_cycles_prev);
 static DEFINE_PER_CPU(u64, arch_core_cycles_prev);
 static cpumask_var_t amu_fie_cpus;
 
-/* Initialize counter reference per-cpu variables for the current CPU */
-void init_cpu_freq_invariance_counters(void)
+void update_freq_counters_refs(void)
 {
-	this_cpu_write(arch_core_cycles_prev,
-		       read_sysreg_s(SYS_AMEVCNTR0_CORE_EL0));
-	this_cpu_write(arch_const_cycles_prev,
-		       read_sysreg_s(SYS_AMEVCNTR0_CONST_EL0));
+	this_cpu_write(arch_core_cycles_prev, read_corecnt());
+	this_cpu_write(arch_const_cycles_prev, read_constcnt());
 }
 
 static int validate_cpu_freq_invariance_counters(int cpu)
@@ -272,11 +282,14 @@ void topology_scale_freq_tick(void)
 	if (!cpumask_test_cpu(cpu, amu_fie_cpus))
 		return;
 
-	const_cnt = read_sysreg_s(SYS_AMEVCNTR0_CONST_EL0);
-	core_cnt = read_sysreg_s(SYS_AMEVCNTR0_CORE_EL0);
 	prev_const_cnt = this_cpu_read(arch_const_cycles_prev);
 	prev_core_cnt = this_cpu_read(arch_core_cycles_prev);
 
+	update_freq_counters_refs();
+
+	const_cnt = this_cpu_read(arch_const_cycles_prev);
+	core_cnt = this_cpu_read(arch_core_cycles_prev);
+
 	if (unlikely(core_cnt <= prev_core_cnt ||
 		     const_cnt <= prev_const_cnt))
 		goto store_and_exit;
@@ -301,4 +314,3 @@ void topology_scale_freq_tick(void)
 	this_cpu_write(arch_core_cycles_prev, core_cnt);
 	this_cpu_write(arch_const_cycles_prev, const_cnt);
 }
-#endif /* CONFIG_ARM64_AMU_EXTN */
-- 
2.17.1

[PATCH 3/4] arm64: split counter validation function

[Ionela Voinescu]

In order for the counter validation function to be reused, split
validate_cpu_freq_invariance_counters() into:
 - freq_counters_valid(cpu) - check cpu for valid cycle counters
 - freq_inv_set_max_ratio(int cpu, u64 max_rate, u64 ref_rate) -
   generic function that sets the normalization ratio used by
   topology_scale_freq_tick()

Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/topology.c | 46 +++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 1241087e92c8..edc44b46e34f 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -150,46 +150,51 @@ void update_freq_counters_refs(void)
 	this_cpu_write(arch_const_cycles_prev, read_constcnt());
 }
 
-static int validate_cpu_freq_invariance_counters(int cpu)
+static inline bool freq_counters_valid(int cpu)
 {
 	const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters();
-	u64 max_freq_hz, ratio;
 
 	if (!cnt_cpu_mask || !cpumask_test_cpu(cpu, cnt_cpu_mask)) {
 		pr_debug("CPU%d: counters are not supported.\n", cpu);
-		return -EINVAL;
+		return false;
 	}
 
 	if (unlikely(!per_cpu(arch_const_cycles_prev, cpu) ||
 		     !per_cpu(arch_core_cycles_prev, cpu))) {
 		pr_debug("CPU%d: cycle counters are not enabled.\n", cpu);
-		return -EINVAL;
+		return false;
 	}
 
-	/* Convert maximum frequency from KHz to Hz and validate */
-	max_freq_hz = cpufreq_get_hw_max_freq(cpu) * 1000;
-	if (unlikely(!max_freq_hz)) {
-		pr_debug("CPU%d: invalid maximum frequency.\n", cpu);
+	return true;
+}
+
+static int freq_inv_set_max_ratio(int cpu, u64 max_rate, u64 ref_rate)
+{
+	u64 ratio;
+
+	if (unlikely(!max_rate || !ref_rate)) {
+		pr_debug("CPU%d: invalid maximum or reference frequency.\n",
+			 cpu);
 		return -EINVAL;
 	}
 
 	/*
 	 * Pre-compute the fixed ratio between the frequency of the constant
-	 * counter and the maximum frequency of the CPU.
+	 * reference counter and the maximum frequency of the CPU.
 	 *
-	 *			      const_freq
-	 * arch_max_freq_scale =   ---------------- * SCHED_CAPACITY_SCALE²
-	 *			   cpuinfo_max_freq
+	 *			    ref_rate
+	 * arch_max_freq_scale =   ---------- * SCHED_CAPACITY_SCALE²
+	 *			    max_rate
 	 *
 	 * We use a factor of 2 * SCHED_CAPACITY_SHIFT -> SCHED_CAPACITY_SCALE²
 	 * in order to ensure a good resolution for arch_max_freq_scale for
-	 * very low arch timer frequencies (down to the KHz range which should
+	 * very low reference frequencies (down to the KHz range which should
 	 * be unlikely).
 	 */
-	ratio = (u64)arch_timer_get_rate() << (2 * SCHED_CAPACITY_SHIFT);
-	ratio = div64_u64(ratio, max_freq_hz);
+	ratio = ref_rate << (2 * SCHED_CAPACITY_SHIFT);
+	ratio = div64_u64(ratio, max_rate);
 	if (!ratio) {
-		WARN_ONCE(1, "System timer frequency too low.\n");
+		WARN_ONCE(1, "Reference frequency too low.\n");
 		return -EINVAL;
 	}
 
@@ -224,6 +229,7 @@ static int __init init_amu_fie(void)
 {
 	cpumask_var_t valid_cpus;
 	bool have_policy = false;
+	u64 max_rate_hz;
 	int ret = 0;
 	int cpu;
 
@@ -236,8 +242,14 @@ static int __init init_amu_fie(void)
 	}
 
 	for_each_present_cpu(cpu) {
-		if (validate_cpu_freq_invariance_counters(cpu))
+		if (!freq_counters_valid(cpu))
 			continue;
+
+		max_rate_hz = cpufreq_get_hw_max_freq(cpu) * 1000;
+		if (freq_inv_set_max_ratio(cpu, max_rate_hz,
+					   arch_timer_get_rate()))
+			continue;
+
 		cpumask_set_cpu(cpu, valid_cpus);
 		have_policy |= enable_policy_freq_counters(cpu, valid_cpus);
 	}
-- 
2.17.1

[PATCH 4/4] arm64: implement CPPC FFH support using AMUs

[Ionela Voinescu]

If Activity Monitors (AMUs) are present, two of the counters can be used
to implement support for CPPC's (Collaborative Processor Performance
Control) delivered and reference performance monitoring functionality
using FFH (Functional Fixed Hardware).

Given that counters for a certain CPU can only be read from that CPU,
while FFH operations can be called from any CPU for any of the CPUs, use
smp_call_function_single() to provide the requested values.

Therefore, depending on the register addresses, the following values
are returned:
 - 0x0 (DeliveredPerformanceCounterRegister): AMU core counter
 - 0x1 (ReferencePerformanceCounterRegister): AMU constant counter

The use of Activity Monitors is hidden behind the generic
{read,store}_{corecnt,constcnt}() functions.

Read functionality for these two registers represents the only current
FFH support for CPPC. Read operations for other register values or write
operation for all registers are unsupported. Therefore, keep CPPC's FFH
unsupported if no CPUs have valid AMU frequency counters.

Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/topology.c | 62 ++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index edc44b46e34f..cb0372de9aa9 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -326,3 +326,65 @@ void topology_scale_freq_tick(void)
 	this_cpu_write(arch_core_cycles_prev, core_cnt);
 	this_cpu_write(arch_const_cycles_prev, const_cnt);
 }
+
+#ifdef CONFIG_ACPI_CPPC_LIB
+#include <acpi/cppc_acpi.h>
+
+static inline
+int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
+{
+	const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters();
+
+	if (!cnt_cpu_mask || !cpumask_test_cpu(cpu, cnt_cpu_mask))
+		return -EOPNOTSUPP;
+
+	smp_call_function_single(cpu, func, val, 1);
+
+	return 0;
+}
+
+/*
+ * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
+ * below.
+ */
+bool cpc_ffh_supported(void)
+{
+	const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters();
+	int cpu = nr_cpu_ids;
+
+	if (cnt_cpu_mask)
+		cpu = cpumask_any_and(cnt_cpu_mask, cpu_present_mask);
+
+	if ((cpu >= nr_cpu_ids) || !freq_counters_valid(cpu))
+		return false;
+
+	return true;
+}
+
+int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch ((u64)reg->address) {
+	case 0x0:
+		ret = counters_read_on_cpu(cpu, store_corecnt, val);
+		break;
+	case 0x1:
+		ret = counters_read_on_cpu(cpu, store_constcnt, val);
+		break;
+	}
+
+	if (!ret) {
+		*val &= GENMASK_ULL(reg->bit_offset + reg->bit_width - 1,
+				    reg->bit_offset);
+		*val >>= reg->bit_offset;
+	}
+
+	return ret;
+}
+
+int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_ACPI_CPPC_LIB */
-- 
2.17.1

Re: [PATCH 4/4] arm64: implement CPPC FFH support using AMUs

[Catalin Marinas]

On Wed, Aug 26, 2020 at 02:03:09PM +0100, Ionela Voinescu wrote:
> +/*
> + * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
> + * below.
> + */
> +bool cpc_ffh_supported(void)
> +{
> +	const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters();
> +	int cpu = nr_cpu_ids;
> +
> +	if (cnt_cpu_mask)
> +		cpu = cpumask_any_and(cnt_cpu_mask, cpu_present_mask);
> +
> +	if ((cpu >= nr_cpu_ids) || !freq_counters_valid(cpu))
> +		return false;
> +
> +	return true;
> +}

IIUC, the only need for the cpumask is this function, the others would
have worked just fine with the existing cpu_has_amu_feat(). So you have
a lot more !cnt_cpu_mask checks now.

I wonder whether instead you could add a new function near
cpu_has_amu_feat(), something like get_cpu_with_amu_feat() and do the
cpumask_any_and() in there.

-- 
Catalin

Re: [PATCH 4/4] arm64: implement CPPC FFH support using AMUs

[Ionela Voinescu]

Hi Catalin,

Sorry for the delayed reply. I took advantage of a last chance for a
holiday before the weather gets bad :).

On Friday 11 Sep 2020 at 15:40:32 (+0100), Catalin Marinas wrote:
> On Wed, Aug 26, 2020 at 02:03:09PM +0100, Ionela Voinescu wrote:
> > +/*
> > + * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
> > + * below.
> > + */
> > +bool cpc_ffh_supported(void)
> > +{
> > +	const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters();
> > +	int cpu = nr_cpu_ids;
> > +
> > +	if (cnt_cpu_mask)
> > +		cpu = cpumask_any_and(cnt_cpu_mask, cpu_present_mask);
> > +
> > +	if ((cpu >= nr_cpu_ids) || !freq_counters_valid(cpu))
> > +		return false;
> > +
> > +	return true;
> > +}
> 
> IIUC, the only need for the cpumask is this function, the others would
> have worked just fine with the existing cpu_has_amu_feat(). So you have
> a lot more !cnt_cpu_mask checks now.
> 
> I wonder whether instead you could add a new function near
> cpu_has_amu_feat(), something like get_cpu_with_amu_feat() and do the
> cpumask_any_and() in there.
> 

Yes, it does look ugly. I went for this because I wanted to avoid adding
new functions to the cpu feature code with new AMU usecase additions,
functions that might just end up doing cpumask operations, and nothing
more. This way there is a single function that basically extracts all the
information that the feature code is able to provide on AMU support and
the user of the counters can then manipulate the mask as it sees fit.

Basically I was thinking that with a potential new usecase we might have
to add yet another function and I did not like that prospect.

From performance point of view, the !cnt_cpu_mask checks wouldn't affect
that much: cpc_ffh_supported() is only called during CPPC probe and
freq_counters_valid() is only called at init.
counters_read_on_cpu() will be called more often (cpufreq .get
function) but not as much as to bring significant overhead.

To make this nicer I can do the following:
 - make the !cnt_cpu_mask unlikely due to CONFIG_ARM64_AMU_EXTN being
   enabled by default.
 - wrap all the cpus_with_amu_counters() uses under:

static inline int amu_cpus_any_and(const struct cpumask *cpus)
{
	const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters();
	int cpu = nr_cpu_ids;

	if (likely(cnt_cpu_mask))
		cpu = cpumask_any_and(cnt_cpu_mask, cpus);

	return cpu;
}

This is to be called as:
 - "if (!freq_counters_valid(amu_cpus_any_and(cpu_present_mask)))"
   in cpc_ffh_supported();
 - "if (amu_cpus_any_and(cpumask_of(cpu)) == cpu)"
   in the other two.

It won't eliminate the useless checks but it will make the code a bit
nicer.

If you don't think it's worth it, I'll go with your suggestion.

Thank you for the review,
Ionela.


> -- 
> Catalin