kvmarm.lists.cs.columbia.edu archive mirror
 help / color / mirror / Atom feed
* [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters
@ 2020-03-02 18:17 Mark Rutland
  2020-03-02 18:17 ` [PATCHv6 1/3] arm64: cpufeature: Extract capped perfmon fields Mark Rutland
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Mark Rutland @ 2020-03-02 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: catalin.marinas, will, kvmarm, maz

This is a respin of Andrew Murray's series to enable support for 64-bit
counters as introduced in ARMv8.5.

I've given this a spin on (ARMv8.2) hardware, to test that there are no
regressions, but I have not had the chance to test in an ARMv8.5 model (which I
beleive Andrew had previously tested).

Since v5 [1]:
* Don't treat perfmon ID fields as signed
* Fix up ID field names
* Explicitly compare ARMV8.5 PMU value

[1] https://lkml.kernel.org/r/1580125469-23887-1-git-send-email-andrew.murray@arm.com

Thanks,
Mark.

Andrew Murray (3):
  arm64: cpufeature: Extract capped perfmon fields
  KVM: arm64: limit PMU version to PMUv3 for ARMv8.1
  arm64: perf: Add support for ARMv8.5-PMU 64-bit counters

 arch/arm64/include/asm/cpufeature.h | 23 ++++++++++
 arch/arm64/include/asm/perf_event.h |  3 +-
 arch/arm64/include/asm/sysreg.h     | 10 +++++
 arch/arm64/kernel/perf_event.c      | 86 +++++++++++++++++++++++++++++--------
 arch/arm64/kvm/sys_regs.c           | 10 +++++
 include/linux/perf/arm_pmu.h        |  1 +
 6 files changed, 115 insertions(+), 18 deletions(-)

-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCHv6 1/3] arm64: cpufeature: Extract capped perfmon fields
  2020-03-02 18:17 [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
@ 2020-03-02 18:17 ` Mark Rutland
  2020-03-02 18:17 ` [PATCHv6 2/3] KVM: arm64: limit PMU version to PMUv3 for ARMv8.1 Mark Rutland
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2020-03-02 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: catalin.marinas, will, kvmarm, maz

From: Andrew Murray <andrew.murray@arm.com>

When emulating ID registers there is often a need to cap the version
bits of a feature such that the guest will not use features that the
host is not aware of. For example, when KVM mediates access to the PMU
by emulating register accesses.

Let's add a helper that extracts a performance monitors ID field and
caps the version to a given value.

Fields that identify the version of the Performance Monitors Extension
do not follow the standard ID scheme, and instead follow the scheme
described in ARM DDI 0487E.a page D13-2825 "Alternative ID scheme used
for the Performance Monitors Extension version". The value 0xF means an
IMPLEMENTATION DEFINED PMU is present, and values 0x0-OxE can be treated
the same as an unsigned field with 0x0 meaning no PMU is present.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
[Mark: rework to handle perfmon fields]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 92ef9539874a..186f4e19207e 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -447,6 +447,29 @@ cpuid_feature_extract_unsigned_field(u64 features, int field)
 	return cpuid_feature_extract_unsigned_field_width(features, field, 4);
 }
 
+/*
+ * Fields that identify the version of the Performance Monitors Extension do
+ * not follow the standard ID scheme. See ARM DDI 0487E.a page D13-2825,
+ * "Alternative ID scheme used for the Performance Monitors Extension version".
+ */
+static inline u64 __attribute_const__
+cpuid_feature_cap_perfmon_field(u64 features, int field, u64 cap)
+{
+	u64 val = cpuid_feature_extract_unsigned_field(features, field);
+	u64 mask = GENMASK_ULL(field + 3, field);
+
+	/* Treat IMPLEMENTATION DEFINED functionality as unimplemented */
+	if (val == 0xf)
+		val = 0;
+
+	if (val > cap) {
+		features &= ~mask;
+		features |= (cap << field) & mask;
+	}
+
+	return features;
+}
+
 static inline u64 arm64_ftr_mask(const struct arm64_ftr_bits *ftrp)
 {
 	return (u64)GENMASK(ftrp->shift + ftrp->width - 1, ftrp->shift);
-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCHv6 2/3] KVM: arm64: limit PMU version to PMUv3 for ARMv8.1
  2020-03-02 18:17 [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
  2020-03-02 18:17 ` [PATCHv6 1/3] arm64: cpufeature: Extract capped perfmon fields Mark Rutland
@ 2020-03-02 18:17 ` Mark Rutland
  2020-03-02 18:17 ` [PATCHv6 3/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2020-03-02 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: catalin.marinas, will, kvmarm, maz

From: Andrew Murray <andrew.murray@arm.com>

We currently expose the PMU version of the host to the guest via
emulation of the DFR0_EL1 and AA64DFR0_EL1 debug feature registers.
However many of the features offered beyond PMUv3 for 8.1 are not
supported in KVM. Examples of this include support for the PMMIR
registers (added in PMUv3 for ARMv8.4) and 64-bit event counters
added in (PMUv3 for ARMv8.5).

Let's trap the Debug Feature Registers in order to limit
PMUVer/PerfMon in the Debug Feature Registers to PMUv3 for ARMv8.1
to avoid unexpected behaviour.

Both ID_AA64DFR0.PMUVer and ID_DFR0.PerfMon follow the "Alternative ID
scheme used for the Performance Monitors Extension version" where 0xF
means an IMPLEMENTATION DEFINED PMU is implemented, and values 0x0-0xE
are treated as with an unsigned field (with 0x0 meaning no PMU is
present). As we don't expect to expose an IMPLEMENTATION DEFINED PMU,
and our cap is below 0xF, we can treat these fields as unsigned when
applying the cap.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
[Mark: make field names consistent, use perfmon cap]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/sysreg.h |  6 ++++++
 arch/arm64/kvm/sys_regs.c       | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index b91570ff9db1..d8f1eed070f0 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -702,6 +702,12 @@
 #define ID_AA64DFR0_TRACEVER_SHIFT	4
 #define ID_AA64DFR0_DEBUGVER_SHIFT	0
 
+#define ID_AA64DFR0_PMUVER_8_1		0x4
+
+#define ID_DFR0_PERFMON_SHIFT		24
+
+#define ID_DFR0_PERFMON_8_1		0x4
+
 #define ID_ISAR5_RDM_SHIFT		24
 #define ID_ISAR5_CRC32_SHIFT		16
 #define ID_ISAR5_SHA2_SHIFT		12
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 3e909b117f0c..b0a3e8976b90 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1085,6 +1085,16 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 			 (0xfUL << ID_AA64ISAR1_API_SHIFT) |
 			 (0xfUL << ID_AA64ISAR1_GPA_SHIFT) |
 			 (0xfUL << ID_AA64ISAR1_GPI_SHIFT));
+	} else if (id == SYS_ID_AA64DFR0_EL1) {
+		/* Limit guests to PMUv3 for ARMv8.1 */
+		val = cpuid_feature_cap_perfmon_field(val,
+						ID_AA64DFR0_PMUVER_SHIFT,
+						ID_AA64DFR0_PMUVER_8_1);
+	} else if (id == SYS_ID_DFR0_EL1) {
+		/* Limit guests to PMUv3 for ARMv8.1 */
+		val = cpuid_feature_cap_perfmon_field(val,
+						ID_DFR0_PERFMON_SHIFT,
+						ID_DFR0_PERFMON_8_1);
 	}
 
 	return val;
