Re: [PATCH 05/13] target/arm: Add and use FIELD definitions for ID_AA64DFR0_EL1

[Philippe Mathieu-Daudé]

On 2/11/20 7:34 PM, Richard Henderson wrote:
> On 2/11/20 9:37 AM, Peter Maydell wrote:
> >       if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> > -        assert(extract32(cpu->id_aa64dfr0, 12, 4) == brps);
> > -        assert(extract32(cpu->id_aa64dfr0, 20, 4) == wrps);
> > -        assert(extract32(cpu->id_aa64dfr0, 28, 4) == ctx_cmps);
> > +        assert(FIELD_EX32(cpu->id_aa64dfr0, ID_AA64DFR0, BRPS) == brps);
> > +        assert(FIELD_EX32(cpu->id_aa64dfr0, ID_AA64DFR0, WRPS) == wrps);
> > +        assert(FIELD_EX32(cpu->id_aa64dfr0, ID_AA64DFR0, CTX_CMPS) == 
> > ctx_cmps);
>
> Should really be FIELD_EX64.  Otherwise,

Similarly to the other previous call, FIELD_DP64:

       cpu->id_aa64dfr0 = FIELD_DP32(cpu->id_aa64dfr0, ID_AA64DFR0, 
PMUVER, 0);

So far the code is safe because the >31 bits macros aren't used:

  FIELD(ID_AA64DFR0, PMSVER, 32, 4)
  FIELD(ID_AA64DFR0, DOUBLELOCK, 36, 4)
  FIELD(ID_AA64DFR0, TRACEFILT, 40, 4)

But you are right, let's fix it now to avoid copy/pasting 32bit macros 
and unpleasant debugging sessions.

> Reviewed-by: Richard Henderson <address@hidden>

Using 64bit macros:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>