Re: [Qemu-devel] [PATCH v8 07/13] target-arm: Make PMCEID[01]_EL0 64 bit registers, add PMCEID[23]

[Aaron Lindsay]

On Nov 30 16:10, Peter Maydell wrote:
> On Tue, 20 Nov 2018 at 21:26, Aaron Lindsay
> <address@hidden> wrote:
> >
> > Signed-off-by: Aaron Lindsay <address@hidden>
> > ---
> >  target/arm/cpu.h    |  4 ++--
> >  target/arm/helper.c | 12 ++++++++++--
> >  2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index 627e5c1995..50de58e4a2 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -837,8 +837,8 @@ struct ARMCPU {
> >      uint32_t id_pfr0;
> >      uint32_t id_pfr1;
> >      uint32_t id_dfr0;
> > -    uint32_t pmceid0;
> > -    uint32_t pmceid1;
> > +    uint64_t pmceid0;
> > +    uint64_t pmceid1;
> >      uint32_t id_afr0;
> >      uint32_t id_mmfr0;
> >      uint32_t id_mmfr1;
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index 71be6fb578..75f054fe79 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -5432,7 +5432,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> >              { .name = "PMCEID0", .state = ARM_CP_STATE_AA32,
> >                .cp = 15, .opc1 = 0, .crn = 9, .crm = 12, .opc2 = 6,
> >                .access = PL0_R, .accessfn = pmreg_access, .type = 
> > ARM_CP_CONST,
> > -              .resetvalue = cpu->pmceid0 },
> > +              .resetvalue = extract64(cpu->pmceid0, 0, 32) },
> > +            { .name = "PMCEID2", .state = ARM_CP_STATE_AA32,
> > +              .cp = 15, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 4,
> > +              .access = PL0_R, .accessfn = pmreg_access, .type = 
> > ARM_CP_CONST,
> > +              .resetvalue = extract64(cpu->pmceid0, 32, 32) },
> >              { .name = "PMCEID0_EL0", .state = ARM_CP_STATE_AA64,
> >                .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 6,
> >                .access = PL0_R, .accessfn = pmreg_access, .type = 
> > ARM_CP_CONST,
> > @@ -5440,7 +5444,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> >              { .name = "PMCEID1", .state = ARM_CP_STATE_AA32,
> >                .cp = 15, .opc1 = 0, .crn = 9, .crm = 12, .opc2 = 7,
> >                .access = PL0_R, .accessfn = pmreg_access, .type = 
> > ARM_CP_CONST,
> > -              .resetvalue = cpu->pmceid1 },
> > +              .resetvalue = extract64(cpu->pmceid1, 0, 32) },
> > +            { .name = "PMCEID3", .state = ARM_CP_STATE_AA32,
> > +              .cp = 15, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 5,
> > +              .access = PL0_R, .accessfn = pmreg_access, .type = 
> > ARM_CP_CONST,
> > +              .resetvalue = extract64(cpu->pmceid1, 32, 32) },
> >              { .name = "PMCEID1_EL0", .state = ARM_CP_STATE_AA64,
> >                .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 7,
> >                .access = PL0_R, .accessfn = pmreg_access, .type = 
> > ARM_CP_CONST,
> > --
> 
> PMCEID2 and PMCEID3 are only defined from ARMv8.1; before that they
> are UNDEFINED. So these registers need to be only defined if a
> suitable feature bit or ID register field check passes.

It looks like we don't currently support any ARMv8.1+ CPUs and don't
have an entry in the `arm_features` enum for it. I'll plan to add
ARM_FEATURE_V81 and make defining these registers depend on it, assuming
any future CPUs supporting it will use that, unless you feel I should do
something different.

-Aaron