[RFC 0/5] Privilege version update

[RFC 0/5] Privilege version update

[Atish Patra]

RISC-V International (RVI) has ratified many RISC-V ISA extensions recently[1].
The privileged specification version is also upgraded to v1.12. It means
certain CSRs introduced in v1.12 should only be accessible only if the
priv specification version supported is equal or greater than v1.12.
Doing this check in predicate function is not scalable as there will be
new CSRs introduced in the future versions of the privileged specification.

This series tries to address this problem by adding a field in the csr_ops
which can be checked in csrrw function before invoking the predicate function.
To keep the code churn to minimum, it is assumed that the minimum version of
the privilege version supported for any CSR is v1.10 unless specified
explicitly in the csr_ops table. Any new CSRs introduced in v1.12 have been
updated accordingly.

This will work fine for any ratified extensions. However, it is bit unclear
what should be done for the stable draft extensions. My suggestion is not
to update the priv field in the CSR ops table until the extension is
marked experimental (i.e. not frozen/ratified). Once the extension is
ratified and graduated from experimental to available stage, the privileged
spec version should be updated in the csr table if required. I am open to
other suggestions as well.

[1] https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions

Atish Patra (5):
target/riscv: Add the privileged spec version 1.12.0
target/riscv: Introduce privilege version field in the CSR ops.
target/riscv: Add support for mconfigptr
target/riscv: Add *envcfg* CSRs support
target/riscv: Enable privileged spec version 1.12

target/riscv/cpu.c      |   8 +-
target/riscv/cpu.h      |  10 ++
target/riscv/cpu_bits.h |  32 +++++++
target/riscv/csr.c      | 199 +++++++++++++++++++++++++++++++++-------
target/riscv/machine.c  |  26 ++++++
5 files changed, 237 insertions(+), 38 deletions(-)

--
2.30.2

[RFC 0/5] Privilege version update

[Atish Patra]

RISC-V International (RVI) has ratified many RISC-V ISA extensions recently[1].
The privileged specification version is also upgraded to v1.12. It means
certain CSRs introduced in v1.12 should only be accessible only if the
priv specification version supported is equal or greater than v1.12.
Doing this check in predicate function is not scalable as there will be
new CSRs introduced in the future versions of the privileged specification.

This series tries to address this problem by adding a field in the csr_ops
which can be checked in csrrw function before invoking the predicate function.
To keep the code churn to minimum, it is assumed that the minimum version of
the privilege version supported for any CSR is v1.10 unless specified
explicitly in the csr_ops table. Any new CSRs introduced in v1.12 have been
updated accordingly.

This will work fine for any ratified extensions. However, it is bit unclear
what should be done for the stable draft extensions. My suggestion is not
to update the priv field in the CSR ops table until the extension is
marked experimental (i.e. not frozen/ratified). Once the extension is
ratified and graduated from experimental to available stage, the privileged
spec version should be updated in the csr table if required. I am open to
other suggestions as well.

[1] https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions

Atish Patra (5):
target/riscv: Add the privileged spec version 1.12.0
target/riscv: Introduce privilege version field in the CSR ops.
target/riscv: Add support for mconfigptr
target/riscv: Add *envcfg* CSRs support
target/riscv: Enable privileged spec version 1.12

target/riscv/cpu.c      |   8 +-
target/riscv/cpu.h      |  10 ++
target/riscv/cpu_bits.h |  32 +++++++
target/riscv/csr.c      | 199 +++++++++++++++++++++++++++++++++-------
target/riscv/machine.c  |  26 ++++++
5 files changed, 237 insertions(+), 38 deletions(-)

--
2.30.2

[RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Atish Patra]

Add the definition for ratified privileged specification version v1.12

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4d630867650a..671f65100b1a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -82,6 +82,7 @@ enum {
 
 #define PRIV_VERSION_1_10_0 0x00011000
 #define PRIV_VERSION_1_11_0 0x00011100
+#define PRIV_VERSION_1_12_0 0x00011200
 
 #define VEXT_VERSION_1_00_0 0x00010000
 
-- 
2.30.2

[RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Atish Patra]

Add the definition for ratified privileged specification version v1.12

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4d630867650a..671f65100b1a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -82,6 +82,7 @@ enum {
 
 #define PRIV_VERSION_1_10_0 0x00011000
 #define PRIV_VERSION_1_11_0 0x00011100
+#define PRIV_VERSION_1_12_0 0x00011200
 
 #define VEXT_VERSION_1_00_0 0x00010000
 
-- 
2.30.2

Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Richard Henderson]

On 1/21/22 7:07 AM, Atish Patra wrote:
> Add the definition for ratified privileged specification version v1.12
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>   target/riscv/cpu.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4d630867650a..671f65100b1a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -82,6 +82,7 @@ enum {
>   
>   #define PRIV_VERSION_1_10_0 0x00011000
>   #define PRIV_VERSION_1_11_0 0x00011100
> +#define PRIV_VERSION_1_12_0 0x00011200

Is there any good reason for defining things this way, as opposed to a simple enumeration?
A simple enum would eliminate the need for


> +    /* The default privilege specification version supported is 1.10 */
> +    if (!csr_min_priv) {
> +        csr_min_priv = PRIV_VERSION_1_10_0;
> +    }

in patch 5.


r~

Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Richard Henderson]

On 1/21/22 7:07 AM, Atish Patra wrote:
> Add the definition for ratified privileged specification version v1.12
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>   target/riscv/cpu.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4d630867650a..671f65100b1a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -82,6 +82,7 @@ enum {
>   
>   #define PRIV_VERSION_1_10_0 0x00011000
>   #define PRIV_VERSION_1_11_0 0x00011100
> +#define PRIV_VERSION_1_12_0 0x00011200

Is there any good reason for defining things this way, as opposed to a simple enumeration?
A simple enum would eliminate the need for


> +    /* The default privilege specification version supported is 1.10 */
> +    if (!csr_min_priv) {
> +        csr_min_priv = PRIV_VERSION_1_10_0;
> +    }

in patch 5.


r~

Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Atish Kumar Patra]

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

On Sun, Jan 23, 2022 at 11:59 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 1/21/22 7:07 AM, Atish Patra wrote:
> > Add the definition for ratified privileged specification version v1.12
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >   target/riscv/cpu.h | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 4d630867650a..671f65100b1a 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -82,6 +82,7 @@ enum {
> >
> >   #define PRIV_VERSION_1_10_0 0x00011000
> >   #define PRIV_VERSION_1_11_0 0x00011100
> > +#define PRIV_VERSION_1_12_0 0x00011200
>
> Is there any good reason for defining things this way, as opposed to a
> simple enumeration?
> A simple enum would eliminate the need for
>
>
Agreed. A simple enum would be much nicer. I was just following the
previous definition of
PRIV_VERSION_1_10_0 & PRIV_VERSION_1_11_0.

I am not sure about the reason behind this scheme.

@Alistair Francis <alistair.francis@wdc.com> Is there any history behind
this scheme ?
or Are you okay if I change it ?


> > +    /* The default privilege specification version supported is 1.10 */
> > +    if (!csr_min_priv) {
> > +        csr_min_priv = PRIV_VERSION_1_10_0;
> > +    }
>
> in patch 5.
>
>
> r~
>

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

Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Atish Kumar Patra]

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

On Sun, Jan 23, 2022 at 11:59 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 1/21/22 7:07 AM, Atish Patra wrote:
> > Add the definition for ratified privileged specification version v1.12
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >   target/riscv/cpu.h | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 4d630867650a..671f65100b1a 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -82,6 +82,7 @@ enum {
> >
> >   #define PRIV_VERSION_1_10_0 0x00011000
> >   #define PRIV_VERSION_1_11_0 0x00011100
> > +#define PRIV_VERSION_1_12_0 0x00011200
>
> Is there any good reason for defining things this way, as opposed to a
> simple enumeration?
> A simple enum would eliminate the need for
>
>
Agreed. A simple enum would be much nicer. I was just following the
previous definition of
PRIV_VERSION_1_10_0 & PRIV_VERSION_1_11_0.

I am not sure about the reason behind this scheme.

@Alistair Francis <alistair.francis@wdc.com> Is there any history behind
this scheme ?
or Are you okay if I change it ?


> > +    /* The default privilege specification version supported is 1.10 */
> > +    if (!csr_min_priv) {
> > +        csr_min_priv = PRIV_VERSION_1_10_0;
> > +    }
>
> in patch 5.
>
>
> r~
>

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

Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Alistair Francis]

On Sat, Jan 29, 2022 at 10:57 AM Atish Kumar Patra <atishp@rivosinc.com> wrote:
>
>
>
> On Sun, Jan 23, 2022 at 11:59 PM Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> On 1/21/22 7:07 AM, Atish Patra wrote:
>> > Add the definition for ratified privileged specification version v1.12
>> >
>> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
>> > ---
>> >   target/riscv/cpu.h | 1 +
>> >   1 file changed, 1 insertion(+)
>> >
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index 4d630867650a..671f65100b1a 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -82,6 +82,7 @@ enum {
>> >
>> >   #define PRIV_VERSION_1_10_0 0x00011000
>> >   #define PRIV_VERSION_1_11_0 0x00011100
>> > +#define PRIV_VERSION_1_12_0 0x00011200
>>
>> Is there any good reason for defining things this way, as opposed to a simple enumeration?
>> A simple enum would eliminate the need for
>>
>
> Agreed. A simple enum would be much nicer. I was just following the previous definition of
> PRIV_VERSION_1_10_0 & PRIV_VERSION_1_11_0.
>
> I am not sure about the reason behind this scheme.
>
> @Alistair Francis Is there any history behind this scheme ?

I don't think so

> or Are you okay if I change it ?

Yep :)

Alistair

Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

[Alistair Francis]

On Sat, Jan 29, 2022 at 10:57 AM Atish Kumar Patra <atishp@rivosinc.com> wrote:
>
>
>
> On Sun, Jan 23, 2022 at 11:59 PM Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> On 1/21/22 7:07 AM, Atish Patra wrote:
>> > Add the definition for ratified privileged specification version v1.12
>> >
>> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
>> > ---
>> >   target/riscv/cpu.h | 1 +
>> >   1 file changed, 1 insertion(+)
>> >
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index 4d630867650a..671f65100b1a 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -82,6 +82,7 @@ enum {
>> >
>> >   #define PRIV_VERSION_1_10_0 0x00011000
>> >   #define PRIV_VERSION_1_11_0 0x00011100
>> > +#define PRIV_VERSION_1_12_0 0x00011200
>>
>> Is there any good reason for defining things this way, as opposed to a simple enumeration?
>> A simple enum would eliminate the need for
>>
>
> Agreed. A simple enum would be much nicer. I was just following the previous definition of
> PRIV_VERSION_1_10_0 & PRIV_VERSION_1_11_0.
>
> I am not sure about the reason behind this scheme.
>
> @Alistair Francis Is there any history behind this scheme ?

I don't think so

> or Are you okay if I change it ?

Yep :)

Alistair

[RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.

[Atish Patra]

To allow/disallow the CSR access based on the privilege spec, a new field
in the csr_ops is introduced. It also adds the privileged specification
version (v1.12) for the CSRs introduced in the v1.12. This includes the
new ratified extensions such as Vector, Hypervisor and secconfig CSR.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu.h |   1 +
 target/riscv/csr.c | 103 ++++++++++++++++++++++++++++++---------------
 2 files changed, 69 insertions(+), 35 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 671f65100b1a..7f87917204c5 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -521,6 +521,7 @@ typedef struct {
     riscv_csr_op_fn op;
     riscv_csr_read128_fn read128;
     riscv_csr_write128_fn write128;
+    uint32_t min_priv_ver;
 } riscv_csr_operations;
 
 /* CSR function table constants */
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index adb3d4381d1f..762d3269b4a4 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1991,13 +1991,20 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_FRM]      = { "frm",      fs,     read_frm,     write_frm    },
     [CSR_FCSR]     = { "fcsr",     fs,     read_fcsr,    write_fcsr   },
     /* Vector CSRs */
-    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart },
-    [CSR_VXSAT]    = { "vxsat",    vs,     read_vxsat,   write_vxsat  },
-    [CSR_VXRM]     = { "vxrm",     vs,     read_vxrm,    write_vxrm   },
-    [CSR_VCSR]     = { "vcsr",     vs,     read_vcsr,    write_vcsr   },
-    [CSR_VL]       = { "vl",       vs,     read_vl                    },
-    [CSR_VTYPE]    = { "vtype",    vs,     read_vtype                 },
-    [CSR_VLENB]    = { "vlenb",    vs,     read_vlenb                 },
+    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VXSAT]    = { "vxsat",    vs,     read_vxsat,   write_vxsat, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VXRM]     = { "vxrm",     vs,     read_vxrm,    write_vxrm, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VCSR]     = { "vcsr",     vs,     read_vcsr,    write_vcsr, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VL]       = { "vl",       vs,     read_vl, NULL, NULL, NULL, NULL,
+                                           PRIV_VERSION_1_12_0 },
+    [CSR_VTYPE]    = { "vtype",    vs,     read_vtype, NULL, NULL, NULL, NULL,
+                                           PRIV_VERSION_1_12_0 },
+    [CSR_VLENB]    = { "vlenb",    vs,     read_vlenb, NULL, NULL, NULL, NULL,
+                                           PRIV_VERSION_1_12_0 },
     /* User Timers and Counters */
     [CSR_CYCLE]    = { "cycle",    ctr,    read_instret  },
     [CSR_INSTRET]  = { "instret",  ctr,    read_instret  },
