| Message ID | 20240702163515.1964784-2-alex.bennee@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Some fixes for running under -cpu max on QEMU | expand |
On 2024/7/3 0:35, Alex Bennée wrote: > The test for number of events is not a substitute for properly > checking the feature register. Fix the define and skip if PMUv3 is not > available on the system. This includes emulator such as QEMU which > don't implement PMU counters as a matter of policy. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Anders Roxell <anders.roxell@linaro.org> > --- > arm/pmu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arm/pmu.c b/arm/pmu.c > index 9ff7a301..66163a40 100644 > --- a/arm/pmu.c > +++ b/arm/pmu.c > @@ -200,7 +200,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits) {} > #define ID_AA64DFR0_PERFMON_MASK 0xf > > #define ID_DFR0_PMU_NOTIMPL 0b0000 > -#define ID_DFR0_PMU_V3 0b0001 > +#define ID_DFR0_PMU_V3 0b0011 Why? This is a macro used for AArch64 and DDI0487J.a (D19.2.59, the description of the PMUVer field) says that "0b0001 Performance Monitors Extension, PMUv3 implemented." while 0b0011 is a reserved value. Thanks, Zenghui
On 2024-07-03 08:09, Zenghui Yu wrote: > On 2024/7/3 0:35, Alex Bennée wrote: >> The test for number of events is not a substitute for properly >> checking the feature register. Fix the define and skip if PMUv3 is not >> available on the system. This includes emulator such as QEMU which >> don't implement PMU counters as a matter of policy. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: Anders Roxell <anders.roxell@linaro.org> >> --- >> arm/pmu.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arm/pmu.c b/arm/pmu.c >> index 9ff7a301..66163a40 100644 >> --- a/arm/pmu.c >> +++ b/arm/pmu.c >> @@ -200,7 +200,7 @@ static void test_overflow_interrupt(bool >> overflow_at_64bits) {} >> #define ID_AA64DFR0_PERFMON_MASK 0xf >> #define ID_DFR0_PMU_NOTIMPL 0b0000 >> -#define ID_DFR0_PMU_V3 0b0001 >> +#define ID_DFR0_PMU_V3 0b0011 > > Why? This is a macro used for AArch64 and DDI0487J.a (D19.2.59, the > description of the PMUVer field) says that > > "0b0001 Performance Monitors Extension, PMUv3 implemented." > > while 0b0011 is a reserved value. I think this is a mix of 32bit and 64bit views (ID_DFR0_EL1.PerfMon instead of ID_AA64DFR0_EL1.PMUVer), and the whole thing is a mess (ID_AA64DFR0_PERFMON_MASK is clearly confused...). I haven't looked at how this patch fits in the rest of the code though. M.
Marc Zyngier <maz@kernel.org> writes: > On 2024-07-03 08:09, Zenghui Yu wrote: >> On 2024/7/3 0:35, Alex Bennée wrote: >>> The test for number of events is not a substitute for properly >>> checking the feature register. Fix the define and skip if PMUv3 is not >>> available on the system. This includes emulator such as QEMU which >>> don't implement PMU counters as a matter of policy. >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> Cc: Anders Roxell <anders.roxell@linaro.org> >>> --- >>> arm/pmu.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> diff --git a/arm/pmu.c b/arm/pmu.c >>> index 9ff7a301..66163a40 100644 >>> --- a/arm/pmu.c >>> +++ b/arm/pmu.c >>> @@ -200,7 +200,7 @@ static void test_overflow_interrupt(bool >>> overflow_at_64bits) {} >>> #define ID_AA64DFR0_PERFMON_MASK 0xf >>> #define ID_DFR0_PMU_NOTIMPL 0b0000 >>> -#define ID_DFR0_PMU_V3 0b0001 >>> +#define ID_DFR0_PMU_V3 0b0011 >> Why? This is a macro used for AArch64 and DDI0487J.a (D19.2.59, the >> description of the PMUVer field) says that >> "0b0001 Performance Monitors Extension, PMUv3 implemented." >> while 0b0011 is a reserved value. > > I think this is a mix of 32bit and 64bit views (ID_DFR0_EL1.PerfMon > instead of ID_AA64DFR0_EL1.PMUVer), and the whole thing is a mess > (ID_AA64DFR0_PERFMON_MASK is clearly confused...). > > I haven't looked at how this patch fits in the rest of the code > though. Doh - yes different set of values for 32 bit. > > M.
diff --git a/arm/pmu.c b/arm/pmu.c index 9ff7a301..66163a40 100644 --- a/arm/pmu.c +++ b/arm/pmu.c @@ -200,7 +200,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits) {} #define ID_AA64DFR0_PERFMON_MASK 0xf #define ID_DFR0_PMU_NOTIMPL 0b0000 -#define ID_DFR0_PMU_V3 0b0001 +#define ID_DFR0_PMU_V3 0b0011 #define ID_DFR0_PMU_V3_8_1 0b0100 #define ID_DFR0_PMU_V3_8_4 0b0101 #define ID_DFR0_PMU_V3_8_5 0b0110 @@ -286,6 +286,11 @@ static void test_event_introspection(void) return; } + if (pmu.version < ID_DFR0_PMU_V3) { + report_skip("PMUv3 extensions not supported, skip ..."); + return; + } + /* PMUv3 requires an implementation includes some common events */ required_events = is_event_supported(SW_INCR, true) && is_event_supported(CPU_CYCLES, true) &&
The test for number of events is not a substitute for properly checking the feature register. Fix the define and skip if PMUv3 is not available on the system. This includes emulator such as QEMU which don't implement PMU counters as a matter of policy. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Anders Roxell <anders.roxell@linaro.org> --- arm/pmu.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)