Re: [PULL 09/11] target/arm: add support for PMUv3 64-bit PMCCNTR in AArch32 mode

[Peter Maydell]

On Mon, 12 Aug 2024 at 12:10, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Sun, 11 Aug 2024 at 22:36, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> On 8/10/24 04:08, Peter Maydell wrote:
> >> > From: Alex Richardson <alexrichardson@google.com>
> >> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> >> > index 8fb4b474e83..94900667c33 100644
> >> > --- a/target/arm/helper.c
> >> > +++ b/target/arm/helper.c
> >> > @@ -5952,6 +5952,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
> >> >         .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> >> >         .writefn = sdcr_write,
> >> >         .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) },
> >> > +    { .name = "PMCCNTR", .state = ARM_CP_STATE_AA32,
> >> > +      .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT,
> >> > +      .cp = 15, .crm = 9, .opc1 = 0,
> >> > +      .access = PL0_RW, .resetvalue = 0, .fgt = FGT_PMCCNTR_EL0,
> >> > +      .readfn = pmccntr_read, .writefn = pmccntr_write,
> >> > +      .accessfn = pmreg_access_ccntr },
> >> >   };
> >> >
> >> >   /* These are present only when EL1 supports AArch32 */
> >>
> >> This fails testing:
> >>
> >> https://gitlab.com/qemu-project/qemu/-/jobs/7551982466
> >>
> >> FAIL: duplicate register {'name': 'PMCCNTR', 'regnum': 96} vs {'name': 
> >> 'PMCCNTR',
> >> 'regnum': 79}
> >> FAIL: counted all 219 registers in XML
> >> FAIL: PMCCNTR 96 == 79 (xml)
> >
> > Hmm, not sure why that didn't get caught by my local testing
> > or by my gitlab run -- does it only get run on an aarch64 host?
>
> It will depend what your local GDB is like - a modern gdb-multiarch
> should be fine but we do test for a minimum version to be able to probe
> the supported architectures.

Mmm, I found that a local "make check-tcg" does catch this for me,
so I guess the answer is "the gdb on the non aarch64 host CI jobs
is too old and/or we missed the coverage, and I forgot to run
this in my local checkout".

Why doesn't "make check" run "check-tcg" as a sub-test ?
Having it be separate is asking for people to forget to
run it, I think.

-- PMM