-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCHv6 3/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters
  2020-03-02 18:17 [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
  2020-03-02 18:17 ` [PATCHv6 1/3] arm64: cpufeature: Extract capped perfmon fields Mark Rutland
  2020-03-02 18:17 ` [PATCHv6 2/3] KVM: arm64: limit PMU version to PMUv3 for ARMv8.1 Mark Rutland
@ 2020-03-02 18:17 ` Mark Rutland
  2020-03-03 19:07 ` [PATCHv6 0/3] " Mark Rutland
  2020-03-17 23:14 ` Will Deacon
  4 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2020-03-02 18:17 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: catalin.marinas, will, kvmarm, maz

From: Andrew Murray <andrew.murray@arm.com>

At present ARMv8 event counters are limited to 32-bits, though by
using the CHAIN event it's possible to combine adjacent counters to
achieve 64-bits. The perf config1:0 bit can be set to use such a
configuration.

With the introduction of ARMv8.5-PMU support, all event counters can
now be used as 64-bit counters.

Let's enable 64-bit event counters where support exists. Unless the
user sets config1:0 we will adjust the counter value such that it
overflows upon 32-bit overflow. This follows the same behaviour as
the cycle counter which has always been (and remains) 64-bits.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
[Mark: fix ID field names, compare with 8.5 value]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/perf_event.h |  3 +-
 arch/arm64/include/asm/sysreg.h     |  4 ++
 arch/arm64/kernel/perf_event.c      | 86 +++++++++++++++++++++++++++++--------
 include/linux/perf/arm_pmu.h        |  1 +
 4 files changed, 76 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h
index 2bdbc79bbd01..e7765b62c712 100644
--- a/arch/arm64/include/asm/perf_event.h
+++ b/arch/arm64/include/asm/perf_event.h
@@ -176,9 +176,10 @@
 #define ARMV8_PMU_PMCR_X	(1 << 4) /* Export to ETM */
 #define ARMV8_PMU_PMCR_DP	(1 << 5) /* Disable CCNT if non-invasive debug*/
 #define ARMV8_PMU_PMCR_LC	(1 << 6) /* Overflow on 64 bit cycle counter */
+#define ARMV8_PMU_PMCR_LP	(1 << 7) /* Long event counter enable */
 #define	ARMV8_PMU_PMCR_N_SHIFT	11	 /* Number of counters supported */
 #define	ARMV8_PMU_PMCR_N_MASK	0x1f
-#define	ARMV8_PMU_PMCR_MASK	0x7f	 /* Mask for writable bits */
+#define	ARMV8_PMU_PMCR_MASK	0xff	 /* Mask for writable bits */
 
 /*
  * PMOVSR: counters overflow flag status reg
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index d8f1eed070f0..9b66c5b5b36f 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -702,7 +702,11 @@
 #define ID_AA64DFR0_TRACEVER_SHIFT	4
 #define ID_AA64DFR0_DEBUGVER_SHIFT	0
 
+#define ID_AA64DFR0_PMUVER_8_0		0x1
 #define ID_AA64DFR0_PMUVER_8_1		0x4
+#define ID_AA64DFR0_PMUVER_8_4		0x5
+#define ID_AA64DFR0_PMUVER_8_5		0x6
+#define ID_AA64DFR0_PMUVER_IMP_DEF	0xf
 
 #define ID_DFR0_PERFMON_SHIFT		24
 
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index e40b65645c86..670a4882c868 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -285,6 +285,17 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
 #define	ARMV8_IDX_COUNTER_LAST(cpu_pmu) \
 	(ARMV8_IDX_CYCLE_COUNTER + cpu_pmu->num_events - 1)
 
+
+/*
+ * We unconditionally enable ARMv8.5-PMU long event counter support
+ * (64-bit events) where supported. Indicate if this arm_pmu has long
+ * event counter support.
+ */
+static bool armv8pmu_has_long_event(struct arm_pmu *cpu_pmu)
+{
+	return (cpu_pmu->pmuver >= ID_AA64DFR0_PMUVER_8_5);
+}
+
 /*
  * We must chain two programmable counters for 64 bit events,
  * except when we have allocated the 64bit cycle counter (for CPU
@@ -294,9 +305,11 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
 static inline bool armv8pmu_event_is_chained(struct perf_event *event)
 {
 	int idx = event->hw.idx;
+	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 
 	return !WARN_ON(idx < 0) &&
 	       armv8pmu_event_is_64bit(event) &&
+	       !armv8pmu_has_long_event(cpu_pmu) &&
 	       (idx != ARMV8_IDX_CYCLE_COUNTER);
 }
 
@@ -345,7 +358,7 @@ static inline void armv8pmu_select_counter(int idx)
 	isb();
 }
 
-static inline u32 armv8pmu_read_evcntr(int idx)
+static inline u64 armv8pmu_read_evcntr(int idx)
 {
 	armv8pmu_select_counter(idx);
 	return read_sysreg(pmxevcntr_el0);
@@ -362,6 +375,44 @@ static inline u64 armv8pmu_read_hw_counter(struct perf_event *event)
 	return val;
 }
 
+/*
+ * The cycle counter is always a 64-bit counter. When ARMV8_PMU_PMCR_LP
+ * is set the event counters also become 64-bit counters. Unless the
+ * user has requested a long counter (attr.config1) then we want to
+ * interrupt upon 32-bit overflow - we achieve this by applying a bias.
+ */
+static bool armv8pmu_event_needs_bias(struct perf_event *event)
+{
+	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	int idx = hwc->idx;
+
+	if (armv8pmu_event_is_64bit(event))
+		return false;
+
+	if (armv8pmu_has_long_event(cpu_pmu) ||
+	    idx == ARMV8_IDX_CYCLE_COUNTER)
+		return true;
+
+	return false;
+}
+
+static u64 armv8pmu_bias_long_counter(struct perf_event *event, u64 value)
+{
+	if (armv8pmu_event_needs_bias(event))
+		value |= GENMASK(63, 32);
+
+	return value;
+}
+
+static u64 armv8pmu_unbias_long_counter(struct perf_event *event, u64 value)
+{
+	if (armv8pmu_event_needs_bias(event))
+		value &= ~GENMASK(63, 32);
+
+	return value;
+}
+
 static u64 armv8pmu_read_counter(struct perf_event *event)
 {
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
@@ -377,10 +428,10 @@ static u64 armv8pmu_read_counter(struct perf_event *event)
 	else
 		value = armv8pmu_read_hw_counter(event);
 
-	return value;
+	return  armv8pmu_unbias_long_counter(event, value);
 }
 
-static inline void armv8pmu_write_evcntr(int idx, u32 value)
+static inline void armv8pmu_write_evcntr(int idx, u64 value)
 {
 	armv8pmu_select_counter(idx);
 	write_sysreg(value, pmxevcntr_el0);
@@ -405,20 +456,14 @@ static void armv8pmu_write_counter(struct perf_event *event, u64 value)
 	struct hw_perf_event *hwc = &event->hw;
 	int idx = hwc->idx;
 
+	value = armv8pmu_bias_long_counter(event, value);
+
 	if (!armv8pmu_counter_valid(cpu_pmu, idx))
 		pr_err("CPU%u writing wrong counter %d\n",
 			smp_processor_id(), idx);
-	else if (idx == ARMV8_IDX_CYCLE_COUNTER) {
-		/*
-		 * The cycles counter is really a 64-bit counter.
-		 * When treating it as a 32-bit counter, we only count
-		 * the lower 32 bits, and set the upper 32-bits so that
-		 * we get an interrupt upon 32-bit overflow.
-		 */
-		if (!armv8pmu_event_is_64bit(event))
-			value |= 0xffffffff00000000ULL;
+	else if (idx == ARMV8_IDX_CYCLE_COUNTER)
 		write_sysreg(value, pmccntr_el0);
-	} else
+	else
 		armv8pmu_write_hw_counter(event, value);
 }
 
@@ -743,7 +788,8 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
 	/*
 	 * Otherwise use events counters
 	 */
-	if (armv8pmu_event_is_64bit(event))
+	if (armv8pmu_event_is_64bit(event) &&
+	    !armv8pmu_has_long_event(cpu_pmu))
 		return	armv8pmu_get_chain_idx(cpuc, cpu_pmu);
 	else
 		return armv8pmu_get_single_idx(cpuc, cpu_pmu);
@@ -815,7 +861,7 @@ static int armv8pmu_filter_match(struct perf_event *event)
 static void armv8pmu_reset(void *info)
 {
 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)info;
-	u32 idx, nb_cnt = cpu_pmu->num_events;
+	u32 idx, pmcr, nb_cnt = cpu_pmu->num_events;
 
 	/* The counter and interrupt enable registers are unknown at reset. */
 	for (idx = ARMV8_IDX_CYCLE_COUNTER; idx < nb_cnt; ++idx) {
@@ -830,8 +876,13 @@ static void armv8pmu_reset(void *info)
 	 * Initialize & Reset PMNC. Request overflow interrupt for
 	 * 64 bit cycle counter but cheat in armv8pmu_write_counter().
 	 */
-	armv8pmu_pmcr_write(ARMV8_PMU_PMCR_P | ARMV8_PMU_PMCR_C |
-			    ARMV8_PMU_PMCR_LC);
+	pmcr = ARMV8_PMU_PMCR_P | ARMV8_PMU_PMCR_C | ARMV8_PMU_PMCR_LC;
+
+	/* Enable long event counter support where available */
+	if (armv8pmu_has_long_event(cpu_pmu))
+		pmcr |= ARMV8_PMU_PMCR_LP;
+
+	armv8pmu_pmcr_write(pmcr);
 }
 
 static int __armv8_pmuv3_map_event(struct perf_event *event,
@@ -914,6 +965,7 @@ static void __armv8pmu_probe_pmu(void *info)
 	if (pmuver == 0xf || pmuver == 0)
 		return;
 
+	cpu_pmu->pmuver = pmuver;
 	probe->present = true;
 
 	/* Read the nb of CNTx counters supported from PMNC */
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 71f525a35ac2..5b616dde9a4c 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -80,6 +80,7 @@ struct arm_pmu {
 	struct pmu	pmu;
 	cpumask_t	supported_cpus;
 	char		*name;
+	int		pmuver;
 	irqreturn_t	(*handle_irq)(struct arm_pmu *pmu);
 	void		(*enable)(struct perf_event *event);
 	void		(*disable)(struct perf_event *event);
-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters
  2020-03-02 18:17 [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
                   ` (2 preceding siblings ...)
  2020-03-02 18:17 ` [PATCHv6 3/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
@ 2020-03-03 19:07 ` Mark Rutland
  2020-03-10 17:28   ` Robin Murphy
  2020-03-17 23:14 ` Will Deacon
  4 siblings, 1 reply; 8+ messages in thread
From: Mark Rutland @ 2020-03-03 19:07 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: catalin.marinas, will, kvmarm, maz

On Mon, Mar 02, 2020 at 06:17:49PM +0000, Mark Rutland wrote:
> This is a respin of Andrew Murray's series to enable support for 64-bit
> counters as introduced in ARMv8.5.
> 
> I've given this a spin on (ARMv8.2) hardware, to test that there are no
> regressions, but I have not had the chance to test in an ARMv8.5 model (which I
> beleive Andrew had previously tested).

Bad news; this is broken. :(

While perf-stat works as expected, perf-record doesn't get samples for
any of the programmable counters.

In ARMv8.4 mode I can do:

| / # perf record -a -c 1 -e armv8_pmuv3/inst_retired/ true
| [ perf record: Woken up 1 times to write data ]
| [ perf record: Captured and wrote 0.023 MB perf.data (367 samples) ]
| / # perf record -a -c 1 -e armv8_pmuv3/inst_retired,long/ true
| [ perf record: Woken up 1 times to write data ]
| [ perf record: Captured and wrote 0.022 MB perf.data (353 samples) ]

... so regular 32-bit and chained events work correctly.

But in ARMv8.5 mode I get no samples in either case:

| / # perf record -a -c 1 -e armv8_pmuv3/inst_retired/ true
| [ perf record: Woken up 1 times to write data ]
| [ perf record: Captured and wrote 0.008 MB perf.data ]
| / # perf report | grep samples
| Error:
| The perf.data file has no samples!
| / # perf record -a -c 1 -e armv8_pmuv3/inst_retired,long/ true
| [ perf record: Woken up 1 times to write data ]
| [ perf record: Captured and wrote 0.008 MB perf.data ]
| / # perf report | grep samples
| Error:
| The perf.data file has no samples!

I'll have to trace the driver to see what's going on. I suspect we've
missed some bias handling, but it's possible that this is a model bug.

Thanks,
Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters
  2020-03-03 19:07 ` [PATCHv6 0/3] " Mark Rutland
@ 2020-03-10 17:28   ` Robin Murphy
  0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2020-03-10 17:28 UTC (permalink / raw)
  To: Mark Rutland, linux-arm-kernel; +Cc: catalin.marinas, will, kvmarm, maz

On 03/03/2020 7:07 pm, Mark Rutland wrote:
> On Mon, Mar 02, 2020 at 06:17:49PM +0000, Mark Rutland wrote:
>> This is a respin of Andrew Murray's series to enable support for 64-bit
>> counters as introduced in ARMv8.5.
>>
>> I've given this a spin on (ARMv8.2) hardware, to test that there are no
>> regressions, but I have not had the chance to test in an ARMv8.5 model (which I
>> beleive Andrew had previously tested).
> 
> Bad news; this is broken. :(
> 
> While perf-stat works as expected, perf-record doesn't get samples for
> any of the programmable counters.
> 
> In ARMv8.4 mode I can do:
> 
> | / # perf record -a -c 1 -e armv8_pmuv3/inst_retired/ true
> | [ perf record: Woken up 1 times to write data ]
> | [ perf record: Captured and wrote 0.023 MB perf.data (367 samples) ]
> | / # perf record -a -c 1 -e armv8_pmuv3/inst_retired,long/ true
> | [ perf record: Woken up 1 times to write data ]
> | [ perf record: Captured and wrote 0.022 MB perf.data (353 samples) ]
> 
> ... so regular 32-bit and chained events work correctly.
> 
> But in ARMv8.5 mode I get no samples in either case:
> 
> | / # perf record -a -c 1 -e armv8_pmuv3/inst_retired/ true
> | [ perf record: Woken up 1 times to write data ]
> | [ perf record: Captured and wrote 0.008 MB perf.data ]
> | / # perf report | grep samples
> | Error:
> | The perf.data file has no samples!
> | / # perf record -a -c 1 -e armv8_pmuv3/inst_retired,long/ true
> | [ perf record: Woken up 1 times to write data ]
> | [ perf record: Captured and wrote 0.008 MB perf.data ]
> | / # perf report | grep samples
> | Error:
> | The perf.data file has no samples!
> 
> I'll have to trace the driver to see what's going on. I suspect we've
> missed some bias handling, but it's possible that this is a model bug.

For the record, further evidence has indeed pointed to there being a bug 
in the model's implementation of ARMv8.5-PMU. It's been raised with the 
models team, so we'll have wait and see what they say...

Robin.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters
  2020-03-02 18:17 [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
                   ` (3 preceding siblings ...)
  2020-03-03 19:07 ` [PATCHv6 0/3] " Mark Rutland
@ 2020-03-17 23:14 ` Will Deacon
  2020-03-18 10:51   ` Mark Rutland
  4 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2020-03-17 23:14 UTC (permalink / raw)
  To: Mark Rutland; +Cc: catalin.marinas, robin.murphy, kvmarm, linux-arm-kernel, maz

On Mon, Mar 02, 2020 at 06:17:49PM +0000, Mark Rutland wrote:
> This is a respin of Andrew Murray's series to enable support for 64-bit
> counters as introduced in ARMv8.5.
> 
> I've given this a spin on (ARMv8.2) hardware, to test that there are no
> regressions, but I have not had the chance to test in an ARMv8.5 model (which I
> beleive Andrew had previously tested).
> 
> Since v5 [1]:
> * Don't treat perfmon ID fields as signed
> * Fix up ID field names
> * Explicitly compare ARMV8.5 PMU value

I'm betting on your issue being a model bug, so I've queued this on top of
Robin's enable/disable rework. Please take a look at for-next/perf [1] in
case I screwed it up.

Thanks,

Will

[1] https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-next/perf
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters
  2020-03-17 23:14 ` Will Deacon
@ 2020-03-18 10:51   ` Mark Rutland
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2020-03-18 10:51 UTC (permalink / raw)
  To: Will Deacon; +Cc: catalin.marinas, robin.murphy, kvmarm, linux-arm-kernel, maz

On Tue, Mar 17, 2020 at 11:14:31PM +0000, Will Deacon wrote:
> On Mon, Mar 02, 2020 at 06:17:49PM +0000, Mark Rutland wrote:
> > This is a respin of Andrew Murray's series to enable support for 64-bit
> > counters as introduced in ARMv8.5.
> > 
> > I've given this a spin on (ARMv8.2) hardware, to test that there are no
> > regressions, but I have not had the chance to test in an ARMv8.5 model (which I
> > beleive Andrew had previously tested).
> > 
> > Since v5 [1]:
> > * Don't treat perfmon ID fields as signed
> > * Fix up ID field names
> > * Explicitly compare ARMV8.5 PMU value
> 
> I'm betting on your issue being a model bug, so I've queued this on top of
> Robin's enable/disable rework. Please take a look at for-next/perf [1] in
> case I screwed it up.

From a cursory review, that all looks good to me.

I'll poke if the issue turns out to be anything beyond a model bug. but
I agree it seems that's all it is.

Thanks,
Mark.

> 
> Thanks,
> 
> Will
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-next/perf
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-18 10:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-02 18:17 [PATCHv6 0/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
2020-03-02 18:17 ` [PATCHv6 1/3] arm64: cpufeature: Extract capped perfmon fields Mark Rutland
2020-03-02 18:17 ` [PATCHv6 2/3] KVM: arm64: limit PMU version to PMUv3 for ARMv8.1 Mark Rutland
2020-03-02 18:17 ` [PATCHv6 3/3] arm64: perf: Add support for ARMv8.5-PMU 64-bit counters Mark Rutland
2020-03-03 19:07 ` [PATCHv6 0/3] " Mark Rutland
2020-03-10 17:28   ` Robin Murphy
2020-03-17 23:14 ` Will Deacon
2020-03-18 10:51   ` Mark Rutland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).