[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/5] target/arm: add descriptions of CLIDR_EL1, CCSIDR_EL1, C
From: |
Leif Lindholm |
Subject: |
Re: [PATCH 3/5] target/arm: add descriptions of CLIDR_EL1, CCSIDR_EL1, CTR_EL0 to cpu.h |
Date: |
Fri, 11 Dec 2020 16:12:36 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
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