@@ -2063,36 +2070,62 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     /* Supervisor Protection and Translation */
     [CSR_SATP]     = { "satp",     smode, read_satp,    write_satp      },
 
-    [CSR_HSTATUS]     = { "hstatus",     hmode,   read_hstatus,     write_hstatus     },
-    [CSR_HEDELEG]     = { "hedeleg",     hmode,   read_hedeleg,     write_hedeleg     },
-    [CSR_HIDELEG]     = { "hideleg",     hmode,   read_hideleg,     write_hideleg     },
-    [CSR_HVIP]        = { "hvip",        hmode,   NULL,   NULL,     rmw_hvip          },
-    [CSR_HIP]         = { "hip",         hmode,   NULL,   NULL,     rmw_hip           },
-    [CSR_HIE]         = { "hie",         hmode,   read_hie,         write_hie         },
-    [CSR_HCOUNTEREN]  = { "hcounteren",  hmode,   read_hcounteren,  write_hcounteren  },
-    [CSR_HGEIE]       = { "hgeie",       hmode,   read_zero,        write_hgeie       },
-    [CSR_HTVAL]       = { "htval",       hmode,   read_htval,       write_htval       },
-    [CSR_HTINST]      = { "htinst",      hmode,   read_htinst,      write_htinst      },
-    [CSR_HGEIP]       = { "hgeip",       hmode,   read_zero,        write_hgeip       },
-    [CSR_HGATP]       = { "hgatp",       hmode,   read_hgatp,       write_hgatp       },
-    [CSR_HTIMEDELTA]  = { "htimedelta",  hmode,   read_htimedelta,  write_htimedelta  },
-    [CSR_HTIMEDELTAH] = { "htimedeltah", hmode32, read_htimedeltah, write_htimedeltah },
-
-    [CSR_VSSTATUS]    = { "vsstatus",    hmode,   read_vsstatus,    write_vsstatus    },
-    [CSR_VSIP]        = { "vsip",        hmode,   NULL,    NULL,    rmw_vsip          },
-    [CSR_VSIE]        = { "vsie",        hmode,   read_vsie,        write_vsie        },
-    [CSR_VSTVEC]      = { "vstvec",      hmode,   read_vstvec,      write_vstvec      },
-    [CSR_VSSCRATCH]   = { "vsscratch",   hmode,   read_vsscratch,   write_vsscratch   },
-    [CSR_VSEPC]       = { "vsepc",       hmode,   read_vsepc,       write_vsepc       },
-    [CSR_VSCAUSE]     = { "vscause",     hmode,   read_vscause,     write_vscause     },
-    [CSR_VSTVAL]      = { "vstval",      hmode,   read_vstval,      write_vstval      },
-    [CSR_VSATP]       = { "vsatp",       hmode,   read_vsatp,       write_vsatp       },
-
-    [CSR_MTVAL2]      = { "mtval2",      hmode,   read_mtval2,      write_mtval2      },
-    [CSR_MTINST]      = { "mtinst",      hmode,   read_mtinst,      write_mtinst      },
+    [CSR_HSTATUS]     = { "hstatus",     hmode,   read_hstatus,   write_hstatus,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HEDELEG]     = { "hedeleg",     hmode,   read_hedeleg,   write_hedeleg,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HIDELEG]     = { "hideleg",     hmode,   read_hideleg,   write_hideleg,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HVIP]        = { "hvip",        hmode,   NULL,   NULL,   rmw_hvip,
+                                         NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HIP]         = { "hip",         hmode,   NULL,   NULL,   rmw_hip,
+                                         NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HIE]         = { "hie",         hmode,   read_hie,       write_hie,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HCOUNTEREN]  = { "hcounteren",  hmode,   read_hcounteren, write_hcounteren,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HGEIE]       = { "hgeie",       hmode,   read_zero,      write_hgeie,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTVAL]       = { "htval",       hmode,   read_htval,     write_htval,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTINST]      = { "htinst",      hmode,   read_htinst,    write_htinst,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HGEIP]       = { "hgeip",       hmode,   read_zero,      write_hgeip,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HGATP]       = { "hgatp",       hmode,   read_hgatp,     write_hgatp,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTIMEDELTA]  = { "htimedelta",  hmode,   read_htimedelta, write_htimedelta,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTIMEDELTAH] = { "htimedeltah", hmode32, read_htimedeltah, write_htimedeltah,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+
+    [CSR_VSSTATUS]    = { "vsstatus",    hmode,   read_vsstatus,  write_vsstatus,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSIP]        = { "vsip",        hmode,   NULL,    NULL,  rmw_vsip,
+                                         NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSIE]        = { "vsie",        hmode,   read_vsie,      write_vsie,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSTVEC]      = { "vstvec",      hmode,   read_vstvec,    write_vstvec,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSSCRATCH]   = { "vsscratch",   hmode,   read_vsscratch, write_vsscratch,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSEPC]       = { "vsepc",       hmode,   read_vsepc,     write_vsepc,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSCAUSE]     = { "vscause",     hmode,   read_vscause,   write_vscause,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSTVAL]      = { "vstval",      hmode,   read_vstval,    write_vstval,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSATP]       = { "vsatp",       hmode,   read_vsatp,     write_vsatp,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+
+    [CSR_MTVAL2]      = { "mtval2",      hmode,   read_mtval2,    write_mtval2,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_MTINST]      = { "mtinst",      hmode,   read_mtinst,    write_mtinst,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
 
     /* Physical Memory Protection */
-    [CSR_MSECCFG]    = { "mseccfg",  epmp, read_mseccfg, write_mseccfg },
+    [CSR_MSECCFG]    = { "mseccfg",  epmp, read_mseccfg, write_mseccfg,
+                                     NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
     [CSR_PMPCFG0]    = { "pmpcfg0",   pmp, read_pmpcfg,  write_pmpcfg  },
     [CSR_PMPCFG1]    = { "pmpcfg1",   pmp, read_pmpcfg,  write_pmpcfg  },
     [CSR_PMPCFG2]    = { "pmpcfg2",   pmp, read_pmpcfg,  write_pmpcfg  },
-- 
2.30.2

[RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.

[Atish Patra]

To allow/disallow the CSR access based on the privilege spec, a new field
in the csr_ops is introduced. It also adds the privileged specification
version (v1.12) for the CSRs introduced in the v1.12. This includes the
new ratified extensions such as Vector, Hypervisor and secconfig CSR.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu.h |   1 +
 target/riscv/csr.c | 103 ++++++++++++++++++++++++++++++---------------
 2 files changed, 69 insertions(+), 35 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 671f65100b1a..7f87917204c5 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -521,6 +521,7 @@ typedef struct {
     riscv_csr_op_fn op;
     riscv_csr_read128_fn read128;
     riscv_csr_write128_fn write128;
+    uint32_t min_priv_ver;
 } riscv_csr_operations;
 
 /* CSR function table constants */
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index adb3d4381d1f..762d3269b4a4 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1991,13 +1991,20 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_FRM]      = { "frm",      fs,     read_frm,     write_frm    },
     [CSR_FCSR]     = { "fcsr",     fs,     read_fcsr,    write_fcsr   },
     /* Vector CSRs */
-    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart },
-    [CSR_VXSAT]    = { "vxsat",    vs,     read_vxsat,   write_vxsat  },
-    [CSR_VXRM]     = { "vxrm",     vs,     read_vxrm,    write_vxrm   },
-    [CSR_VCSR]     = { "vcsr",     vs,     read_vcsr,    write_vcsr   },
-    [CSR_VL]       = { "vl",       vs,     read_vl                    },
-    [CSR_VTYPE]    = { "vtype",    vs,     read_vtype                 },
-    [CSR_VLENB]    = { "vlenb",    vs,     read_vlenb                 },
+    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VXSAT]    = { "vxsat",    vs,     read_vxsat,   write_vxsat, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VXRM]     = { "vxrm",     vs,     read_vxrm,    write_vxrm, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VCSR]     = { "vcsr",     vs,     read_vcsr,    write_vcsr, NULL,
+                                           NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VL]       = { "vl",       vs,     read_vl, NULL, NULL, NULL, NULL,
+                                           PRIV_VERSION_1_12_0 },
+    [CSR_VTYPE]    = { "vtype",    vs,     read_vtype, NULL, NULL, NULL, NULL,
+                                           PRIV_VERSION_1_12_0 },
+    [CSR_VLENB]    = { "vlenb",    vs,     read_vlenb, NULL, NULL, NULL, NULL,
+                                           PRIV_VERSION_1_12_0 },
     /* User Timers and Counters */
     [CSR_CYCLE]    = { "cycle",    ctr,    read_instret  },
     [CSR_INSTRET]  = { "instret",  ctr,    read_instret  },
@@ -2063,36 +2070,62 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     /* Supervisor Protection and Translation */
     [CSR_SATP]     = { "satp",     smode, read_satp,    write_satp      },
 
-    [CSR_HSTATUS]     = { "hstatus",     hmode,   read_hstatus,     write_hstatus     },
-    [CSR_HEDELEG]     = { "hedeleg",     hmode,   read_hedeleg,     write_hedeleg     },
-    [CSR_HIDELEG]     = { "hideleg",     hmode,   read_hideleg,     write_hideleg     },
-    [CSR_HVIP]        = { "hvip",        hmode,   NULL,   NULL,     rmw_hvip          },
-    [CSR_HIP]         = { "hip",         hmode,   NULL,   NULL,     rmw_hip           },
-    [CSR_HIE]         = { "hie",         hmode,   read_hie,         write_hie         },
-    [CSR_HCOUNTEREN]  = { "hcounteren",  hmode,   read_hcounteren,  write_hcounteren  },
-    [CSR_HGEIE]       = { "hgeie",       hmode,   read_zero,        write_hgeie       },
-    [CSR_HTVAL]       = { "htval",       hmode,   read_htval,       write_htval       },
-    [CSR_HTINST]      = { "htinst",      hmode,   read_htinst,      write_htinst      },
-    [CSR_HGEIP]       = { "hgeip",       hmode,   read_zero,        write_hgeip       },
-    [CSR_HGATP]       = { "hgatp",       hmode,   read_hgatp,       write_hgatp       },
-    [CSR_HTIMEDELTA]  = { "htimedelta",  hmode,   read_htimedelta,  write_htimedelta  },
-    [CSR_HTIMEDELTAH] = { "htimedeltah", hmode32, read_htimedeltah, write_htimedeltah },
-
-    [CSR_VSSTATUS]    = { "vsstatus",    hmode,   read_vsstatus,    write_vsstatus    },
-    [CSR_VSIP]        = { "vsip",        hmode,   NULL,    NULL,    rmw_vsip          },
-    [CSR_VSIE]        = { "vsie",        hmode,   read_vsie,        write_vsie        },
-    [CSR_VSTVEC]      = { "vstvec",      hmode,   read_vstvec,      write_vstvec      },
-    [CSR_VSSCRATCH]   = { "vsscratch",   hmode,   read_vsscratch,   write_vsscratch   },
-    [CSR_VSEPC]       = { "vsepc",       hmode,   read_vsepc,       write_vsepc       },
-    [CSR_VSCAUSE]     = { "vscause",     hmode,   read_vscause,     write_vscause     },
-    [CSR_VSTVAL]      = { "vstval",      hmode,   read_vstval,      write_vstval      },
-    [CSR_VSATP]       = { "vsatp",       hmode,   read_vsatp,       write_vsatp       },
-
-    [CSR_MTVAL2]      = { "mtval2",      hmode,   read_mtval2,      write_mtval2      },
-    [CSR_MTINST]      = { "mtinst",      hmode,   read_mtinst,      write_mtinst      },
+    [CSR_HSTATUS]     = { "hstatus",     hmode,   read_hstatus,   write_hstatus,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HEDELEG]     = { "hedeleg",     hmode,   read_hedeleg,   write_hedeleg,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HIDELEG]     = { "hideleg",     hmode,   read_hideleg,   write_hideleg,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HVIP]        = { "hvip",        hmode,   NULL,   NULL,   rmw_hvip,
+                                         NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HIP]         = { "hip",         hmode,   NULL,   NULL,   rmw_hip,
+                                         NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HIE]         = { "hie",         hmode,   read_hie,       write_hie,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HCOUNTEREN]  = { "hcounteren",  hmode,   read_hcounteren, write_hcounteren,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HGEIE]       = { "hgeie",       hmode,   read_zero,      write_hgeie,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTVAL]       = { "htval",       hmode,   read_htval,     write_htval,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTINST]      = { "htinst",      hmode,   read_htinst,    write_htinst,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HGEIP]       = { "hgeip",       hmode,   read_zero,      write_hgeip,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HGATP]       = { "hgatp",       hmode,   read_hgatp,     write_hgatp,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTIMEDELTA]  = { "htimedelta",  hmode,   read_htimedelta, write_htimedelta,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_HTIMEDELTAH] = { "htimedeltah", hmode32, read_htimedeltah, write_htimedeltah,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+
+    [CSR_VSSTATUS]    = { "vsstatus",    hmode,   read_vsstatus,  write_vsstatus,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSIP]        = { "vsip",        hmode,   NULL,    NULL,  rmw_vsip,
+                                         NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSIE]        = { "vsie",        hmode,   read_vsie,      write_vsie,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSTVEC]      = { "vstvec",      hmode,   read_vstvec,    write_vstvec,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSSCRATCH]   = { "vsscratch",   hmode,   read_vsscratch, write_vsscratch,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSEPC]       = { "vsepc",       hmode,   read_vsepc,     write_vsepc,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSCAUSE]     = { "vscause",     hmode,   read_vscause,   write_vscause,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSTVAL]      = { "vstval",      hmode,   read_vstval,    write_vstval,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_VSATP]       = { "vsatp",       hmode,   read_vsatp,     write_vsatp,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+
+    [CSR_MTVAL2]      = { "mtval2",      hmode,   read_mtval2,    write_mtval2,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
+    [CSR_MTINST]      = { "mtinst",      hmode,   read_mtinst,    write_mtinst,
+                                         NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
 
     /* Physical Memory Protection */
-    [CSR_MSECCFG]    = { "mseccfg",  epmp, read_mseccfg, write_mseccfg },
+    [CSR_MSECCFG]    = { "mseccfg",  epmp, read_mseccfg, write_mseccfg,
+                                     NULL, NULL, NULL, PRIV_VERSION_1_12_0 },
     [CSR_PMPCFG0]    = { "pmpcfg0",   pmp, read_pmpcfg,  write_pmpcfg  },
     [CSR_PMPCFG1]    = { "pmpcfg1",   pmp, read_pmpcfg,  write_pmpcfg  },
     [CSR_PMPCFG2]    = { "pmpcfg2",   pmp, read_pmpcfg,  write_pmpcfg  },
-- 
2.30.2

Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.

[Richard Henderson]

On 1/21/22 7:07 AM, Atish Patra wrote:
> +    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart, NULL,
> +                                           NULL, NULL, PRIV_VERSION_1_12_0 },

I think adding all of these NULLs are hard to read.
Better to just add

     .min_priv_ver = PRIV_VERSION_1_12_0

to the existing entry.


r~

Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.

[Richard Henderson]

On 1/21/22 7:07 AM, Atish Patra wrote:
> +    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart, NULL,
> +                                           NULL, NULL, PRIV_VERSION_1_12_0 },

I think adding all of these NULLs are hard to read.
Better to just add

     .min_priv_ver = PRIV_VERSION_1_12_0

to the existing entry.


r~

Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.

[Atish Kumar Patra]

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

On Sun, Jan 23, 2022 at 11:56 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 1/21/22 7:07 AM, Atish Patra wrote:
> > +    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart,
> NULL,
> > +                                           NULL, NULL,
> PRIV_VERSION_1_12_0 },
>
> I think adding all of these NULLs are hard to read.
> Better to just add
>
>      .min_priv_ver = PRIV_VERSION_1_12_0
>
> to the existing entry.
>
>
Yeah. That's better. Fixed it.


>
> r~
>

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

Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.

[Atish Kumar Patra]

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

On Sun, Jan 23, 2022 at 11:56 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 1/21/22 7:07 AM, Atish Patra wrote:
> > +    [CSR_VSTART]   = { "vstart",   vs,     read_vstart,  write_vstart,
> NULL,
> > +                                           NULL, NULL,
> PRIV_VERSION_1_12_0 },
>
> I think adding all of these NULLs are hard to read.
> Better to just add
>
>      .min_priv_ver = PRIV_VERSION_1_12_0
>
> to the existing entry.
>
>
Yeah. That's better. Fixed it.


>
> r~
>

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

[RFC 3/5] target/riscv: Add support for mconfigptr

[Atish Patra]

RISC-V privileged specification v1.12 introduced a mconfigptr
which will hold the physical address of a configuration data
structure. As Qemu doesn't have a configuration data structure,
is read as zero which is valid as per the priv spec.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu_bits.h | 1 +
 target/riscv/csr.c      | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5a6d49aa64cc..f6f90b5cbd52 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -148,6 +148,7 @@
 #define CSR_MARCHID         0xf12
 #define CSR_MIMPID          0xf13
 #define CSR_MHARTID         0xf14
+#define CSR_MCONFIGPTR      0xf15
 
 /* Machine Trap Setup */
 #define CSR_MSTATUS         0x300
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 762d3269b4a4..e66bf2201857 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2030,6 +2030,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
     [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
     [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
+    [CSR_MCONFIGPTR]   = { "mconfigptr", any, read_zero, NULL, NULL, NULL, NULL,
+                                         PRIV_VERSION_1_12_0},
 
     /* Machine Trap Setup */
     [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
-- 
2.30.2

[RFC 3/5] target/riscv: Add support for mconfigptr

[Atish Patra]

RISC-V privileged specification v1.12 introduced a mconfigptr
which will hold the physical address of a configuration data
structure. As Qemu doesn't have a configuration data structure,
is read as zero which is valid as per the priv spec.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu_bits.h | 1 +
 target/riscv/csr.c      | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5a6d49aa64cc..f6f90b5cbd52 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -148,6 +148,7 @@
 #define CSR_MARCHID         0xf12
 #define CSR_MIMPID          0xf13
 #define CSR_MHARTID         0xf14
+#define CSR_MCONFIGPTR      0xf15
 
 /* Machine Trap Setup */
 #define CSR_MSTATUS         0x300
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 762d3269b4a4..e66bf2201857 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2030,6 +2030,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
     [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
     [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
+    [CSR_MCONFIGPTR]   = { "mconfigptr", any, read_zero, NULL, NULL, NULL, NULL,
+                                         PRIV_VERSION_1_12_0},
 
     /* Machine Trap Setup */
     [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
-- 
2.30.2

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

[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.h b/target/riscv/cpu.h
index 7f87917204c5..b9462300a472 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -264,6 +264,14 @@ struct CPURISCVState {
     target_ulong spmbase;
     target_ulong upmmask;
     target_ulong upmbase;
+
+    /* CSRs for execution enviornment configuration */
+
+    target_ulong menvcfg;
+    target_ulong menvcfgh;
+    target_ulong senvcfg;
+    target_ulong henvcfg;
+    target_ulong henvcfgh;
 #endif
 
     float_status fp_status;
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
@@ -177,6 +177,9 @@
 #define CSR_STVEC           0x105
 #define CSR_SCOUNTEREN      0x106
 
+/* Supervisor Configuration CSRs */
+#define CSR_SENVCFG         0x10A
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141
@@ -204,6 +207,10 @@
 #define CSR_HTIMEDELTA      0x605
 #define CSR_HTIMEDELTAH     0x615
 
+/* Hypervisor Configuration CSRs */
+#define CSR_HENVCFG         0x60A
+#define CSR_HENVCFGH        0x61A
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204
@@ -218,6 +225,10 @@
 #define CSR_MTINST          0x34a
 #define CSR_MTVAL2          0x34b
 
+/* Machine Configuration CSRs */
+#define CSR_MENVCFG         0x30A
+#define CSR_MENVCFGH        0x31A
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757
@@ -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
+#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
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e66bf2201857..a4bbae7a1bbd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+/* Execution environment configuration setup */
+static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->menvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->menvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->menvcfgh;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->menvcfgh = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->senvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->senvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->henvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->henvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->henvcfgh;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->henvcfgh = val;
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException rmw_mip(CPURISCVState *env, int csrno,
                               target_ulong *ret_value,
                               target_ulong new_value, target_ulong write_mask)
@@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval    },
     [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
 
+    /* Execution environment configuration */
+    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,  write_menvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh, write_menvcfgh, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,  write_senvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg, write_henvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+
+
     /* Supervisor Trap Setup */
     [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,    write_sstatus, NULL,
                                               read_sstatus_i128                 },
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 13b9ab375b95..59479a999b87 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
     }
 };
 
+/* TODO: henvcfg need both hyper_needed & envcfg_needed */
+static bool envcfg_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
+}
+
+static const VMStateDescription vmstate_envcfg = {
+    .name = "cpu/envcfg",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = envcfg_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
+        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
+
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 3,
@@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_vector,
         &vmstate_pointermasking,
         &vmstate_rv128,
+        &vmstate_envcfg,
         NULL
     }
 };
-- 
2.30.2

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

[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.h b/target/riscv/cpu.h
index 7f87917204c5..b9462300a472 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -264,6 +264,14 @@ struct CPURISCVState {
     target_ulong spmbase;
     target_ulong upmmask;
     target_ulong upmbase;
+
+    /* CSRs for execution enviornment configuration */
+
+    target_ulong menvcfg;
+    target_ulong menvcfgh;
+    target_ulong senvcfg;
+    target_ulong henvcfg;
+    target_ulong henvcfgh;
 #endif
 
     float_status fp_status;
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
@@ -177,6 +177,9 @@
 #define CSR_STVEC           0x105
 #define CSR_SCOUNTEREN      0x106
 
+/* Supervisor Configuration CSRs */
+#define CSR_SENVCFG         0x10A
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141
@@ -204,6 +207,10 @@
 #define CSR_HTIMEDELTA      0x605
 #define CSR_HTIMEDELTAH     0x615
 
+/* Hypervisor Configuration CSRs */
+#define CSR_HENVCFG         0x60A
+#define CSR_HENVCFGH        0x61A
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204
@@ -218,6 +225,10 @@
 #define CSR_MTINST          0x34a
 #define CSR_MTVAL2          0x34b
 
+/* Machine Configuration CSRs */
+#define CSR_MENVCFG         0x30A
+#define CSR_MENVCFGH        0x31A
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757
@@ -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
+#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
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e66bf2201857..a4bbae7a1bbd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+/* Execution environment configuration setup */
+static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->menvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->menvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->menvcfgh;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->menvcfgh = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->senvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->senvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->henvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->henvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->henvcfgh;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->henvcfgh = val;
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException rmw_mip(CPURISCVState *env, int csrno,
                               target_ulong *ret_value,
                               target_ulong new_value, target_ulong write_mask)
@@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval    },
     [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
 
+    /* Execution environment configuration */
+    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,  write_menvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh, write_menvcfgh, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,  write_senvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg, write_henvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+
+
     /* Supervisor Trap Setup */
     [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,    write_sstatus, NULL,
                                               read_sstatus_i128                 },
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 13b9ab375b95..59479a999b87 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
     }
 };
 
+/* TODO: henvcfg need both hyper_needed & envcfg_needed */
+static bool envcfg_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
+}
+
+static const VMStateDescription vmstate_envcfg = {
+    .name = "cpu/envcfg",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = envcfg_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
+        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
+
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 3,
@@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_vector,
         &vmstate_pointermasking,
         &vmstate_rv128,
+        &vmstate_envcfg,
         NULL
     }
 };
-- 
2.30.2

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

[Weiwei Li]

在 2022/1/21 上午4:07, 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.h b/target/riscv/cpu.h
> index 7f87917204c5..b9462300a472 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -264,6 +264,14 @@ struct CPURISCVState {
>       target_ulong spmbase;
>       target_ulong upmmask;
>       target_ulong upmbase;
> +
> +    /* CSRs for execution enviornment configuration */
> +
> +    target_ulong menvcfg;
> +    target_ulong menvcfgh;

I think we needn't maintain seperate menvcfg and menvcfgh, just use 
"uint64_t menvcfg" as the way of mstatus.

Similar to  henvcfg and henvcfg.

> +    target_ulong senvcfg;
> +    target_ulong henvcfg;
> +    target_ulong henvcfgh;
>   #endif
>   
>       float_status fp_status;
> 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
> @@ -177,6 +177,9 @@
>   #define CSR_STVEC           0x105
>   #define CSR_SCOUNTEREN      0x106
>   
> +/* Supervisor Configuration CSRs */
> +#define CSR_SENVCFG         0x10A
> +
>   /* Supervisor Trap Handling */
>   #define CSR_SSCRATCH        0x140
>   #define CSR_SEPC            0x141
> @@ -204,6 +207,10 @@
>   #define CSR_HTIMEDELTA      0x605
>   #define CSR_HTIMEDELTAH     0x615
>   
> +/* Hypervisor Configuration CSRs */
> +#define CSR_HENVCFG         0x60A
> +#define CSR_HENVCFGH        0x61A
> +
>   /* Virtual CSRs */
>   #define CSR_VSSTATUS        0x200
>   #define CSR_VSIE            0x204
> @@ -218,6 +225,10 @@
>   #define CSR_MTINST          0x34a
>   #define CSR_MTVAL2          0x34b
>   
> +/* Machine Configuration CSRs */
> +#define CSR_MENVCFG         0x30A
> +#define CSR_MENVCFGH        0x31A
> +
>   /* Enhanced Physical Memory Protection (ePMP) */
>   #define CSR_MSECCFG         0x747
>   #define CSR_MSECCFGH        0x757
> @@ -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
> +#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
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e66bf2201857..a4bbae7a1bbd 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
>       return RISCV_EXCP_NONE;
>   }
>   
> +/* Execution environment configuration setup */
> +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->menvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->menvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->menvcfgh;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->menvcfgh = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->senvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->senvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->henvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->henvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->henvcfgh;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->henvcfgh = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
>   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>                                 target_ulong *ret_value,
>                                 target_ulong new_value, target_ulong write_mask)
> @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval    },
>       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
>   
> +    /* Execution environment configuration */
> +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,  write_menvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh, write_menvcfgh, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,  write_senvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg, write_henvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +
> +
Two new lines here.
>       /* Supervisor Trap Setup */
>       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,    write_sstatus, NULL,
>                                                 read_sstatus_i128                 },
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 13b9ab375b95..59479a999b87 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
>       }
>   };
>   
> +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
> +static bool envcfg_needed(void *opaque)
> +{
> +    RISCVCPU *cpu = opaque;
> +    CPURISCVState *env = &cpu->env;
> +
> +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
> +}
> +
> +static const VMStateDescription vmstate_envcfg = {
> +    .name = "cpu/envcfg",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = envcfg_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
> +
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>   const VMStateDescription vmstate_riscv_cpu = {
>       .name = "cpu",
>       .version_id = 3,
> @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>           &vmstate_vector,
>           &vmstate_pointermasking,
>           &vmstate_rv128,
> +        &vmstate_envcfg,
>           NULL
>       }
>   };

