Currently, PMUv3 driver is only available for ARMv8 aarch64
platforms, ARMv8 running in aarch32 mode dont have access to
the driver. This is, especially, a problem for ARMv8 platforms
that only have aarch32 support, like the Cortex-A32.
Make the PMUv3 driver available to arm arch (ARMv8 aarch32) by
moving the PMUv3 driver from arm64 to drivers, that makes the
driver common to both arm and arm64 architectures, then add
PMUv3 arm Support.
The main work in this patchset was made a while back by Marc
Zyngier on a private branch. This patchset rebases Marc's
patches to the latest kernel revision and adds additional
patches to accommodate the changes in the kernel since Marc
wrote the patches.
Marc Zyngier (5):
arm64: perf: Move PMUv3 driver to drivers/perf
arm64: perf: Abstract system register accesses away
ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations
ARM: perf: Allow the use of the PMUv3 driver on 32bit ARM
ARM: mach-virt: Select PMUv3 driver by default
Zaid Al-Bassam (3):
perf: pmuv3: Add common defines for the PMU version
perf: pmuv3: Add wrappers for KVM accesses
perf: pmuv3: Change GENMASK to GENMASK_ULL
arch/arm/Kconfig | 1 +
arch/arm/include/asm/arm_pmuv3.h | 238 +++++++++++++++
arch/arm/mm/Kconfig | 2 +-
arch/arm64/include/asm/arm_pmuv3.h | 210 +++++++++++++
arch/arm64/include/asm/perf_event.h | 249 ----------------
arch/arm64/kernel/Makefile | 1 -
drivers/perf/Kconfig | 10 +
drivers/perf/Makefile | 1 +
.../perf_event.c => drivers/perf/arm_pmuv3.c | 139 +++------
include/kvm/arm_pmu.h | 2 +-
include/linux/perf/arm_pmuv3.h | 276 ++++++++++++++++++
11 files changed, 773 insertions(+), 356 deletions(-)
create mode 100644 arch/arm/include/asm/arm_pmuv3.h
create mode 100644 arch/arm64/include/asm/arm_pmuv3.h
rename arch/arm64/kernel/perf_event.c => drivers/perf/arm_pmuv3.c (92%)
create mode 100644 include/linux/perf/arm_pmuv3.h
--
2.39.0.246.g2a6d74b583-goog
From: Marc Zyngier <[email protected]>
As we want to enable 32bit support, we need to distanciate the
PMUv3 driver from the AArch64 system register names.
This patch moves all system register accesses to an architecture
specific include file, allowing the 32bit counterpart to be
slotted in at a later time.
Signed-off-by: Marc Zyngier <[email protected]>
Co-developed-by: Zaid Al-Bassam <[email protected]>
Signed-off-by: Zaid Al-Bassam <[email protected]>
---
arch/arm64/include/asm/arm_pmuv3.h | 194 +++++++++++++++++++++++++++++
drivers/perf/arm_pmuv3.c | 115 ++++-------------
2 files changed, 217 insertions(+), 92 deletions(-)
create mode 100644 arch/arm64/include/asm/arm_pmuv3.h
diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h
new file mode 100644
index 000000000000..f41a354d1022
--- /dev/null
+++ b/arch/arm64/include/asm/arm_pmuv3.h
@@ -0,0 +1,194 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ASM_PMUV3_H
+#define __ASM_PMUV3_H
+
+#include <asm/cpufeature.h>
+#include <asm/sysreg.h>
+
+/*
+ * This code is really good
+ */
+
+#define PMEVN_CASE(n, case_macro) \
+ case n: case_macro(n); break
+
+#define PMEVN_SWITCH(x, case_macro) \
+ do { \
+ switch (x) { \
+ PMEVN_CASE(0, case_macro); \
+ PMEVN_CASE(1, case_macro); \
+ PMEVN_CASE(2, case_macro); \
+ PMEVN_CASE(3, case_macro); \
+ PMEVN_CASE(4, case_macro); \
+ PMEVN_CASE(5, case_macro); \
+ PMEVN_CASE(6, case_macro); \
+ PMEVN_CASE(7, case_macro); \
+ PMEVN_CASE(8, case_macro); \
+ PMEVN_CASE(9, case_macro); \
+ PMEVN_CASE(10, case_macro); \
+ PMEVN_CASE(11, case_macro); \
+ PMEVN_CASE(12, case_macro); \
+ PMEVN_CASE(13, case_macro); \
+ PMEVN_CASE(14, case_macro); \
+ PMEVN_CASE(15, case_macro); \
+ PMEVN_CASE(16, case_macro); \
+ PMEVN_CASE(17, case_macro); \
+ PMEVN_CASE(18, case_macro); \
+ PMEVN_CASE(19, case_macro); \
+ PMEVN_CASE(20, case_macro); \
+ PMEVN_CASE(21, case_macro); \
+ PMEVN_CASE(22, case_macro); \
+ PMEVN_CASE(23, case_macro); \
+ PMEVN_CASE(24, case_macro); \
+ PMEVN_CASE(25, case_macro); \
+ PMEVN_CASE(26, case_macro); \
+ PMEVN_CASE(27, case_macro); \
+ PMEVN_CASE(28, case_macro); \
+ PMEVN_CASE(29, case_macro); \
+ PMEVN_CASE(30, case_macro); \
+ default: WARN(1, "Invalid PMEV* index\n"); \
+ } \
+ } while (0)
+
+#define RETURN_READ_PMEVCNTRN(n) \
+ return read_sysreg(pmevcntr##n##_el0)
+static unsigned long read_pmevcntrn(int n)
+{
+ PMEVN_SWITCH(n, RETURN_READ_PMEVCNTRN);
+ return 0;
+}
+
+#define WRITE_PMEVCNTRN(n) \
+ write_sysreg(val, pmevcntr##n##_el0)
+static void write_pmevcntrn(int n, unsigned long val)
+{
+ PMEVN_SWITCH(n, WRITE_PMEVCNTRN);
+}
+
+#define WRITE_PMEVTYPERN(n) \
+ write_sysreg(val, pmevtyper##n##_el0)
+static void write_pmevtypern(int n, unsigned long val)
+{
+ PMEVN_SWITCH(n, WRITE_PMEVTYPERN);
+}
+
+static inline unsigned long read_pmmir(void)
+{
+ return read_cpuid(PMMIR_EL1);
+}
+
+static inline u32 read_pmuver(void)
+{
+ u64 dfr0 = read_sysreg(id_aa64dfr0_el1);
+
+ return cpuid_feature_extract_unsigned_field(dfr0,
+ ID_AA64DFR0_EL1_PMUVer_SHIFT);
+}
+
+static inline void write_pmcr(u32 val)
+{
+ write_sysreg(val, pmcr_el0);
+}
+
+static inline u32 read_pmcr(void)
+{
+ return read_sysreg(pmcr_el0);
+}
+
+static inline void write_pmselr(u32 val)
+{
+ write_sysreg(val, pmselr_el0);
+}
+
+static inline void write_pmccntr(u64 val)
+{
+ write_sysreg(val, pmccntr_el0);
+}
+
+static inline u64 read_pmccntr(void)
+{
+ return read_sysreg(pmccntr_el0);
+}
+
+static inline void write_pmxevcntr(u32 val)
+{
+ write_sysreg(val, pmxevcntr_el0);
+}
+
+static inline u32 read_pmxevcntr(void)
+{
+ return read_sysreg(pmxevcntr_el0);
+}
+
+static inline void write_pmxevtyper(u32 val)
+{
+ write_sysreg(val, pmxevtyper_el0);
+}
+
+static inline void write_pmcntenset(u32 val)
+{
+ write_sysreg(val, pmcntenset_el0);
+}
+
+static inline void write_pmcntenclr(u32 val)
+{
+ write_sysreg(val, pmcntenclr_el0);
+}
+
+static inline void write_pmintenset(u32 val)
+{
+ write_sysreg(val, pmintenset_el1);
+}
+
+static inline void write_pmintenclr(u32 val)
+{
+ write_sysreg(val, pmintenclr_el1);
+}
+
+static inline void write_pmccfiltr(u32 val)
+{
+ write_sysreg(val, pmccfiltr_el0);
+}
+
+static inline void write_pmovsclr(u32 val)
+{
+ write_sysreg(val, pmovsclr_el0);
+}
+
+static inline u32 read_pmovsclr(void)
+{
+ return read_sysreg(pmovsclr_el0);
+}
+
+static inline void write_pmuserenr(u32 val)
+{
+ write_sysreg(val, pmuserenr_el0);
+}
+
+static inline u32 read_pmceid0(void)
+{
+ return read_sysreg(pmceid0_el0);
+}
+
+static inline u32 read_pmceid1(void)
+{
+ return read_sysreg(pmceid1_el0);
+}
+
+#endif
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 781364bf3f41..94e4098b662d 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -10,7 +10,6 @@
#include <asm/irq_regs.h>
#include <asm/perf_event.h>
-#include <asm/sysreg.h>
#include <asm/virt.h>
#include <clocksource/arm_arch_timer.h>
@@ -25,6 +24,8 @@
#include <linux/sched_clock.h>
#include <linux/smp.h>
+#include <asm/arm_pmuv3.h>
+
/* ARMv8 Cortex-A53 specific event types. */
#define ARMV8_A53_PERFCTR_PREF_LINEFILL 0xC2
@@ -425,83 +426,16 @@ static inline bool armv8pmu_event_is_chained(struct perf_event *event)
#define ARMV8_IDX_TO_COUNTER(x) \
(((x) - ARMV8_IDX_COUNTER0) & ARMV8_PMU_COUNTER_MASK)
-/*
- * This code is really good
- */
-
-#define PMEVN_CASE(n, case_macro) \
- case n: case_macro(n); break
-
-#define PMEVN_SWITCH(x, case_macro) \
- do { \
- switch (x) { \
- PMEVN_CASE(0, case_macro); \
- PMEVN_CASE(1, case_macro); \
- PMEVN_CASE(2, case_macro); \
- PMEVN_CASE(3, case_macro); \
- PMEVN_CASE(4, case_macro); \
- PMEVN_CASE(5, case_macro); \
- PMEVN_CASE(6, case_macro); \
- PMEVN_CASE(7, case_macro); \
- PMEVN_CASE(8, case_macro); \
- PMEVN_CASE(9, case_macro); \
- PMEVN_CASE(10, case_macro); \
- PMEVN_CASE(11, case_macro); \
- PMEVN_CASE(12, case_macro); \
- PMEVN_CASE(13, case_macro); \
- PMEVN_CASE(14, case_macro); \
- PMEVN_CASE(15, case_macro); \
- PMEVN_CASE(16, case_macro); \
- PMEVN_CASE(17, case_macro); \
- PMEVN_CASE(18, case_macro); \
- PMEVN_CASE(19, case_macro); \
- PMEVN_CASE(20, case_macro); \
- PMEVN_CASE(21, case_macro); \
- PMEVN_CASE(22, case_macro); \
- PMEVN_CASE(23, case_macro); \
- PMEVN_CASE(24, case_macro); \
- PMEVN_CASE(25, case_macro); \
- PMEVN_CASE(26, case_macro); \
- PMEVN_CASE(27, case_macro); \
- PMEVN_CASE(28, case_macro); \
- PMEVN_CASE(29, case_macro); \
- PMEVN_CASE(30, case_macro); \
- default: WARN(1, "Invalid PMEV* index\n"); \
- } \
- } while (0)
-
-#define RETURN_READ_PMEVCNTRN(n) \
- return read_sysreg(pmevcntr##n##_el0)
-static unsigned long read_pmevcntrn(int n)
-{
- PMEVN_SWITCH(n, RETURN_READ_PMEVCNTRN);
- return 0;
-}
-
-#define WRITE_PMEVCNTRN(n) \
- write_sysreg(val, pmevcntr##n##_el0)
-static void write_pmevcntrn(int n, unsigned long val)
-{
- PMEVN_SWITCH(n, WRITE_PMEVCNTRN);
-}
-
-#define WRITE_PMEVTYPERN(n) \
- write_sysreg(val, pmevtyper##n##_el0)
-static void write_pmevtypern(int n, unsigned long val)
-{
- PMEVN_SWITCH(n, WRITE_PMEVTYPERN);
-}
-
static inline u32 armv8pmu_pmcr_read(void)
{
- return read_sysreg(pmcr_el0);
+ return read_pmcr();
}
static inline void armv8pmu_pmcr_write(u32 val)
{
val &= ARMV8_PMU_PMCR_MASK;
isb();
- write_sysreg(val, pmcr_el0);
+ write_pmcr(val);
}
static inline int armv8pmu_has_overflowed(u32 pmovsr)
@@ -576,7 +510,7 @@ static u64 armv8pmu_read_counter(struct perf_event *event)
u64 value;
if (idx == ARMV8_IDX_CYCLE_COUNTER)
- value = read_sysreg(pmccntr_el0);
+ value = read_pmccntr();
else
value = armv8pmu_read_hw_counter(event);
@@ -611,7 +545,7 @@ static void armv8pmu_write_counter(struct perf_event *event, u64 value)
value = armv8pmu_bias_long_counter(event, value);
if (idx == ARMV8_IDX_CYCLE_COUNTER)
- write_sysreg(value, pmccntr_el0);
+ write_pmccntr(value);
else
armv8pmu_write_hw_counter(event, value);
}
@@ -642,7 +576,7 @@ static inline void armv8pmu_write_event_type(struct perf_event *event)
armv8pmu_write_evtype(idx, chain_evt);
} else {
if (idx == ARMV8_IDX_CYCLE_COUNTER)
- write_sysreg(hwc->config_base, pmccfiltr_el0);
+ write_pmccfiltr(hwc->config_base);
else
armv8pmu_write_evtype(idx, hwc->config_base);
}
@@ -665,7 +599,7 @@ static inline void armv8pmu_enable_counter(u32 mask)
* enable the counter.
* */
isb();
- write_sysreg(mask, pmcntenset_el0);
+ write_pmcntenset(mask);
}
static inline void armv8pmu_enable_event_counter(struct perf_event *event)
@@ -682,7 +616,7 @@ static inline void armv8pmu_enable_event_counter(struct perf_event *event)
static inline void armv8pmu_disable_counter(u32 mask)
{
- write_sysreg(mask, pmcntenclr_el0);
+ write_pmcntenclr(mask);
/*
* Make sure the effects of disabling the counter are visible before we
* start configuring the event.
@@ -704,7 +638,7 @@ static inline void armv8pmu_disable_event_counter(struct perf_event *event)
static inline void armv8pmu_enable_intens(u32 mask)
{
- write_sysreg(mask, pmintenset_el1);
+ write_pmintenset(mask);
}
static inline void armv8pmu_enable_event_irq(struct perf_event *event)
@@ -715,10 +649,10 @@ static inline void armv8pmu_enable_event_irq(struct perf_event *event)
static inline void armv8pmu_disable_intens(u32 mask)
{
- write_sysreg(mask, pmintenclr_el1);
+ write_pmintenclr(mask);
isb();
/* Clear the overflow flag in case an interrupt is pending. */
- write_sysreg(mask, pmovsclr_el0);
+ write_pmovsclr(mask);
isb();
}
@@ -733,18 +667,18 @@ static inline u32 armv8pmu_getreset_flags(void)
u32 value;
/* Read */
- value = read_sysreg(pmovsclr_el0);
+ value = read_pmovsclr();
/* Write to clear flags */
value &= ARMV8_PMU_OVSR_MASK;
- write_sysreg(value, pmovsclr_el0);
+ write_pmovsclr(value);
return value;
}
static void armv8pmu_disable_user_access(void)
{
- write_sysreg(0, pmuserenr_el0);
+ write_pmuserenr(0);
}
static void armv8pmu_enable_user_access(struct arm_pmu *cpu_pmu)
@@ -755,13 +689,13 @@ static void armv8pmu_enable_user_access(struct arm_pmu *cpu_pmu)
/* Clear any unused counters to avoid leaking their contents */
for_each_clear_bit(i, cpuc->used_mask, cpu_pmu->num_events) {
if (i == ARMV8_IDX_CYCLE_COUNTER)
- write_sysreg(0, pmccntr_el0);
+ write_pmccntr(0);
else
armv8pmu_write_evcntr(i, 0);
}
- write_sysreg(0, pmuserenr_el0);
- write_sysreg(ARMV8_PMU_USERENR_ER | ARMV8_PMU_USERENR_CR, pmuserenr_el0);
+ write_pmuserenr(0);
+ write_pmuserenr(ARMV8_PMU_USERENR_ER | ARMV8_PMU_USERENR_CR);
}
static void armv8pmu_enable_event(struct perf_event *event)
@@ -1143,14 +1077,11 @@ static void __armv8pmu_probe_pmu(void *info)
{
struct armv8pmu_probe_info *probe = info;
struct arm_pmu *cpu_pmu = probe->pmu;
- u64 dfr0;
u64 pmceid_raw[2];
u32 pmceid[2];
int pmuver;
- dfr0 = read_sysreg(id_aa64dfr0_el1);
- pmuver = cpuid_feature_extract_unsigned_field(dfr0,
- ID_AA64DFR0_EL1_PMUVer_SHIFT);
+ pmuver = read_pmuver();
if (pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF ||
pmuver == ID_AA64DFR0_EL1_PMUVer_NI)
return;
@@ -1165,8 +1096,8 @@ static void __armv8pmu_probe_pmu(void *info)
/* Add the CPU cycles counter */
cpu_pmu->num_events += 1;
- pmceid[0] = pmceid_raw[0] = read_sysreg(pmceid0_el0);
- pmceid[1] = pmceid_raw[1] = read_sysreg(pmceid1_el0);
+ pmceid[0] = pmceid_raw[0] = read_pmceid0();
+ pmceid[1] = pmceid_raw[1] = read_pmceid1();
bitmap_from_arr32(cpu_pmu->pmceid_bitmap,
pmceid, ARMV8_PMUV3_MAX_COMMON_EVENTS);
@@ -1177,9 +1108,9 @@ static void __armv8pmu_probe_pmu(void *info)
bitmap_from_arr32(cpu_pmu->pmceid_ext_bitmap,
pmceid, ARMV8_PMUV3_MAX_COMMON_EVENTS);
- /* store PMMIR_EL1 register for sysfs */
+ /* store PMMIR register for sysfs */
if (pmuver >= ID_AA64DFR0_EL1_PMUVer_V3P4 && (pmceid_raw[1] & BIT(31)))
- cpu_pmu->reg_pmmir = read_cpuid(PMMIR_EL1);
+ cpu_pmu->reg_pmmir = read_pmmir();
else
cpu_pmu->reg_pmmir = 0;
}
--
2.39.0.246.g2a6d74b583-goog
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 <[email protected]>
---
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
KVM host support is available only on arm64. This patch adds wrappers
to the KVM host function references in the arm_pmuv3.c, so that it is
up to architecture to populate these wrappers if supported.
Signed-off-by: Zaid Al-Bassam <[email protected]>
---
arch/arm64/include/asm/arm_pmuv3.h | 16 ++++++++++++++++
drivers/perf/arm_pmuv3.c | 11 +++++------
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h
index f41a354d1022..9f2a48f5340c 100644
--- a/arch/arm64/include/asm/arm_pmuv3.h
+++ b/arch/arm64/include/asm/arm_pmuv3.h
@@ -20,6 +20,7 @@
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <linux/kvm_host.h>
/*
* This code is really good
@@ -191,4 +192,19 @@ static inline u32 read_pmceid1(void)
return read_sysreg(pmceid1_el0);
}
+static inline void armv8pmu_kvm_set_events(u32 set, struct perf_event_attr *attr)
+{
+ kvm_set_pmu_events(set, attr);
+}
+
+static inline void armv8pmu_kvm_clr_events(u32 clr)
+{
+ kvm_clr_pmu_events(clr);
+}
+
+static inline bool armv8pmu_kvm_counter_deferred(struct perf_event_attr *attr)
+{
+ return kvm_pmu_counter_deferred(attr);
+}
+
#endif
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 505f0758260c..d7063dd52827 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -16,7 +16,6 @@
#include <linux/acpi.h>
#include <linux/clocksource.h>
-#include <linux/kvm_host.h>
#include <linux/of.h>
#include <linux/perf/arm_pmu.h>
#include <linux/perf/arm_pmuv3.h>
@@ -607,10 +606,10 @@ static inline void armv8pmu_enable_event_counter(struct perf_event *event)
struct perf_event_attr *attr = &event->attr;
u32 mask = armv8pmu_event_cnten_mask(event);
- kvm_set_pmu_events(mask, attr);
+ armv8pmu_kvm_set_events(mask, attr);
/* We rely on the hypervisor switch code to enable guest counters */
- if (!kvm_pmu_counter_deferred(attr))
+ if (!armv8pmu_kvm_counter_deferred(attr))
armv8pmu_enable_counter(mask);
}
@@ -629,10 +628,10 @@ static inline void armv8pmu_disable_event_counter(struct perf_event *event)
struct perf_event_attr *attr = &event->attr;
u32 mask = armv8pmu_event_cnten_mask(event);
- kvm_clr_pmu_events(mask);
+ armv8pmu_kvm_clr_events(mask);
/* We rely on the hypervisor switch code to disable guest counters */
- if (!kvm_pmu_counter_deferred(attr))
+ if (!armv8pmu_kvm_counter_deferred(attr))
armv8pmu_disable_counter(mask);
}
@@ -974,7 +973,7 @@ static void armv8pmu_reset(void *info)
armv8pmu_disable_intens(U32_MAX);
/* Clear the counters we flip at guest entry/exit */
- kvm_clr_pmu_events(U32_MAX);
+ armv8pmu_kvm_clr_events(U32_MAX);
/*
* Initialize & Reset PMNC. Request overflow interrupt for
--
2.39.0.246.g2a6d74b583-goog
GENMASK macro uses "unsigned long" (32-bit wide on arm and 64-bit
on arm64), This causes build issues when enabling PMUv3 on arm as
it tries to access bits > 31. This patch switches the GENMASK to
GENMASK_ULL, which uses "unsigned long long" (64-bit on both arm
and arm64).
Signed-off-by: Zaid Al-Bassam <[email protected]>
---
drivers/perf/arm_pmuv3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index d7063dd52827..67d90c1d4bae 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -489,7 +489,7 @@ static bool armv8pmu_event_needs_bias(struct perf_event *event)
static u64 armv8pmu_bias_long_counter(struct perf_event *event, u64 value)
{
if (armv8pmu_event_needs_bias(event))
- value |= GENMASK(63, 32);
+ value |= GENMASK_ULL(63, 32);
return value;
}
@@ -497,7 +497,7 @@ static u64 armv8pmu_bias_long_counter(struct perf_event *event, u64 value)
static u64 armv8pmu_unbias_long_counter(struct perf_event *event, u64 value)
{
if (armv8pmu_event_needs_bias(event))
- value &= ~GENMASK(63, 32);
+ value &= ~GENMASK_ULL(63, 32);
return value;
}
--
2.39.0.246.g2a6d74b583-goog
From: Marc Zyngier <[email protected]>
ARMv8 is a superset of ARMv7, and all the ARMv8 features are
discoverable with a set of ID registers. It means that we can
use CPU_V7 to guard ARMv8 features at compile time.
This commit simply amends the CPU_V7 configuration symbol comment
to reflect that CPU_V7 also covers ARMv8.
Signed-off-by: Marc Zyngier <[email protected]>
Signed-off-by: Zaid Al-Bassam <[email protected]>
---
arch/arm/mm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index e949e84a2dec..b77f4a0d9072 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -403,7 +403,7 @@ config CPU_V6K
select CPU_THUMB_CAPABLE
select CPU_TLB_V6 if MMU
-# ARMv7
+# ARMv7 and ARMv8 architectures
config CPU_V7
bool
select CPU_32v6K
--
2.39.0.246.g2a6d74b583-goog
From: Marc Zyngier <[email protected]>
The only thing stopping the PMUv3 driver from compiling on 32bit
is the lack of defined system registers names. This is easily
solved by providing the sysreg accessors and updating the Kconfig entry.
Signed-off-by: Marc Zyngier <[email protected]>
Co-developed-by: Zaid Al-Bassam <[email protected]>
Signed-off-by: Zaid Al-Bassam <[email protected]>
---
arch/arm/include/asm/arm_pmuv3.h | 238 +++++++++++++++++++++++++++++++
drivers/perf/Kconfig | 5 +-
2 files changed, 240 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/include/asm/arm_pmuv3.h
diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
new file mode 100644
index 000000000000..816873c74eda
--- /dev/null
+++ b/arch/arm/include/asm/arm_pmuv3.h
@@ -0,0 +1,238 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ASM_PMUV3_H
+#define __ASM_PMUV3_H
+
+#include <asm/cp15.h>
+#include <asm/cputype.h>
+
+#define PMCCNTR __ACCESS_CP15_64(0, c9)
+
+#define PMCR __ACCESS_CP15(c9, 0, c12, 0)
+#define PMCNTENSET __ACCESS_CP15(c9, 0, c12, 1)
+#define PMCNTENCLR __ACCESS_CP15(c9, 0, c12, 2)
+#define PMOVSR __ACCESS_CP15(c9, 0, c12, 3)
+#define PMSELR __ACCESS_CP15(c9, 0, c12, 5)
+#define PMCEID0 __ACCESS_CP15(c9, 0, c12, 6)
+#define PMCEID1 __ACCESS_CP15(c9, 0, c12, 7)
+#define PMXEVTYPER __ACCESS_CP15(c9, 0, c13, 1)
+#define PMXEVCNTR __ACCESS_CP15(c9, 0, c13, 2)
+#define PMUSERENR __ACCESS_CP15(c9, 0, c14, 0)
+#define PMINTENSET __ACCESS_CP15(c9, 0, c14, 1)
+#define PMINTENCLR __ACCESS_CP15(c9, 0, c14, 2)
+#define PMMIR __ACCESS_CP15(c9, 0, c14, 6)
+#define PMCCFILTR __ACCESS_CP15(c14, 0, c15, 7)
+#define PMEVCNTR0(n) __ACCESS_CP15(c14, 0, c8, n)
+#define PMEVCNTR1(n) __ACCESS_CP15(c14, 0, c9, n)
+#define PMEVCNTR2(n) __ACCESS_CP15(c14, 0, c10, n)
+#define PMEVCNTR3(n) __ACCESS_CP15(c14, 0, c11, n)
+#define PMEVTYPER0(n) __ACCESS_CP15(c14, 0, c12, n)
+#define PMEVTYPER1(n) __ACCESS_CP15(c14, 0, c13, n)
+#define PMEVTYPER2(n) __ACCESS_CP15(c14, 0, c14, n)
+#define PMEVTYPER3(n) __ACCESS_CP15(c14, 0, c15, n)
+
+#define PMEV_EVENTS_PER_REG 8
+#define PMEV_REGISTER(n) (n / PMEV_EVENTS_PER_REG)
+#define PMEV_EVENT(n) (n % PMEV_EVENTS_PER_REG)
+
+#define PMEV_CASE(reg, ev, case_macro) \
+ case ev: \
+ case_macro(reg, ev); \
+ break
+
+#define PMEV_EV_SWITCH(reg, ev, case_macro) \
+ do { \
+ switch (ev) { \
+ PMEV_CASE(reg, 0, case_macro); \
+ PMEV_CASE(reg, 1, case_macro); \
+ PMEV_CASE(reg, 2, case_macro); \
+ PMEV_CASE(reg, 3, case_macro); \
+ PMEV_CASE(reg, 4, case_macro); \
+ PMEV_CASE(reg, 5, case_macro); \
+ PMEV_CASE(reg, 6, case_macro); \
+ PMEV_CASE(reg, 7, case_macro); \
+ default: \
+ WARN(1, "Invalid PMEV* event index\n"); \
+ } \
+ } while (0)
+
+#define PMEV_REG_SWITCH(reg, ev, case_macro) \
+ do { \
+ switch (reg) { \
+ case 0: \
+ PMEV_EV_SWITCH(0, ev, case_macro); \
+ break; \
+ case 1: \
+ PMEV_EV_SWITCH(1, ev, case_macro); \
+ break; \
+ case 2: \
+ PMEV_EV_SWITCH(2, ev, case_macro); \
+ break; \
+ case 3: \
+ PMEV_EV_SWITCH(3, ev, case_macro); \
+ break; \
+ default: \
+ WARN(1, "Invalid PMEV* register index\n"); \
+ } \
+ } while (0)
+
+#define RETURN_READ_PMEVCNTR(reg, ev) \
+ return read_sysreg(PMEVCNTR##reg(ev))
+static unsigned long read_pmevcntrn(int n)
+{
+ const int reg = PMEV_REGISTER(n);
+ const int event = PMEV_EVENT(n);
+
+ PMEV_REG_SWITCH(reg, event, RETURN_READ_PMEVCNTR);
+ return 0;
+}
+
+#define WRITE_PMEVCNTR(reg, ev) \
+ write_sysreg(val, PMEVCNTR##reg(ev))
+static void write_pmevcntrn(int n, unsigned long val)
+{
+ const int reg = PMEV_REGISTER(n);
+ const int event = PMEV_EVENT(n);
+
+ PMEV_REG_SWITCH(reg, event, WRITE_PMEVCNTR);
+}
+
+#define WRITE_PMEVTYPER(reg, ev) \
+ write_sysreg(val, PMEVTYPER##reg(ev))
+static void write_pmevtypern(int n, unsigned long val)
+{
+ const int reg = PMEV_REGISTER(n);
+ const int event = PMEV_EVENT(n);
+
+ PMEV_REG_SWITCH(reg, event, WRITE_PMEVTYPER);
+}
+
+static inline unsigned long read_pmmir(void)
+{
+ return read_sysreg(PMMIR);
+}
+
+static inline u32 read_pmuver(void)
+{
+ /* PMUVers is not a signed field */
+ u32 dfr0 = read_cpuid_ext(CPUID_EXT_DFR0);
+
+ return (dfr0 >> 24) & 0xf;
+}
+
+static inline void write_pmcr(u32 val)
+{
+ write_sysreg(val, PMCR);
+}
+
+static inline u32 read_pmcr(void)
+{
+ return read_sysreg(PMCR);
+}
+
+static inline void write_pmselr(u32 val)
+{
+ write_sysreg(val, PMSELR);
+}
+
+static inline void write_pmccntr(u64 val)
+{
+ write_sysreg(val, PMCCNTR);
+}
+
+static inline u64 read_pmccntr(void)
+{
+ return read_sysreg(PMCCNTR);
+}
+
+static inline void write_pmxevcntr(u32 val)
+{
+ write_sysreg(val, PMXEVCNTR);
+}
+
+static inline u32 read_pmxevcntr(void)
+{
+ return read_sysreg(PMXEVCNTR);
+}
+
+static inline void write_pmxevtyper(u32 val)
+{
+ write_sysreg(val, PMXEVTYPER);
+}
+
+static inline void write_pmcntenset(u32 val)
+{
+ write_sysreg(val, PMCNTENSET);
+}
+
+static inline void write_pmcntenclr(u32 val)
+{
+ write_sysreg(val, PMCNTENCLR);
+}
+
+static inline void write_pmintenset(u32 val)
+{
+ write_sysreg(val, PMINTENSET);
+}
+
+static inline void write_pmintenclr(u32 val)
+{
+ write_sysreg(val, PMINTENCLR);
+}
+
+static inline void write_pmccfiltr(u32 val)
+{
+ write_sysreg(val, PMCCFILTR);
+}
+
+static inline void write_pmovsclr(u32 val)
+{
+ write_sysreg(val, PMOVSR);
+}
+
+static inline u32 read_pmovsclr(void)
+{
+ return read_sysreg(PMOVSR);
+}
+
+static inline void write_pmuserenr(u32 val)
+{
+ write_sysreg(val, PMUSERENR);
+}
+
+static inline u32 read_pmceid0(void)
+{
+ return read_sysreg(PMCEID0);
+}
+
+static inline u32 read_pmceid1(void)
+{
+ return read_sysreg(PMCEID1);
+}
+
+static inline void
+armv8pmu_kvm_set_events(u32 set, struct perf_event_attr *attr) {}
+
+static inline void armv8pmu_kvm_clr_events(u32 clr) {}
+
+static inline bool armv8pmu_kvm_counter_deferred(struct perf_event_attr *attr)
+{
+ return false;
+}
+
+#endif
diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index defe6b47854b..711f82400086 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -101,15 +101,14 @@ config ARM_SMMU_V3_PMU
based on the Stream ID of the corresponding master.
config ARM_PMUV3
- depends on HW_PERF_EVENTS && ARM64
+ depends on HW_PERF_EVENTS && ((ARM && CPU_V7) || ARM64)
bool "ARM PMUv3 support" if !ARM64
- default y
+ default ARM64
help
Say y if you want to use the ARM performance monitor unit (PMU)
version 3. The PMUv3 is the CPU performance monitors on ARMv8
(aarch32 and aarch64) systems that implement the PMUv3
architecture.
- Currently, PMUv3 is only supported on aarch64 (arm64)
config ARM_DSU_PMU
tristate "ARM DynamIQ Shared Unit (DSU) PMU"
--
2.39.0.246.g2a6d74b583-goog
From: Marc Zyngier <[email protected]>
Since 32bit guests are not unlikely to run on an ARMv8 host,
let's select the PMUv3 driver, which allows the PMU to be used
on such systems.
Signed-off-by: Marc Zyngier <[email protected]>
Signed-off-by: Zaid Al-Bassam <[email protected]>
---
arch/arm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 137f89ad0521..3802997fb36e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -400,6 +400,7 @@ config ARCH_VIRT
select ARM_GIC_V3
select ARM_GIC_V3_ITS if PCI
select ARM_PSCI
+ select ARM_PMUV3 if PERF_EVENTS
select HAVE_ARM_ARCH_TIMER
config ARCH_AIROHA
--
2.39.0.246.g2a6d74b583-goog
On Thu, 26 Jan 2023 20:44:39 +0000,
Zaid Al-Bassam <[email protected]> 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 <[email protected]>
> ---
> 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.
On Thu, 26 Jan 2023 20:44:40 +0000,
Zaid Al-Bassam <[email protected]> wrote:
>
> KVM host support is available only on arm64. This patch adds wrappers
> to the KVM host function references in the arm_pmuv3.c, so that it is
> up to architecture to populate these wrappers if supported.
>
> Signed-off-by: Zaid Al-Bassam <[email protected]>
> ---
> arch/arm64/include/asm/arm_pmuv3.h | 16 ++++++++++++++++
> drivers/perf/arm_pmuv3.c | 11 +++++------
> 2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h
> index f41a354d1022..9f2a48f5340c 100644
> --- a/arch/arm64/include/asm/arm_pmuv3.h
> +++ b/arch/arm64/include/asm/arm_pmuv3.h
> @@ -20,6 +20,7 @@
>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <linux/kvm_host.h>
"linux" includes must be placed before the "asm" ones.
>
> /*
> * This code is really good
> @@ -191,4 +192,19 @@ static inline u32 read_pmceid1(void)
> return read_sysreg(pmceid1_el0);
> }
>
> +static inline void armv8pmu_kvm_set_events(u32 set, struct perf_event_attr *attr)
> +{
> + kvm_set_pmu_events(set, attr);
> +}
> +
> +static inline void armv8pmu_kvm_clr_events(u32 clr)
> +{
> + kvm_clr_pmu_events(clr);
> +}
> +
> +static inline bool armv8pmu_kvm_counter_deferred(struct perf_event_attr *attr)
> +{
> + return kvm_pmu_counter_deferred(attr);
> +}
> +
> #endif
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 505f0758260c..d7063dd52827 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -16,7 +16,6 @@
>
> #include <linux/acpi.h>
> #include <linux/clocksource.h>
> -#include <linux/kvm_host.h>
> #include <linux/of.h>
> #include <linux/perf/arm_pmu.h>
> #include <linux/perf/arm_pmuv3.h>
> @@ -607,10 +606,10 @@ static inline void armv8pmu_enable_event_counter(struct perf_event *event)
> struct perf_event_attr *attr = &event->attr;
> u32 mask = armv8pmu_event_cnten_mask(event);
>
> - kvm_set_pmu_events(mask, attr);
> + armv8pmu_kvm_set_events(mask, attr);
Why the change of name? given that you will implement empty helpers in
the AArch32 code, you might as well keep the names and reduce the churn.
At the end of the day, the only thing you need to do in this patch is
to move the "#include <linux/kvm_host.h>" from this file into the
arch-specific file. Nothing else.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Thu, 26 Jan 2023 20:44:41 +0000,
Zaid Al-Bassam <[email protected]> wrote:
>
> GENMASK macro uses "unsigned long" (32-bit wide on arm and 64-bit
> on arm64), This causes build issues when enabling PMUv3 on arm as
> it tries to access bits > 31. This patch switches the GENMASK to
> GENMASK_ULL, which uses "unsigned long long" (64-bit on both arm
> and arm64).
>
> Signed-off-by: Zaid Al-Bassam <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
M.
--
Without deviation from the norm, progress is not possible.
On Thu, 26 Jan 2023 20:44:43 +0000,
Zaid Al-Bassam <[email protected]> wrote:
>
> From: Marc Zyngier <[email protected]>
>
> The only thing stopping the PMUv3 driver from compiling on 32bit
> is the lack of defined system registers names. This is easily
> solved by providing the sysreg accessors and updating the Kconfig entry.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> Co-developed-by: Zaid Al-Bassam <[email protected]>
> Signed-off-by: Zaid Al-Bassam <[email protected]>
> ---
> arch/arm/include/asm/arm_pmuv3.h | 238 +++++++++++++++++++++++++++++++
> drivers/perf/Kconfig | 5 +-
> 2 files changed, 240 insertions(+), 3 deletions(-)
> create mode 100644 arch/arm/include/asm/arm_pmuv3.h
>
> diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
> new file mode 100644
> index 000000000000..816873c74eda
> --- /dev/null
> +++ b/arch/arm/include/asm/arm_pmuv3.h
> @@ -0,0 +1,238 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2012 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __ASM_PMUV3_H
> +#define __ASM_PMUV3_H
> +
> +#include <asm/cp15.h>
> +#include <asm/cputype.h>
> +
> +#define PMCCNTR __ACCESS_CP15_64(0, c9)
> +
> +#define PMCR __ACCESS_CP15(c9, 0, c12, 0)
> +#define PMCNTENSET __ACCESS_CP15(c9, 0, c12, 1)
> +#define PMCNTENCLR __ACCESS_CP15(c9, 0, c12, 2)
> +#define PMOVSR __ACCESS_CP15(c9, 0, c12, 3)
> +#define PMSELR __ACCESS_CP15(c9, 0, c12, 5)
> +#define PMCEID0 __ACCESS_CP15(c9, 0, c12, 6)
> +#define PMCEID1 __ACCESS_CP15(c9, 0, c12, 7)
> +#define PMXEVTYPER __ACCESS_CP15(c9, 0, c13, 1)
> +#define PMXEVCNTR __ACCESS_CP15(c9, 0, c13, 2)
> +#define PMUSERENR __ACCESS_CP15(c9, 0, c14, 0)
> +#define PMINTENSET __ACCESS_CP15(c9, 0, c14, 1)
> +#define PMINTENCLR __ACCESS_CP15(c9, 0, c14, 2)
> +#define PMMIR __ACCESS_CP15(c9, 0, c14, 6)
> +#define PMCCFILTR __ACCESS_CP15(c14, 0, c15, 7)
> +#define PMEVCNTR0(n) __ACCESS_CP15(c14, 0, c8, n)
> +#define PMEVCNTR1(n) __ACCESS_CP15(c14, 0, c9, n)
> +#define PMEVCNTR2(n) __ACCESS_CP15(c14, 0, c10, n)
> +#define PMEVCNTR3(n) __ACCESS_CP15(c14, 0, c11, n)
> +#define PMEVTYPER0(n) __ACCESS_CP15(c14, 0, c12, n)
> +#define PMEVTYPER1(n) __ACCESS_CP15(c14, 0, c13, n)
> +#define PMEVTYPER2(n) __ACCESS_CP15(c14, 0, c14, n)
> +#define PMEVTYPER3(n) __ACCESS_CP15(c14, 0, c15, n)
> +
> +#define PMEV_EVENTS_PER_REG 8
> +#define PMEV_REGISTER(n) (n / PMEV_EVENTS_PER_REG)
> +#define PMEV_EVENT(n) (n % PMEV_EVENTS_PER_REG)
> +
> +#define PMEV_CASE(reg, ev, case_macro) \
> + case ev: \
> + case_macro(reg, ev); \
> + break
> +
> +#define PMEV_EV_SWITCH(reg, ev, case_macro) \
> + do { \
> + switch (ev) { \
> + PMEV_CASE(reg, 0, case_macro); \
> + PMEV_CASE(reg, 1, case_macro); \
> + PMEV_CASE(reg, 2, case_macro); \
> + PMEV_CASE(reg, 3, case_macro); \
> + PMEV_CASE(reg, 4, case_macro); \
> + PMEV_CASE(reg, 5, case_macro); \
> + PMEV_CASE(reg, 6, case_macro); \
> + PMEV_CASE(reg, 7, case_macro); \
> + default: \
> + WARN(1, "Invalid PMEV* event index\n"); \
> + } \
> + } while (0)
Please fix your editor, the indentation of the "\" is totally wrong.
> +
> +#define PMEV_REG_SWITCH(reg, ev, case_macro) \
> + do { \
> + switch (reg) { \
> + case 0: \
> + PMEV_EV_SWITCH(0, ev, case_macro); \
> + break; \
> + case 1: \
> + PMEV_EV_SWITCH(1, ev, case_macro); \
> + break; \
> + case 2: \
> + PMEV_EV_SWITCH(2, ev, case_macro); \
> + break; \
> + case 3: \
> + PMEV_EV_SWITCH(3, ev, case_macro); \
> + break; \
> + default: \
> + WARN(1, "Invalid PMEV* register index\n"); \
> + } \
> + } while (0)
No, please don't do that. This makes the whole thing unmaintainable to
macro abuse. It also makes the code generation absolutely horrible,
due to the switch nesting. Just look at the disassembly.
The right way to do that is to declare the registers, one after the
other, all 60 of them, and then use the arm64 big switch
*unchanged*. You could even share it between the two
architectures. The codegen is slightly bad (one big switch), and it is
now trivial to read.
See below for the actual change.
M.
diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
index 816873c74eda..4d483e055519 100644
--- a/arch/arm/include/asm/arm_pmuv3.h
+++ b/arch/arm/include/asm/arm_pmuv3.h
@@ -37,89 +37,132 @@
#define PMINTENCLR __ACCESS_CP15(c9, 0, c14, 2)
#define PMMIR __ACCESS_CP15(c9, 0, c14, 6)
#define PMCCFILTR __ACCESS_CP15(c14, 0, c15, 7)
-#define PMEVCNTR0(n) __ACCESS_CP15(c14, 0, c8, n)
-#define PMEVCNTR1(n) __ACCESS_CP15(c14, 0, c9, n)
-#define PMEVCNTR2(n) __ACCESS_CP15(c14, 0, c10, n)
-#define PMEVCNTR3(n) __ACCESS_CP15(c14, 0, c11, n)
-#define PMEVTYPER0(n) __ACCESS_CP15(c14, 0, c12, n)
-#define PMEVTYPER1(n) __ACCESS_CP15(c14, 0, c13, n)
-#define PMEVTYPER2(n) __ACCESS_CP15(c14, 0, c14, n)
-#define PMEVTYPER3(n) __ACCESS_CP15(c14, 0, c15, n)
-
-#define PMEV_EVENTS_PER_REG 8
-#define PMEV_REGISTER(n) (n / PMEV_EVENTS_PER_REG)
-#define PMEV_EVENT(n) (n % PMEV_EVENTS_PER_REG)
-
-#define PMEV_CASE(reg, ev, case_macro) \
- case ev: \
- case_macro(reg, ev); \
- break
-
-#define PMEV_EV_SWITCH(reg, ev, case_macro) \
- do { \
- switch (ev) { \
- PMEV_CASE(reg, 0, case_macro); \
- PMEV_CASE(reg, 1, case_macro); \
- PMEV_CASE(reg, 2, case_macro); \
- PMEV_CASE(reg, 3, case_macro); \
- PMEV_CASE(reg, 4, case_macro); \
- PMEV_CASE(reg, 5, case_macro); \
- PMEV_CASE(reg, 6, case_macro); \
- PMEV_CASE(reg, 7, case_macro); \
- default: \
- WARN(1, "Invalid PMEV* event index\n"); \
- } \
- } while (0)
-#define PMEV_REG_SWITCH(reg, ev, case_macro) \
- do { \
- switch (reg) { \
- case 0: \
- PMEV_EV_SWITCH(0, ev, case_macro); \
- break; \
- case 1: \
- PMEV_EV_SWITCH(1, ev, case_macro); \
- break; \
- case 2: \
- PMEV_EV_SWITCH(2, ev, case_macro); \
- break; \
- case 3: \
- PMEV_EV_SWITCH(3, ev, case_macro); \
- break; \
- default: \
- WARN(1, "Invalid PMEV* register index\n"); \
- } \
+#define PMEVCNTR0 __ACCESS_CP15(c14, 0, c8, 0)
+#define PMEVCNTR1 __ACCESS_CP15(c14, 0, c8, 1)
+#define PMEVCNTR2 __ACCESS_CP15(c14, 0, c8, 2)
+#define PMEVCNTR3 __ACCESS_CP15(c14, 0, c8, 3)
+#define PMEVCNTR4 __ACCESS_CP15(c14, 0, c8, 4)
+#define PMEVCNTR5 __ACCESS_CP15(c14, 0, c8, 5)
+#define PMEVCNTR6 __ACCESS_CP15(c14, 0, c8, 6)
+#define PMEVCNTR7 __ACCESS_CP15(c14, 0, c8, 7)
+#define PMEVCNTR8 __ACCESS_CP15(c14, 0, c9, 0)
+#define PMEVCNTR9 __ACCESS_CP15(c14, 0, c9, 1)
+#define PMEVCNTR10 __ACCESS_CP15(c14, 0, c9, 2)
+#define PMEVCNTR11 __ACCESS_CP15(c14, 0, c9, 3)
+#define PMEVCNTR12 __ACCESS_CP15(c14, 0, c9, 4)
+#define PMEVCNTR13 __ACCESS_CP15(c14, 0, c9, 5)
+#define PMEVCNTR14 __ACCESS_CP15(c14, 0, c9, 6)
+#define PMEVCNTR15 __ACCESS_CP15(c14, 0, c9, 7)
+#define PMEVCNTR16 __ACCESS_CP15(c14, 0, c10, 0)
+#define PMEVCNTR17 __ACCESS_CP15(c14, 0, c10, 1)
+#define PMEVCNTR18 __ACCESS_CP15(c14, 0, c10, 2)
+#define PMEVCNTR19 __ACCESS_CP15(c14, 0, c10, 3)
+#define PMEVCNTR20 __ACCESS_CP15(c14, 0, c10, 4)
+#define PMEVCNTR21 __ACCESS_CP15(c14, 0, c10, 5)
+#define PMEVCNTR22 __ACCESS_CP15(c14, 0, c10, 6)
+#define PMEVCNTR23 __ACCESS_CP15(c14, 0, c10, 7)
+#define PMEVCNTR24 __ACCESS_CP15(c14, 0, c11, 0)
+#define PMEVCNTR25 __ACCESS_CP15(c14, 0, c11, 1)
+#define PMEVCNTR26 __ACCESS_CP15(c14, 0, c11, 2)
+#define PMEVCNTR27 __ACCESS_CP15(c14, 0, c11, 3)
+#define PMEVCNTR28 __ACCESS_CP15(c14, 0, c11, 4)
+#define PMEVCNTR29 __ACCESS_CP15(c14, 0, c11, 5)
+#define PMEVCNTR30 __ACCESS_CP15(c14, 0, c11, 6)
+
+#define PMEVTYPER0 __ACCESS_CP15(c14, 0, c12, 0)
+#define PMEVTYPER1 __ACCESS_CP15(c14, 0, c12, 1)
+#define PMEVTYPER2 __ACCESS_CP15(c14, 0, c12, 2)
+#define PMEVTYPER3 __ACCESS_CP15(c14, 0, c12, 3)
+#define PMEVTYPER4 __ACCESS_CP15(c14, 0, c12, 4)
+#define PMEVTYPER5 __ACCESS_CP15(c14, 0, c12, 5)
+#define PMEVTYPER6 __ACCESS_CP15(c14, 0, c12, 6)
+#define PMEVTYPER7 __ACCESS_CP15(c14, 0, c12, 7)
+#define PMEVTYPER8 __ACCESS_CP15(c14, 0, c13, 0)
+#define PMEVTYPER9 __ACCESS_CP15(c14, 0, c13, 1)
+#define PMEVTYPER10 __ACCESS_CP15(c14, 0, c13, 2)
+#define PMEVTYPER11 __ACCESS_CP15(c14, 0, c13, 3)
+#define PMEVTYPER12 __ACCESS_CP15(c14, 0, c13, 4)
+#define PMEVTYPER13 __ACCESS_CP15(c14, 0, c13, 5)
+#define PMEVTYPER14 __ACCESS_CP15(c14, 0, c13, 6)
+#define PMEVTYPER15 __ACCESS_CP15(c14, 0, c13, 7)
+#define PMEVTYPER16 __ACCESS_CP15(c14, 0, c14, 0)
+#define PMEVTYPER17 __ACCESS_CP15(c14, 0, c14, 1)
+#define PMEVTYPER18 __ACCESS_CP15(c14, 0, c14, 2)
+#define PMEVTYPER19 __ACCESS_CP15(c14, 0, c14, 3)
+#define PMEVTYPER20 __ACCESS_CP15(c14, 0, c14, 4)
+#define PMEVTYPER21 __ACCESS_CP15(c14, 0, c14, 5)
+#define PMEVTYPER22 __ACCESS_CP15(c14, 0, c14, 6)
+#define PMEVTYPER23 __ACCESS_CP15(c14, 0, c14, 7)
+#define PMEVTYPER24 __ACCESS_CP15(c14, 0, c15, 0)
+#define PMEVTYPER25 __ACCESS_CP15(c14, 0, c15, 1)
+#define PMEVTYPER26 __ACCESS_CP15(c14, 0, c15, 2)
+#define PMEVTYPER27 __ACCESS_CP15(c14, 0, c15, 3)
+#define PMEVTYPER28 __ACCESS_CP15(c14, 0, c15, 4)
+#define PMEVTYPER29 __ACCESS_CP15(c14, 0, c15, 5)
+#define PMEVTYPER30 __ACCESS_CP15(c14, 0, c15, 6)
+
+#define PMEVN_CASE(n, case_macro) \
+ case n: case_macro(n); break
+
+#define PMEVN_SWITCH(x, case_macro) \
+ do { \
+ switch (x) { \
+ PMEVN_CASE(0, case_macro); \
+ PMEVN_CASE(1, case_macro); \
+ PMEVN_CASE(2, case_macro); \
+ PMEVN_CASE(3, case_macro); \
+ PMEVN_CASE(4, case_macro); \
+ PMEVN_CASE(5, case_macro); \
+ PMEVN_CASE(6, case_macro); \
+ PMEVN_CASE(7, case_macro); \
+ PMEVN_CASE(8, case_macro); \
+ PMEVN_CASE(9, case_macro); \
+ PMEVN_CASE(10, case_macro); \
+ PMEVN_CASE(11, case_macro); \
+ PMEVN_CASE(12, case_macro); \
+ PMEVN_CASE(13, case_macro); \
+ PMEVN_CASE(14, case_macro); \
+ PMEVN_CASE(15, case_macro); \
+ PMEVN_CASE(16, case_macro); \
+ PMEVN_CASE(17, case_macro); \
+ PMEVN_CASE(18, case_macro); \
+ PMEVN_CASE(19, case_macro); \
+ PMEVN_CASE(20, case_macro); \
+ PMEVN_CASE(21, case_macro); \
+ PMEVN_CASE(22, case_macro); \
+ PMEVN_CASE(23, case_macro); \
+ PMEVN_CASE(24, case_macro); \
+ PMEVN_CASE(25, case_macro); \
+ PMEVN_CASE(26, case_macro); \
+ PMEVN_CASE(27, case_macro); \
+ PMEVN_CASE(28, case_macro); \
+ PMEVN_CASE(29, case_macro); \
+ PMEVN_CASE(30, case_macro); \
+ default: WARN(1, "Invalid PMEV* index\n"); \
+ } \
} while (0)
-#define RETURN_READ_PMEVCNTR(reg, ev) \
- return read_sysreg(PMEVCNTR##reg(ev))
+#define RETURN_READ_PMEVCNTRN(n) \
+ return read_sysreg(PMEVCNTR##n)
static unsigned long read_pmevcntrn(int n)
{
- const int reg = PMEV_REGISTER(n);
- const int event = PMEV_EVENT(n);
-
- PMEV_REG_SWITCH(reg, event, RETURN_READ_PMEVCNTR);
+ PMEVN_SWITCH(n, RETURN_READ_PMEVCNTRN);
return 0;
}
-#define WRITE_PMEVCNTR(reg, ev) \
- write_sysreg(val, PMEVCNTR##reg(ev))
+#define WRITE_PMEVCNTRN(n) \
+ write_sysreg(val, PMEVCNTR##n)
static void write_pmevcntrn(int n, unsigned long val)
{
- const int reg = PMEV_REGISTER(n);
- const int event = PMEV_EVENT(n);
-
- PMEV_REG_SWITCH(reg, event, WRITE_PMEVCNTR);
+ PMEVN_SWITCH(n, WRITE_PMEVCNTRN);
}
-#define WRITE_PMEVTYPER(reg, ev) \
- write_sysreg(val, PMEVTYPER##reg(ev))
+#define WRITE_PMEVTYPERN(n) \
+ write_sysreg(val, PMEVTYPER##n)
static void write_pmevtypern(int n, unsigned long val)
{
- const int reg = PMEV_REGISTER(n);
- const int event = PMEV_EVENT(n);
-
- PMEV_REG_SWITCH(reg, event, WRITE_PMEVTYPER);
+ PMEVN_SWITCH(n, WRITE_PMEVTYPERN);
}
static inline unsigned long read_pmmir(void)
--
Without deviation from the norm, progress is not possible.
On Thu, 26 Jan 2023 20:44:36 +0000,
Zaid Al-Bassam <[email protected]> wrote:
>
> Currently, PMUv3 driver is only available for ARMv8 aarch64
> platforms, ARMv8 running in aarch32 mode dont have access to
> the driver. This is, especially, a problem for ARMv8 platforms
> that only have aarch32 support, like the Cortex-A32.
FWIW, I've pushed out my own interpretation of this series to [1],
including the changes I suggested. The results seems to correctly work
in a 32bit guest on a 64bit host, which is what I (used to) care
about.
Feel free to pick it up.
M.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=arm/pmuv3
--
Without deviation from the norm, progress is not possible.
On 2/8/23 08:40, Marc Zyngier wrote:
> On Thu, 26 Jan 2023 20:44:36 +0000,
> Zaid Al-Bassam <[email protected]> wrote:
>>
>> Currently, PMUv3 driver is only available for ARMv8 aarch64
>> platforms, ARMv8 running in aarch32 mode dont have access to
>> the driver. This is, especially, a problem for ARMv8 platforms
>> that only have aarch32 support, like the Cortex-A32.
>
> FWIW, I've pushed out my own interpretation of this series to [1],
> including the changes I suggested. The results seems to correctly work
> in a 32bit guest on a 64bit host, which is what I (used to) care
> about.
>
> Feel free to pick it up.
Tested-by: Florian Fainelli <[email protected]>
Thanks!
--
Florian
Thank you Marc for reviewing the change and fixing the issues, I have
picked up your patches and sent out v2. Tested on Cortex-A32.
Best Regards,
Zaid Al-Bassam
On Wed, Feb 8, 2023 at 11:40 AM Marc Zyngier <[email protected]> wrote:
>
> On Thu, 26 Jan 2023 20:44:36 +0000,
> Zaid Al-Bassam <[email protected]> wrote:
> >
> > Currently, PMUv3 driver is only available for ARMv8 aarch64
> > platforms, ARMv8 running in aarch32 mode dont have access to
> > the driver. This is, especially, a problem for ARMv8 platforms
> > that only have aarch32 support, like the Cortex-A32.
>
> FWIW, I've pushed out my own interpretation of this series to [1],
> including the changes I suggested. The results seems to correctly work
> in a 32bit guest on a 64bit host, which is what I (used to) care
> about.
>
> Feel free to pick it up.
>
> M.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=arm/pmuv3
>
> --
> Without deviation from the norm, progress is not possible.