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

[Laurent Desnogues]

On Thu, Dec 17, 2020 at 1:10 PM Leif Lindholm <leif@nuviainc.com> wrote:
[...]
> > > > > +FIELD(CCSIDR_EL1, LINESIZE, 0, 3)
> > > > > +FIELD(CCSIDR_EL1, ASSOCIATIVITY, 3, 21)
> > > > > +FIELD(CCSIDR_EL1, NUMSETS, 32, 24)
> > > >
> > > > The positions and sizes of the ASSOCIATIVITY and NUMSETS CCSIDR fields
> > > > depend on whether the ARMv8.3-CCIDX extension is implemented or not.
> > > > If we really want to define the fields this way, we perhaps should
> > > > define two sets.  Or at the very least, add a comment stating this
> > > > definition is for ARMv8.3-CCIDX.
> > >
> > > Urgh, sorry for this.
> > > I added the fields only to make the CPU definition more readable, so I
> > > think we don't need to worry about runtime handling of this?
> > > But I don't think it makes sense to add only the one form.
> > > Should I use CCIDX_CCSIDR_EL1 for these ones and add
> > >
> > > /* When FEAT_CCIDX is not implemented */
> > > FIELD(CCSIDR_EL1, LINESIZE, 0, 3)
> > > FIELD(CCSIDR_EL1, ASSOCIATIVITY, 3, 10)
> > > FIELD(CCSIDR_EL1, NUMSETS, 13, 15)
> > >
> > > with a comment that
> > > /* When FEAT_CCIDX is implemented */
> > > for the former set
> > > ?
> >
> > Having both would be handy, but you need to have different names for
> > the fields.
>
> Different names for the same field?
> I.e.
> FIELD(CCIDX_CCSIDR_EL1, LINESIZE, 0, 3)
> would need a different name for LINESIZE than
> FIELD(CCSIDR_EL1, LINESIZE, 0, 3)
> ?

I was thinking about changing the field names, not the register name
because the register is the same, only the layout changes.  So
LINESIZE -> CCIDX_LINESIZE, etc.

That's personal preference, Peter might have a different one.

Thanks,

Laurent