Regards,

Weiwei Li

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

[Weiwei Li]

在 2022/1/21 上午4:07, 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.h b/target/riscv/cpu.h
> index 7f87917204c5..b9462300a472 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -264,6 +264,14 @@ struct CPURISCVState {
>       target_ulong spmbase;
>       target_ulong upmmask;
>       target_ulong upmbase;
> +
> +    /* CSRs for execution enviornment configuration */
> +
> +    target_ulong menvcfg;
> +    target_ulong menvcfgh;

I think we needn't maintain seperate menvcfg and menvcfgh, just use 
"uint64_t menvcfg" as the way of mstatus.

Similar to  henvcfg and henvcfg.

> +    target_ulong senvcfg;
> +    target_ulong henvcfg;
> +    target_ulong henvcfgh;
>   #endif
>   
>       float_status fp_status;
> 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
> @@ -177,6 +177,9 @@
>   #define CSR_STVEC           0x105
>   #define CSR_SCOUNTEREN      0x106
>   
> +/* Supervisor Configuration CSRs */
> +#define CSR_SENVCFG         0x10A
> +
>   /* Supervisor Trap Handling */
>   #define CSR_SSCRATCH        0x140
>   #define CSR_SEPC            0x141
> @@ -204,6 +207,10 @@
>   #define CSR_HTIMEDELTA      0x605
>   #define CSR_HTIMEDELTAH     0x615
>   
> +/* Hypervisor Configuration CSRs */
> +#define CSR_HENVCFG         0x60A
> +#define CSR_HENVCFGH        0x61A
> +
>   /* Virtual CSRs */
>   #define CSR_VSSTATUS        0x200
>   #define CSR_VSIE            0x204
> @@ -218,6 +225,10 @@
>   #define CSR_MTINST          0x34a
>   #define CSR_MTVAL2          0x34b
>   
> +/* Machine Configuration CSRs */
> +#define CSR_MENVCFG         0x30A
> +#define CSR_MENVCFGH        0x31A
> +
>   /* Enhanced Physical Memory Protection (ePMP) */
>   #define CSR_MSECCFG         0x747
>   #define CSR_MSECCFGH        0x757
> @@ -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
> +#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
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e66bf2201857..a4bbae7a1bbd 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
>       return RISCV_EXCP_NONE;
>   }
>   
> +/* Execution environment configuration setup */
> +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->menvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->menvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->menvcfgh;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->menvcfgh = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->senvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->senvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->henvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->henvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->henvcfgh;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->henvcfgh = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
>   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>                                 target_ulong *ret_value,
>                                 target_ulong new_value, target_ulong write_mask)
> @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval    },
>       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
>   
> +    /* Execution environment configuration */
> +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,  write_menvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh, write_menvcfgh, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,  write_senvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg, write_henvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +
> +
Two new lines here.
>       /* Supervisor Trap Setup */
>       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,    write_sstatus, NULL,
>                                                 read_sstatus_i128                 },
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 13b9ab375b95..59479a999b87 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
>       }
>   };
>   
> +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
> +static bool envcfg_needed(void *opaque)
> +{
> +    RISCVCPU *cpu = opaque;
> +    CPURISCVState *env = &cpu->env;
> +
> +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
> +}
> +
> +static const VMStateDescription vmstate_envcfg = {
> +    .name = "cpu/envcfg",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = envcfg_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
> +
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>   const VMStateDescription vmstate_riscv_cpu = {
>       .name = "cpu",
>       .version_id = 3,
> @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>           &vmstate_vector,
>           &vmstate_pointermasking,
>           &vmstate_rv128,
> +        &vmstate_envcfg,
>           NULL
>       }
>   };

