Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support

[Atish Kumar Patra <atishp@rivosinc.com>]

[-- Attachment #1: Type: text/plain, Size: 2744 bytes --]

On Thu, Feb 3, 2022 at 4:23 AM Heiko Stübner <heiko@sntech.de> wrote:

> Hi Atish,
>
> Am Donnerstag, 20. Januar 2022, 21:07:34 CET schrieb Atish Patra:
> > The RISC-V privileged specification v1.12 defines few execution
> > environment configuration CSRs that can be used enable/disable
> > extensions per privilege levels.
> >
> > Add the basic support for these CSRs.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >  target/riscv/cpu.h      |  8 ++++
> >  target/riscv/cpu_bits.h | 31 +++++++++++++++
> >  target/riscv/csr.c      | 84 +++++++++++++++++++++++++++++++++++++++++
> >  target/riscv/machine.c  | 26 +++++++++++++
> >  4 files changed, 149 insertions(+)
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index f6f90b5cbd52..afb237c2313b 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
>
> [...]
>
> > @@ -578,6 +589,26 @@ typedef enum RISCVException {
> >  #define PM_EXT_CLEAN    0x00000002ULL
> >  #define PM_EXT_DIRTY    0x00000003ULL
> >
> > +/* Execution enviornment configuration bits */
> > +#define MENVCFG_FIOM                       (1 << 0)
>
> > +#define MENVCFG_CBE                        0x30000ULL
>
> Looking both at the cmo spec as well as the most recent privileged spec
> (draft) the field is called CBIE it seems.
>
> Also the shift looks wrong. Both cmo as well as privileged spec show
> it at bits [5:4] and _not_ [17:16].
>
>
This looks like a typo from my side. These bits are reserved in the spec!
Apologies for such a silly mistake. Fixed it in v2.


> Also wouldn't doing it like (_UL(3) << 4) be better to catch such things?
>
>
Of course.


> > +#define MENVCFG_CBCFE                      (1 << 6)
> > +#define MENVCFG_CBZE                       (1 << 7)
> > +#define MENVCFG_PBMTE                      (1 << 62)
> > +#define MENVCFG_STCE                       (1 << 63)
> > +
> > +#define SENVCFG_FIOM                       MENVCFG_FIOM
> > +#define SENVCFG_CBE                        MENVCFG_CBE
> > +#define SENVCFG_CBCFE                      MENVCFG_CBCFE
> > +#define SENVCFG_CBZE                       MENVCFG_CBZE
> > +
> > +#define HENVCFG_FIOM                       MENVCFG_FIOM
> > +#define HENVCFG_CBE                        MENVCFG_CBE
> > +#define HENVCFG_CBCFE                      MENVCFG_CBCFE
> > +#define HENVCFG_CBZE                       MENVCFG_CBZE
> > +#define HENVCFG_PBMTE                      MENVCFG_PBMTE
> > +#define HENVCFG_STCE                       MENVCFG_STCE
> > +
> >  /* Offsets for every pair of control bits per each priv level */
> >  #define XS_OFFSET    0ULL
> >  #define U_OFFSET     2ULL
>
>
> Heiko
>
>
>

[-- Attachment #2: Type: text/html, Size: 3980 bytes --]