[PATCH 3/8] perf: pmuv3: Add common defines for the PMU version

Zaid Al-Bassam posted 8 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH 3/8] perf: pmuv3: Add common defines for the PMU version
Posted by Zaid Al-Bassam 2 years, 7 months ago
The current PMU version defines are available for arm64 only,
As we want to add PMUv3 support to arm (32-bit), this patch makes
these defines available for both arm/arm64 by defining them in
the common arm_pmuv3.h header.

Signed-off-by: Zaid Al-Bassam <zalbassam@google.com>
---
 drivers/perf/arm_pmuv3.c       | 8 ++++----
 include/linux/perf/arm_pmuv3.h | 6 ++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 94e4098b662d..505f0758260c 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -392,7 +392,7 @@ static const struct attribute_group armv8_pmuv3_caps_attr_group = {
  */
 static bool armv8pmu_has_long_event(struct arm_pmu *cpu_pmu)
 {
-	return (cpu_pmu->pmuver >= ID_AA64DFR0_EL1_PMUVer_V3P5);
+	return (cpu_pmu->pmuver >= ARMV8_PMU_DFR_VER_V3P5);
 }
 
 static inline bool armv8pmu_event_has_user_read(struct perf_event *event)
@@ -1082,8 +1082,8 @@ static void __armv8pmu_probe_pmu(void *info)
 	int pmuver;
 
 	pmuver = read_pmuver();
-	if (pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF ||
-	    pmuver == ID_AA64DFR0_EL1_PMUVer_NI)
+	if (pmuver == ARMV8_PMU_DFR_VER_IMP_DEF ||
+	    pmuver == ARMV8_PMU_DFR_VER_NI)
 		return;
 
 	cpu_pmu->pmuver = pmuver;
@@ -1109,7 +1109,7 @@ static void __armv8pmu_probe_pmu(void *info)
 			     pmceid, ARMV8_PMUV3_MAX_COMMON_EVENTS);
 
 	/* store PMMIR register for sysfs */
-	if (pmuver >= ID_AA64DFR0_EL1_PMUVer_V3P4 && (pmceid_raw[1] & BIT(31)))
+	if (pmuver >= ARMV8_PMU_DFR_VER_V3P4 && (pmceid_raw[1] & BIT(31)))
 		cpu_pmu->reg_pmmir = read_pmmir();
 	else
 		cpu_pmu->reg_pmmir = 0;
diff --git a/include/linux/perf/arm_pmuv3.h b/include/linux/perf/arm_pmuv3.h
index 5bc9cd6826ea..18b29fde27fa 100644
--- a/include/linux/perf/arm_pmuv3.h
+++ b/include/linux/perf/arm_pmuv3.h
@@ -267,4 +267,10 @@
 #define ARMV8_PMU_BUS_WIDTH_SHIFT 16
 #define ARMV8_PMU_BUS_WIDTH_MASK 0xf
 
+/* PMU Version in DFR Register */
+#define ARMV8_PMU_DFR_VER_NI        0
+#define ARMV8_PMU_DFR_VER_V3P4      0x5
+#define ARMV8_PMU_DFR_VER_V3P5      0x6
+#define ARMV8_PMU_DFR_VER_IMP_DEF   0xF
+
 #endif
-- 
2.39.0.246.g2a6d74b583-goog
Re: [PATCH 3/8] perf: pmuv3: Add common defines for the PMU version
Posted by Marc Zyngier 2 years, 7 months ago
On Thu, 26 Jan 2023 20:44:39 +0000,
Zaid Al-Bassam <zalbassam@google.com> wrote:
> 
> The current PMU version defines are available for arm64 only,
> As we want to add PMUv3 support to arm (32-bit), this patch makes
> these defines available for both arm/arm64 by defining them in
> the common arm_pmuv3.h header.
> 
> Signed-off-by: Zaid Al-Bassam <zalbassam@google.com>
> ---
>  drivers/perf/arm_pmuv3.c       | 8 ++++----
>  include/linux/perf/arm_pmuv3.h | 6 ++++++
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 94e4098b662d..505f0758260c 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -392,7 +392,7 @@ static const struct attribute_group armv8_pmuv3_caps_attr_group = {
>   */
>  static bool armv8pmu_has_long_event(struct arm_pmu *cpu_pmu)
>  {
> -	return (cpu_pmu->pmuver >= ID_AA64DFR0_EL1_PMUVer_V3P5);
> +	return (cpu_pmu->pmuver >= ARMV8_PMU_DFR_VER_V3P5);

This doesn't really makes any sense. As per the architecture spec,
PMUv3p5 on AArch32 cannot expose the top 32 bits (DDI0487I.a, G8.4.10
"PMEVCNTR<n>, Performance Monitors Event Count Registers, n = 0 -
30"):

<quote>
There is no means to access bits [63:32] directly from AArch32 state.
</quote>

So on AArch32, this should always return false, no ifs, no buts.

Also, turning the architectural symbols (ID_AA64DFR0_EL1_*) into
custom stuff is a total non-starter. We generate these names and want
to use them everywhere.

Either you abstract them in the architecture specific headers, or you
define the AArch64 name in the AArch32 code.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.