Regards,

Weiwei Li

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

[Atish Patra]

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

On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:

>
> 在 2022/1/21 上午4:07, 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.h b/target/riscv/cpu.h
> > index 7f87917204c5..b9462300a472 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -264,6 +264,14 @@ struct CPURISCVState {
> >       target_ulong spmbase;
> >       target_ulong upmmask;
> >       target_ulong upmbase;
> > +
> > +    /* CSRs for execution enviornment configuration */
> > +
> > +    target_ulong menvcfg;
> > +    target_ulong menvcfgh;
>
> I think we needn't maintain seperate menvcfg and menvcfgh, just use
> "uint64_t menvcfg" as the way of mstatus.
>
>
unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
predicate for stimecmp/vstimecmp.

We have to do the 32 bit shifting during every check which makes the code
hard to read
at the cost of 2 ulongs.

IMO, having separate variables is much simpler.

Similar to  henvcfg and henvcfg.
>
> > +    target_ulong senvcfg;
> > +    target_ulong henvcfg;
> > +    target_ulong henvcfgh;
> >   #endif
> >
> >       float_status fp_status;
> > 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
> > @@ -177,6 +177,9 @@
> >   #define CSR_STVEC           0x105
> >   #define CSR_SCOUNTEREN      0x106
> >
> > +/* Supervisor Configuration CSRs */
> > +#define CSR_SENVCFG         0x10A
> > +
> >   /* Supervisor Trap Handling */
> >   #define CSR_SSCRATCH        0x140
> >   #define CSR_SEPC            0x141
> > @@ -204,6 +207,10 @@
> >   #define CSR_HTIMEDELTA      0x605
> >   #define CSR_HTIMEDELTAH     0x615
> >
> > +/* Hypervisor Configuration CSRs */
> > +#define CSR_HENVCFG         0x60A
> > +#define CSR_HENVCFGH        0x61A
> > +
> >   /* Virtual CSRs */
> >   #define CSR_VSSTATUS        0x200
> >   #define CSR_VSIE            0x204
> > @@ -218,6 +225,10 @@
> >   #define CSR_MTINST          0x34a
> >   #define CSR_MTVAL2          0x34b
> >
> > +/* Machine Configuration CSRs */
> > +#define CSR_MENVCFG         0x30A
> > +#define CSR_MENVCFGH        0x31A
> > +
> >   /* Enhanced Physical Memory Protection (ePMP) */
> >   #define CSR_MSECCFG         0x747
> >   #define CSR_MSECCFGH        0x757
> > @@ -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
> > +#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
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index e66bf2201857..a4bbae7a1bbd 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState
> *env, int csrno,
> >       return RISCV_EXCP_NONE;
> >   }
> >
> > +/* Execution environment configuration setup */
> > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->menvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->menvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->menvcfgh;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->menvcfgh = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->senvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->senvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->henvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->henvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->henvcfgh;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->henvcfgh = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
> >                                 target_ulong *ret_value,
> >                                 target_ulong new_value, target_ulong
> write_mask)
> > @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval
> },
> >       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip
> },
> >
> > +    /* Execution environment configuration */
> > +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,
> write_menvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh,
> write_menvcfgh, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,
> write_senvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg,
> write_henvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
> write_henvcfgh, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +
> > +
> Two new lines here.
>

