Re: [PATCH 3/5] target/arm: add descriptions of CLIDR_EL1, CCSIDR_EL1, CTR_EL0 to cpu.h

[Leif Lindholm <leif@nuviainc.com>]

On Fri, Dec 11, 2020 at 14:45:55 +0000, Peter Maydell wrote:
> On Tue, 8 Dec 2020 at 12:23, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> > ---
> >  target/arm/cpu.h | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index b54d1dc092..5e9e8061f7 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -1713,6 +1713,30 @@ FIELD(V7M_FPCCR, ASPEN, 31, 1)
> >  /*
> >   * System register ID fields.
> >   */
> > +FIELD(CLIDR_EL1, CTYPE1, 0, 3)
> > +FIELD(CLIDR_EL1, CTYPE2, 3, 3)
> > +FIELD(CLIDR_EL1, CTYPE3, 6, 3)
> > +FIELD(CLIDR_EL1, CTYPE4, 9, 3)
> > +FIELD(CLIDR_EL1, CTYPE5, 12, 3)
> > +FIELD(CLIDR_EL1, CTYPE6, 15, 3)
> > +FIELD(CLIDR_EL1, CTYPE7, 18, 3)
> > +FIELD(CLIDR_EL1, LOUIS, 21, 3)
> > +FIELD(CLIDR_EL1, LOC, 24, 3)
> > +FIELD(CLIDR_EL1, LOUU, 27, 3)
> > +FIELD(CLIDR_EL1, ICB, 30, 3)
> > +
> > +FIELD(CCSIDR_EL1, LINESIZE, 0, 3)
> > +FIELD(CCSIDR_EL1, ASSOCIATIVITY, 3, 20)
> 
> The ASSOCIATIVITY field is bits [23:3], so it's
> 21 bits long, not 20, right ?

Err, indeed.

> > +FIELD(CCSIDR_EL1, NUMSETS, 32, 23)
> 
> Similarly, NUMSETS is [55:32] so 24 bits long.

Sorry, brain must have taken holiday.

Would you like a v2 of this patch, fixing those?

> > +
> > +FIELD(CTR_EL0,  IMINLINE, 0, 4)
> > +FIELD(CTR_EL0,  L1IP, 14, 2)
> > +FIELD(CTR_EL0,  DMINLINE, 16, 4)
> > +FIELD(CTR_EL0,  ERG, 20, 4)
> > +FIELD(CTR_EL0,  CWG, 24, 4)
> > +FIELD(CTR_EL0,  IDC, 28, 1)
> > +FIELD(CTR_EL0,  DIC, 29, 1)
> > +
> >  FIELD(MIDR_EL1, REVISION, 0, 4)
> >  FIELD(MIDR_EL1, PARTNUM, 4, 12)
> >  FIELD(MIDR_EL1, ARCHITECTURE, 16, 4)
> 
> Any reason not to define the other fields here?
> FIELD(MIDR_EL1, VARIANT, 20, 4)
> FIELD(MIDR_EL1, IMPLEMENTER, 24, 8)

Those are just context, not added by this patch.
(Glad to see I'm not the only one making that mistake...)

Best Regards,

Leif

> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM