Hi,
On Tue, Jul 09, 2024 at 03:05:07PM +0100, Alex Bennée wrote:
> Peter Maydell <peter.maydell@xxxxxxxxxx> writes:
>
> > On Tue, 9 Jul 2024 at 09:58, Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> On Tue, Jul 02, 2024 at 05:35:14PM +0100, 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@xxxxxxxxxx>
> >> > Cc: Anders Roxell <anders.roxell@xxxxxxxxxx>
> >> > ---
> >> > 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
> >> > #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;
> >> > + }
> >> > +
> >>
> >> I don't get this patch - test_event_introspection() is only run on 64bit. On
> >> arm64, if there is a PMU present, that PMU is a PMUv3. A prerequisite to
> >> running any PMU tests is for pmu_probe() to succeed, and pmu_probe() fails if
> >> there is no PMU implemented (PMUVer is either 0, or 0b1111). As a result, if
> >> test_event_introspection() is executed, then a PMUv3 is present.
> >>
> >> When does QEMU advertise FEAT_PMUv3*, but no event counters (other than the cycle
> >> counter)?
>
> The other option I have is this:
>
> --8<---------------cut here---------------start------------->8---
> arm/pmu: event-introspection needs icount for TCG
>
> The TCG accelerator will report a PMU (unless explicitly disabled with
> -cpu foo,pmu=off) however not all events are available unless you run
> under icount. Fix this by splitting the test into a kvm and tcg
> version.
As far as I can tell, if test_event_introspection() fails under TCG without
icount then there are two possible explanations for that:
1. Not all the events whose presence is checked by test_event_introspection()
are actually required by the architecture.
2. TCG without icount is not implementing all the events required by the
architecture.
If 1, then test_event_introspection() should be fixed. I had a look and the
function looked correct to me (except that the event name is not INST_PREC,
it's INST_SPEC in the Arm DDI0487J.A and K.a, but that's not relevant for
correctness).