Fixed it. Thanks.


> >       /* Supervisor Trap Setup */
> >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,
> write_sstatus, NULL,
> >                                                 read_sstatus_i128
>          },
> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > index 13b9ab375b95..59479a999b87 100644
> > --- a/target/riscv/machine.c
> > +++ b/target/riscv/machine.c
> > @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
> >       }
> >   };
> >
> > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
> > +static bool envcfg_needed(void *opaque)
> > +{
> > +    RISCVCPU *cpu = opaque;
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
> > +}
> > +
> > +static const VMStateDescription vmstate_envcfg = {
> > +    .name = "cpu/envcfg",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = envcfg_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
> > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
> > +
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >   const VMStateDescription vmstate_riscv_cpu = {
> >       .name = "cpu",
> >       .version_id = 3,
> > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
> >           &vmstate_vector,
> >           &vmstate_pointermasking,
> >           &vmstate_rv128,
> > +        &vmstate_envcfg,
> >           NULL
> >       }
> >   };
>
> Regards,
>
> Weiwei Li
>
>
>

-- 
Regards,
Atish

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

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

[Atish Patra]

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

On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:

>
> 在 2022/1/21 上午4:07, 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.h b/target/riscv/cpu.h
> > index 7f87917204c5..b9462300a472 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -264,6 +264,14 @@ struct CPURISCVState {
> >       target_ulong spmbase;
> >       target_ulong upmmask;
> >       target_ulong upmbase;
> > +
> > +    /* CSRs for execution enviornment configuration */
> > +
> > +    target_ulong menvcfg;
> > +    target_ulong menvcfgh;
>
> I think we needn't maintain seperate menvcfg and menvcfgh, just use
> "uint64_t menvcfg" as the way of mstatus.
>
>
unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
predicate for stimecmp/vstimecmp.

We have to do the 32 bit shifting during every check which makes the code
hard to read
at the cost of 2 ulongs.

IMO, having separate variables is much simpler.

Similar to  henvcfg and henvcfg.
>
> > +    target_ulong senvcfg;
> > +    target_ulong henvcfg;
> > +    target_ulong henvcfgh;
> >   #endif
> >
> >       float_status fp_status;
> > 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
> > @@ -177,6 +177,9 @@
> >   #define CSR_STVEC           0x105
> >   #define CSR_SCOUNTEREN      0x106
> >
> > +/* Supervisor Configuration CSRs */
> > +#define CSR_SENVCFG         0x10A
> > +
> >   /* Supervisor Trap Handling */
> >   #define CSR_SSCRATCH        0x140
> >   #define CSR_SEPC            0x141
> > @@ -204,6 +207,10 @@
> >   #define CSR_HTIMEDELTA      0x605
> >   #define CSR_HTIMEDELTAH     0x615
> >
> > +/* Hypervisor Configuration CSRs */
> > +#define CSR_HENVCFG         0x60A
> > +#define CSR_HENVCFGH        0x61A
> > +
> >   /* Virtual CSRs */
> >   #define CSR_VSSTATUS        0x200
> >   #define CSR_VSIE            0x204
> > @@ -218,6 +225,10 @@
> >   #define CSR_MTINST          0x34a
> >   #define CSR_MTVAL2          0x34b
> >
> > +/* Machine Configuration CSRs */
> > +#define CSR_MENVCFG         0x30A
> > +#define CSR_MENVCFGH        0x31A
> > +
> >   /* Enhanced Physical Memory Protection (ePMP) */
> >   #define CSR_MSECCFG         0x747
> >   #define CSR_MSECCFGH        0x757
> > @@ -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
> > +#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
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index e66bf2201857..a4bbae7a1bbd 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState
> *env, int csrno,
> >       return RISCV_EXCP_NONE;
> >   }
> >
> > +/* Execution environment configuration setup */
> > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->menvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->menvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->menvcfgh;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->menvcfgh = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->senvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->senvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->henvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->henvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->henvcfgh;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->henvcfgh = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
> >                                 target_ulong *ret_value,
> >                                 target_ulong new_value, target_ulong
> write_mask)
> > @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval
> },
> >       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip
> },
> >
> > +    /* Execution environment configuration */
> > +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,
> write_menvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh,
> write_menvcfgh, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,
> write_senvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg,
> write_henvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
> write_henvcfgh, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +
> > +
> Two new lines here.
>

Fixed it. Thanks.


> >       /* Supervisor Trap Setup */
> >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,
> write_sstatus, NULL,
> >                                                 read_sstatus_i128
>          },
> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > index 13b9ab375b95..59479a999b87 100644
> > --- a/target/riscv/machine.c
> > +++ b/target/riscv/machine.c
> > @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
> >       }
> >   };
> >
> > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
> > +static bool envcfg_needed(void *opaque)
> > +{
> > +    RISCVCPU *cpu = opaque;
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
> > +}
> > +
> > +static const VMStateDescription vmstate_envcfg = {
> > +    .name = "cpu/envcfg",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = envcfg_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
> > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
> > +
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >   const VMStateDescription vmstate_riscv_cpu = {
> >       .name = "cpu",
> >       .version_id = 3,
> > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
> >           &vmstate_vector,
> >           &vmstate_pointermasking,
> >           &vmstate_rv128,
> > +        &vmstate_envcfg,
> >           NULL
> >       }
> >   };
>
> Regards,
>
> Weiwei Li
>
>
>

-- 
Regards,
Atish

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

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

[angell1518]

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


在 2022/1/29 上午9:28, Atish Patra 写道:
>
>
> On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn 
> <mailto:liweiwei@iscas.ac.cn>> wrote:
>
>
>     在 2022/1/21 上午4:07, 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
>     <mailto: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.h b/target/riscv/cpu.h
>     > index 7f87917204c5..b9462300a472 100644
>     > --- a/target/riscv/cpu.h
>     > +++ b/target/riscv/cpu.h
>     > @@ -264,6 +264,14 @@ struct CPURISCVState {
>     >       target_ulong spmbase;
>     >       target_ulong upmmask;
>     >       target_ulong upmbase;
>     > +
>     > +    /* CSRs for execution enviornment configuration */
>     > +
>     > +    target_ulong menvcfg;
>     > +    target_ulong menvcfgh;
>
>     I think we needn't maintain seperate menvcfg and menvcfgh, just use
>     "uint64_t menvcfg" as the way of mstatus.
>
>
> unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
> predicate for stimecmp/vstimecmp.
>
> We have to do the 32 bit shifting during every check which makes the 
> code hard to read
> at the cost of 2 ulongs.
>
> IMO, having separate variables is much simpler.

Do you mean check STCE/VSTCE bit in menvcfg/henvcfg?

If so, I think use a simple "uint64_t menvcfg/henvcfg" may be better,  
then we can only check the 63 bit of them.

Or we should decide where to get this bit from(mencvfg/henvcfg, or 
mencfgh/henvcfgh) based on the MXLEN/HSXLEN.

Regards,

Weiwei Li

>
>     Similar to  henvcfg and henvcfg.
>
>     > +    target_ulong senvcfg;
>     > +    target_ulong henvcfg;
>     > +    target_ulong henvcfgh;
>     >   #endif
>     >
>     >       float_status fp_status;
>     > 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
>     > @@ -177,6 +177,9 @@
>     >   #define CSR_STVEC           0x105
>     >   #define CSR_SCOUNTEREN      0x106
>     >
>     > +/* Supervisor Configuration CSRs */
>     > +#define CSR_SENVCFG         0x10A
>     > +
>     >   /* Supervisor Trap Handling */
>     >   #define CSR_SSCRATCH        0x140
>     >   #define CSR_SEPC            0x141
>     > @@ -204,6 +207,10 @@
>     >   #define CSR_HTIMEDELTA      0x605
>     >   #define CSR_HTIMEDELTAH     0x615
>     >
>     > +/* Hypervisor Configuration CSRs */
>     > +#define CSR_HENVCFG         0x60A
>     > +#define CSR_HENVCFGH        0x61A
>     > +
>     >   /* Virtual CSRs */
>     >   #define CSR_VSSTATUS        0x200
>     >   #define CSR_VSIE            0x204
>     > @@ -218,6 +225,10 @@
>     >   #define CSR_MTINST          0x34a
>     >   #define CSR_MTVAL2          0x34b
>     >
>     > +/* Machine Configuration CSRs */
>     > +#define CSR_MENVCFG         0x30A
>     > +#define CSR_MENVCFGH        0x31A
>     > +
>     >   /* Enhanced Physical Memory Protection (ePMP) */
>     >   #define CSR_MSECCFG         0x747
>     >   #define CSR_MSECCFGH        0x757
>     > @@ -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
>     > +#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
>     > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>     > index e66bf2201857..a4bbae7a1bbd 100644
>     > --- a/target/riscv/csr.c
>     > +++ b/target/riscv/csr.c
>     > @@ -853,6 +853,77 @@ static RISCVException
>     write_mtval(CPURISCVState *env, int csrno,
>     >       return RISCV_EXCP_NONE;
>     >   }
>     >
>     > +/* Execution environment configuration setup */
>     > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->menvcfg;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->menvcfg = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->menvcfgh;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->menvcfgh = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->senvcfg;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->senvcfg = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->henvcfg;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->henvcfg = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->henvcfgh;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->henvcfgh = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>     >                                 target_ulong *ret_value,
>     >                                 target_ulong new_value,
>     target_ulong write_mask)
>     > @@ -2054,6 +2125,19 @@ riscv_csr_operations
>     csr_ops[CSR_TABLE_SIZE] = {
>     >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,  
>     write_mtval    },
>     >       [CSR_MIP]      = { "mip",      any,  NULL, NULL, rmw_mip 
>           },
>     >
>     > +    /* Execution environment configuration */
>     > +    [CSR_MENVCFG]  = { "menvcfg",  any,  read_menvcfg, 
>     write_menvcfg, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_MENVCFGH] = { "menvcfgh", any32,  read_menvcfgh,
>     write_menvcfgh, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_SENVCFG]  = { "senvcfg",  smode,  read_senvcfg, 
>     write_senvcfg, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,  read_henvcfg,
>     write_henvcfg, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
>     write_henvcfgh, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +
>     > +
>     Two new lines here.
>
>
> Fixed it. Thanks.
>
>     >       /* Supervisor Trap Setup */
>     >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,   
>     write_sstatus, NULL,
>     >  read_sstatus_i128                 },
>     > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>     > index 13b9ab375b95..59479a999b87 100644
>     > --- a/target/riscv/machine.c
>     > +++ b/target/riscv/machine.c
>     > @@ -185,6 +185,31 @@ static const VMStateDescription
>     vmstate_rv128 = {
>     >       }
>     >   };
>     >
>     > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
>     > +static bool envcfg_needed(void *opaque)
>     > +{
>     > +    RISCVCPU *cpu = opaque;
>     > +    CPURISCVState *env = &cpu->env;
>     > +
>     > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
>     > +}
>     > +
>     > +static const VMStateDescription vmstate_envcfg = {
>     > +    .name = "cpu/envcfg",
>     > +    .version_id = 1,
>     > +    .minimum_version_id = 1,
>     > +    .needed = envcfg_needed,
>     > +    .fields = (VMStateField[]) {
>     > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
>     > +
>     > +        VMSTATE_END_OF_LIST()
>     > +    }
>     > +};
>     > +
>     >   const VMStateDescription vmstate_riscv_cpu = {
>     >       .name = "cpu",
>     >       .version_id = 3,
>     > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>     >           &vmstate_vector,
>     >           &vmstate_pointermasking,
>     >           &vmstate_rv128,
>     > +        &vmstate_envcfg,
>     >           NULL
>     >       }
>     >   };
>
>     Regards,
>
>     Weiwei Li
>
>
>
>
> -- 
> Regards,
> Atish

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

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

[Atish Kumar Patra]

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

On Fri, Jan 28, 2022 at 5:50 PM angell1518 <angell1518@163.com> wrote:

>
> 在 2022/1/29 上午9:28, Atish Patra 写道:
>
>
>
> On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>>
>> 在 2022/1/21 上午4:07, 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.h b/target/riscv/cpu.h
>> > index 7f87917204c5..b9462300a472 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -264,6 +264,14 @@ struct CPURISCVState {
>> >       target_ulong spmbase;
>> >       target_ulong upmmask;
>> >       target_ulong upmbase;
>> > +
>> > +    /* CSRs for execution enviornment configuration */
>> > +
>> > +    target_ulong menvcfg;
>> > +    target_ulong menvcfgh;
>>
>> I think we needn't maintain seperate menvcfg and menvcfgh, just use
>> "uint64_t menvcfg" as the way of mstatus.
>>
>>
> unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
> predicate for stimecmp/vstimecmp.
>
> We have to do the 32 bit shifting during every check which makes the code
> hard to read
> at the cost of 2 ulongs.
>
> IMO, having separate variables is much simpler.
>
> Do you mean check STCE/VSTCE bit in menvcfg/henvcfg?
>
> If so, I think use a simple "uint64_t menvcfg/henvcfg" may be better,
> then we can only check the 63 bit of them.
>

Which is a bit confusing as the STCE bit in mencfgh/henvcfgh is 31 not 63.
But that's my personal preference.

I will just leave a comment to clarify the confusion for now. I will send a
patch with unified menvcfg and wait for others's feedback.

> Or we should decide where to get this bit from(mencvfg/henvcfg, or
> mencfgh/henvcfgh) based on the MXLEN/HSXLEN.
>
> Regards,
>
> Weiwei Li
>
>
> Similar to  henvcfg and henvcfg.
>>
>> > +    target_ulong senvcfg;
>> > +    target_ulong henvcfg;
>> > +    target_ulong henvcfgh;
>> >   #endif
>> >
>> >       float_status fp_status;
>> > 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
>> > @@ -177,6 +177,9 @@
>> >   #define CSR_STVEC           0x105
>> >   #define CSR_SCOUNTEREN      0x106
>> >
>> > +/* Supervisor Configuration CSRs */
>> > +#define CSR_SENVCFG         0x10A
>> > +
>> >   /* Supervisor Trap Handling */
>> >   #define CSR_SSCRATCH        0x140
>> >   #define CSR_SEPC            0x141
>> > @@ -204,6 +207,10 @@
>> >   #define CSR_HTIMEDELTA      0x605
>> >   #define CSR_HTIMEDELTAH     0x615
>> >
>> > +/* Hypervisor Configuration CSRs */
>> > +#define CSR_HENVCFG         0x60A
>> > +#define CSR_HENVCFGH        0x61A
>> > +
>> >   /* Virtual CSRs */
>> >   #define CSR_VSSTATUS        0x200
>> >   #define CSR_VSIE            0x204
>> > @@ -218,6 +225,10 @@
>> >   #define CSR_MTINST          0x34a
>> >   #define CSR_MTVAL2          0x34b
>> >
>> > +/* Machine Configuration CSRs */
>> > +#define CSR_MENVCFG         0x30A
>> > +#define CSR_MENVCFGH        0x31A
>> > +
>> >   /* Enhanced Physical Memory Protection (ePMP) */
>> >   #define CSR_MSECCFG         0x747
>> >   #define CSR_MSECCFGH        0x757
>> > @@ -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
>> > +#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
>> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > index e66bf2201857..a4bbae7a1bbd 100644
>> > --- a/target/riscv/csr.c
>> > +++ b/target/riscv/csr.c
>> > @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState
>> *env, int csrno,
>> >       return RISCV_EXCP_NONE;
>> >   }
>> >
>> > +/* Execution environment configuration setup */
>> > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->menvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->menvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->menvcfgh;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->menvcfgh = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->senvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->senvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->henvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->henvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->henvcfgh;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->henvcfgh = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>> >                                 target_ulong *ret_value,
>> >                                 target_ulong new_value, target_ulong
>> write_mask)
>> > @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>> >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval
>>   },
>> >       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip
>>   },
>> >
>> > +    /* Execution environment configuration */
>> > +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,
>> write_menvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh,
>> write_menvcfgh, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,
>> write_senvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg,
>> write_henvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
>> write_henvcfgh, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +
>> > +
>> Two new lines here.
>>
>
> Fixed it. Thanks.
>
>
>> >       /* Supervisor Trap Setup */
>> >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,
>> write_sstatus, NULL,
>> >                                                 read_sstatus_i128
>>            },
>> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> > index 13b9ab375b95..59479a999b87 100644
>> > --- a/target/riscv/machine.c
>> > +++ b/target/riscv/machine.c
>> > @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
>> >       }
>> >   };
>> >
>> > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
>> > +static bool envcfg_needed(void *opaque)
>> > +{
>> > +    RISCVCPU *cpu = opaque;
>> > +    CPURISCVState *env = &cpu->env;
>> > +
>> > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
>> > +}
>> > +
>> > +static const VMStateDescription vmstate_envcfg = {
>> > +    .name = "cpu/envcfg",
>> > +    .version_id = 1,
>> > +    .minimum_version_id = 1,
>> > +    .needed = envcfg_needed,
>> > +    .fields = (VMStateField[]) {
>> > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
>> > +
>> > +        VMSTATE_END_OF_LIST()
>> > +    }
>> > +};
>> > +
>> >   const VMStateDescription vmstate_riscv_cpu = {
>> >       .name = "cpu",
>> >       .version_id = 3,
>> > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>> >           &vmstate_vector,
>> >           &vmstate_pointermasking,
>> >           &vmstate_rv128,
>> > +        &vmstate_envcfg,
>> >           NULL
>> >       }
>> >   };
>>
>> Regards,
>>
>> Weiwei Li
>>
>>
>>
>
> --
> Regards,
> Atish
>
>

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

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

[Atish Kumar Patra]

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

On Fri, Jan 28, 2022 at 5:50 PM angell1518 <angell1518@163.com> wrote:

>
> 在 2022/1/29 上午9:28, Atish Patra 写道:
>
>
>
> On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>>
>> 在 2022/1/21 上午4:07, 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.h b/target/riscv/cpu.h
>> > index 7f87917204c5..b9462300a472 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -264,6 +264,14 @@ struct CPURISCVState {
>> >       target_ulong spmbase;
>> >       target_ulong upmmask;
>> >       target_ulong upmbase;
>> > +
>> > +    /* CSRs for execution enviornment configuration */
>> > +
>> > +    target_ulong menvcfg;
>> > +    target_ulong menvcfgh;
>>
>> I think we needn't maintain seperate menvcfg and menvcfgh, just use
>> "uint64_t menvcfg" as the way of mstatus.
>>
>>
> unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
> predicate for stimecmp/vstimecmp.
>
> We have to do the 32 bit shifting during every check which makes the code
> hard to read
> at the cost of 2 ulongs.
>
> IMO, having separate variables is much simpler.
>
> Do you mean check STCE/VSTCE bit in menvcfg/henvcfg?
>
> If so, I think use a simple "uint64_t menvcfg/henvcfg" may be better,
> then we can only check the 63 bit of them.
>

Which is a bit confusing as the STCE bit in mencfgh/henvcfgh is 31 not 63.
But that's my personal preference.

I will just leave a comment to clarify the confusion for now. I will send a
patch with unified menvcfg and wait for others's feedback.

> Or we should decide where to get this bit from(mencvfg/henvcfg, or
> mencfgh/henvcfgh) based on the MXLEN/HSXLEN.
>
> Regards,
>
> Weiwei Li
>
>
> Similar to  henvcfg and henvcfg.
>>
>> > +    target_ulong senvcfg;
>> > +    target_ulong henvcfg;
>> > +    target_ulong henvcfgh;
>> >   #endif
>> >
>> >       float_status fp_status;
>> > 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
>> > @@ -177,6 +177,9 @@
>> >   #define CSR_STVEC           0x105
>> >   #define CSR_SCOUNTEREN      0x106
>> >
>> > +/* Supervisor Configuration CSRs */
>> > +#define CSR_SENVCFG         0x10A
>> > +
>> >   /* Supervisor Trap Handling */
>> >   #define CSR_SSCRATCH        0x140
>> >   #define CSR_SEPC            0x141
>> > @@ -204,6 +207,10 @@
>> >   #define CSR_HTIMEDELTA      0x605
>> >   #define CSR_HTIMEDELTAH     0x615
>> >
>> > +/* Hypervisor Configuration CSRs */
>> > +#define CSR_HENVCFG         0x60A
>> > +#define CSR_HENVCFGH        0x61A
>> > +
>> >   /* Virtual CSRs */
>> >   #define CSR_VSSTATUS        0x200
>> >   #define CSR_VSIE            0x204
>> > @@ -218,6 +225,10 @@
>> >   #define CSR_MTINST          0x34a
>> >   #define CSR_MTVAL2          0x34b
>> >
>> > +/* Machine Configuration CSRs */
>> > +#define CSR_MENVCFG         0x30A
>> > +#define CSR_MENVCFGH        0x31A
>> > +
>> >   /* Enhanced Physical Memory Protection (ePMP) */
>> >   #define CSR_MSECCFG         0x747
>> >   #define CSR_MSECCFGH        0x757
>> > @@ -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
>> > +#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
>> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > index e66bf2201857..a4bbae7a1bbd 100644
>> > --- a/target/riscv/csr.c
>> > +++ b/target/riscv/csr.c
>> > @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState
>> *env, int csrno,
>> >       return RISCV_EXCP_NONE;
>> >   }
>> >
>> > +/* Execution environment configuration setup */
>> > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->menvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->menvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->menvcfgh;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->menvcfgh = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->senvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->senvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->henvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->henvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->henvcfgh;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->henvcfgh = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>> >                                 target_ulong *ret_value,
>> >                                 target_ulong new_value, target_ulong
>> write_mask)
>> > @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>> >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval
>>   },
>> >       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip
>>   },
>> >
>> > +    /* Execution environment configuration */
>> > +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,
>> write_menvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh,
>> write_menvcfgh, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,
>> write_senvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg,
>> write_henvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
>> write_henvcfgh, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +
>> > +
>> Two new lines here.
>>
>
> Fixed it. Thanks.
>
>
>> >       /* Supervisor Trap Setup */
>> >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,
>> write_sstatus, NULL,
>> >                                                 read_sstatus_i128
>>            },
>> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> > index 13b9ab375b95..59479a999b87 100644
>> > --- a/target/riscv/machine.c
>> > +++ b/target/riscv/machine.c
>> > @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
>> >       }
>> >   };
>> >
>> > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
>> > +static bool envcfg_needed(void *opaque)
>> > +{
>> > +    RISCVCPU *cpu = opaque;
>> > +    CPURISCVState *env = &cpu->env;
>> > +
>> > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
>> > +}
>> > +
>> > +static const VMStateDescription vmstate_envcfg = {
>> > +    .name = "cpu/envcfg",
>> > +    .version_id = 1,
>> > +    .minimum_version_id = 1,
>> > +    .needed = envcfg_needed,
>> > +    .fields = (VMStateField[]) {
>> > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
>> > +
>> > +        VMSTATE_END_OF_LIST()
>> > +    }
>> > +};
>> > +
>> >   const VMStateDescription vmstate_riscv_cpu = {
>> >       .name = "cpu",
>> >       .version_id = 3,
>> > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>> >           &vmstate_vector,
>> >           &vmstate_pointermasking,
>> >           &vmstate_rv128,
>> > +        &vmstate_envcfg,
>> >           NULL
>> >       }
>> >   };
>>
>> Regards,
>>
>> Weiwei Li
>>
>>
>>
>
> --
> Regards,
> Atish
>
>

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

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

