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

[Leif Lindholm]

On Thu, Dec 17, 2020 at 13:18:03 +0100, Laurent Desnogues wrote:
> 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.

I see. Sure, that works too, and doesn't pollute the register name.
I'll wait for Peter before sending out v3.

Thanks!

/
    Leif

> 
> Thanks,
> 
> Laurent