[Heiko Stübner]

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].

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

> +#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

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

[Atish Kumar Patra]

[-- 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 --]

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

[Atish Kumar Patra]

[-- 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 --]

[RFC 5/5] target/riscv: Enable privileged spec version 1.12

[Atish Patra]

Virt machine uses privileged specification version 1.12 now.
All other machine continue to use the default one defined for that
machine unless changed to 1.12 by the user explicitly.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu.c |  8 +++++---
 target/riscv/csr.c | 10 ++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9bc25d3055d4..cec5791151e7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -153,7 +153,7 @@ static void riscv_any_cpu_init(Object *obj)
 #elif defined(TARGET_RISCV64)
     set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
 #endif
-    set_priv_version(env, PRIV_VERSION_1_11_0);
+    set_priv_version(env, PRIV_VERSION_1_12_0);
 }
 
 #if defined(TARGET_RISCV64)
@@ -439,7 +439,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     }
 
     if (cpu->cfg.priv_spec) {
-        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
+        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
+            priv_version = PRIV_VERSION_1_12_0;
+        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
             priv_version = PRIV_VERSION_1_11_0;
         } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
             priv_version = PRIV_VERSION_1_10_0;
@@ -454,7 +456,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     if (priv_version) {
         set_priv_version(env, priv_version);
     } else if (!env->priv_ver) {
-        set_priv_version(env, PRIV_VERSION_1_11_0);
+        set_priv_version(env, PRIV_VERSION_1_12_0);
     }
 
     if (cpu->cfg.mmu) {
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a4bbae7a1bbd..62d429cc3f17 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1874,6 +1874,12 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
     int read_only = get_field(csrno, 0xC00) == 3;
 #if !defined(CONFIG_USER_ONLY)
     int effective_priv = env->priv;
+    int csr_min_priv = csr_ops[csrno].min_priv_ver;
+
+    /* The default privilege specification version supported is 1.10 */
+    if (!csr_min_priv) {
+        csr_min_priv = PRIV_VERSION_1_10_0;
+    }
 
     if (riscv_has_ext(env, RVH) &&
         env->priv == PRV_S &&
@@ -1904,6 +1910,10 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
+    if (env->priv_ver < csr_min_priv) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
     return csr_ops[csrno].predicate(env, csrno);
 }
 
-- 
2.30.2

[RFC 5/5] target/riscv: Enable privileged spec version 1.12

[Atish Patra]

Virt machine uses privileged specification version 1.12 now.
All other machine continue to use the default one defined for that
machine unless changed to 1.12 by the user explicitly.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu.c |  8 +++++---
 target/riscv/csr.c | 10 ++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9bc25d3055d4..cec5791151e7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -153,7 +153,7 @@ static void riscv_any_cpu_init(Object *obj)
 #elif defined(TARGET_RISCV64)
     set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
 #endif
-    set_priv_version(env, PRIV_VERSION_1_11_0);
+    set_priv_version(env, PRIV_VERSION_1_12_0);
 }
 
 #if defined(TARGET_RISCV64)
@@ -439,7 +439,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     }
 
     if (cpu->cfg.priv_spec) {
-        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
+        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
+            priv_version = PRIV_VERSION_1_12_0;
+        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
             priv_version = PRIV_VERSION_1_11_0;
         } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
             priv_version = PRIV_VERSION_1_10_0;
@@ -454,7 +456,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     if (priv_version) {
         set_priv_version(env, priv_version);
     } else if (!env->priv_ver) {
-        set_priv_version(env, PRIV_VERSION_1_11_0);
+        set_priv_version(env, PRIV_VERSION_1_12_0);
     }
 
     if (cpu->cfg.mmu) {
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a4bbae7a1bbd..62d429cc3f17 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1874,6 +1874,12 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
     int read_only = get_field(csrno, 0xC00) == 3;
 #if !defined(CONFIG_USER_ONLY)
     int effective_priv = env->priv;
+    int csr_min_priv = csr_ops[csrno].min_priv_ver;
+
+    /* The default privilege specification version supported is 1.10 */
+    if (!csr_min_priv) {
+        csr_min_priv = PRIV_VERSION_1_10_0;
+    }
 
     if (riscv_has_ext(env, RVH) &&
         env->priv == PRV_S &&
@@ -1904,6 +1910,10 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
+    if (env->priv_ver < csr_min_priv) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
     return csr_ops[csrno].predicate(env, csrno);
 }
 
-- 
2.30.2

Re: [RFC 5/5] target/riscv: Enable privileged spec version 1.12

[Christoph Müllner]

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

On Fri, Jan 21, 2022 at 12:16 AM Atish Patra <atishp@rivosinc.com> wrote:

> Virt machine uses privileged specification version 1.12 now.
> All other machine continue to use the default one defined for that
> machine unless changed to 1.12 by the user explicitly.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  target/riscv/cpu.c |  8 +++++---
>  target/riscv/csr.c | 10 ++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9bc25d3055d4..cec5791151e7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -153,7 +153,7 @@ static void riscv_any_cpu_init(Object *obj)
>  #elif defined(TARGET_RISCV64)
>      set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
>  #endif
> -    set_priv_version(env, PRIV_VERSION_1_11_0);
> +    set_priv_version(env, PRIV_VERSION_1_12_0);
>  }
>
>  #if defined(TARGET_RISCV64)
> @@ -439,7 +439,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
>      }
>
>      if (cpu->cfg.priv_spec) {
> -        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
> +        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
> +            priv_version = PRIV_VERSION_1_12_0;
> +        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
>              priv_version = PRIV_VERSION_1_11_0;
>          } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
>              priv_version = PRIV_VERSION_1_10_0;
> @@ -454,7 +456,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
>      if (priv_version) {
>          set_priv_version(env, priv_version);
>      } else if (!env->priv_ver) {
> -        set_priv_version(env, PRIV_VERSION_1_11_0);
> +        set_priv_version(env, PRIV_VERSION_1_12_0);
>      }
>
>      if (cpu->cfg.mmu) {
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a4bbae7a1bbd..62d429cc3f17 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1874,6 +1874,12 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
>      int read_only = get_field(csrno, 0xC00) == 3;
>  #if !defined(CONFIG_USER_ONLY)
>      int effective_priv = env->priv;
> +    int csr_min_priv = csr_ops[csrno].min_priv_ver;
> +
> +    /* The default privilege specification version supported is 1.10 */
> +    if (!csr_min_priv) {
> +        csr_min_priv = PRIV_VERSION_1_10_0;
> +    }
>
>      if (riscv_has_ext(env, RVH) &&
>          env->priv == PRV_S &&
> @@ -1904,6 +1910,10 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
>          return RISCV_EXCP_ILLEGAL_INST;
>      }
>
> +    if (env->priv_ver < csr_min_priv) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
>

This does not compile if CONFIG_USER_ONLY is defined.


> +
>      return csr_ops[csrno].predicate(env, csrno);
>  }
>
> --
> 2.30.2
>
>

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

Re: [RFC 5/5] target/riscv: Enable privileged spec version 1.12

[Christoph Müllner]

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

On Fri, Jan 21, 2022 at 12:16 AM Atish Patra <atishp@rivosinc.com> wrote:

> Virt machine uses privileged specification version 1.12 now.
> All other machine continue to use the default one defined for that
> machine unless changed to 1.12 by the user explicitly.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  target/riscv/cpu.c |  8 +++++---
>  target/riscv/csr.c | 10 ++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9bc25d3055d4..cec5791151e7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -153,7 +153,7 @@ static void riscv_any_cpu_init(Object *obj)
>  #elif defined(TARGET_RISCV64)
>      set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
>  #endif
> -    set_priv_version(env, PRIV_VERSION_1_11_0);
> +    set_priv_version(env, PRIV_VERSION_1_12_0);
>  }
>
>  #if defined(TARGET_RISCV64)
> @@ -439,7 +439,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
>      }
>
>      if (cpu->cfg.priv_spec) {
> -        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
> +        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
> +            priv_version = PRIV_VERSION_1_12_0;
> +        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
>              priv_version = PRIV_VERSION_1_11_0;
>          } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
>              priv_version = PRIV_VERSION_1_10_0;
> @@ -454,7 +456,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
>      if (priv_version) {
>          set_priv_version(env, priv_version);
>      } else if (!env->priv_ver) {
> -        set_priv_version(env, PRIV_VERSION_1_11_0);
> +        set_priv_version(env, PRIV_VERSION_1_12_0);
>      }
>
>      if (cpu->cfg.mmu) {
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a4bbae7a1bbd..62d429cc3f17 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1874,6 +1874,12 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
>      int read_only = get_field(csrno, 0xC00) == 3;
>  #if !defined(CONFIG_USER_ONLY)
>      int effective_priv = env->priv;
> +    int csr_min_priv = csr_ops[csrno].min_priv_ver;
> +
> +    /* The default privilege specification version supported is 1.10 */
> +    if (!csr_min_priv) {
> +        csr_min_priv = PRIV_VERSION_1_10_0;
> +    }
>
>      if (riscv_has_ext(env, RVH) &&
>          env->priv == PRV_S &&
> @@ -1904,6 +1910,10 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
>          return RISCV_EXCP_ILLEGAL_INST;
>      }
>
> +    if (env->priv_ver < csr_min_priv) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
>

This does not compile if CONFIG_USER_ONLY is defined.


> +
>      return csr_ops[csrno].predicate(env, csrno);
>  }
>
> --
> 2.30.2
>
>

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