All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/5] Privilege version update
@ 2022-01-20 20:07 ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Atish Patra, Alistair Francis,
	Palmer Dabbelt

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



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [RFC 0/5] Privilege version update
@ 2022-01-20 20:07 ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Atish Patra, Alistair Francis, Bin Meng, Palmer Dabbelt,
	Anup Patel, qemu-riscv

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



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
  2022-01-20 20:07 ` Atish Patra
@ 2022-01-20 20:07   ` Atish Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Atish Patra, Alistair Francis,
	Palmer Dabbelt

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
@ 2022-01-20 20:07   ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Atish Patra, Alistair Francis, Bin Meng, Palmer Dabbelt,
	Anup Patel, qemu-riscv

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.
  2022-01-20 20:07 ` Atish Patra
@ 2022-01-20 20:07   ` Atish Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Atish Patra, Alistair Francis,
	Palmer Dabbelt

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.
@ 2022-01-20 20:07   ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Atish Patra, Alistair Francis, Bin Meng, Palmer Dabbelt,
	Anup Patel, qemu-riscv

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 3/5] target/riscv: Add support for mconfigptr
  2022-01-20 20:07 ` Atish Patra
@ 2022-01-20 20:07   ` Atish Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Atish Patra, Alistair Francis,
	Palmer Dabbelt

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 3/5] target/riscv: Add support for mconfigptr
@ 2022-01-20 20:07   ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Atish Patra, Alistair Francis, Bin Meng, Palmer Dabbelt,
	Anup Patel, qemu-riscv

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 4/5] target/riscv: Add *envcfg* CSRs support
  2022-01-20 20:07 ` Atish Patra
@ 2022-01-20 20:07   ` Atish Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Atish Patra, Alistair Francis,
	Palmer Dabbelt

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 4/5] target/riscv: Add *envcfg* CSRs support
@ 2022-01-20 20:07   ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Atish Patra, Alistair Francis, Bin Meng, Palmer Dabbelt,
	Anup Patel, qemu-riscv

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 5/5] target/riscv: Enable privileged spec version 1.12
  2022-01-20 20:07 ` Atish Patra
@ 2022-01-20 20:07   ` Atish Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Atish Patra, Alistair Francis,
	Palmer Dabbelt

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC 5/5] target/riscv: Enable privileged spec version 1.12
@ 2022-01-20 20:07   ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-20 20:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Atish Patra, Alistair Francis, Bin Meng, Palmer Dabbelt,
	Anup Patel, qemu-riscv

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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.
  2022-01-20 20:07   ` Atish Patra
@ 2022-01-24  7:56     ` Richard Henderson
  -1 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2022-01-24  7:56 UTC (permalink / raw)
  To: Atish Patra, qemu-devel
  Cc: Anup Patel, Alistair Francis, Bin Meng, qemu-riscv, Palmer Dabbelt

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~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.
@ 2022-01-24  7:56     ` Richard Henderson
  0 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2022-01-24  7:56 UTC (permalink / raw)
  To: Atish Patra, qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Alistair Francis, Palmer Dabbelt

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~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
  2022-01-20 20:07   ` Atish Patra
@ 2022-01-24  7:59     ` Richard Henderson
  -1 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2022-01-24  7:59 UTC (permalink / raw)
  To: Atish Patra, qemu-devel
  Cc: Anup Patel, Alistair Francis, Bin Meng, qemu-riscv, Palmer Dabbelt

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~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
@ 2022-01-24  7:59     ` Richard Henderson
  0 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2022-01-24  7:59 UTC (permalink / raw)
  To: Atish Patra, qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Alistair Francis, Palmer Dabbelt

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~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 5/5] target/riscv: Enable privileged spec version 1.12
  2022-01-20 20:07   ` Atish Patra
@ 2022-01-24 13:32     ` Christoph Müllner
  -1 siblings, 0 replies; 34+ messages in thread
From: Christoph Müllner @ 2022-01-24 13:32 UTC (permalink / raw)
  To: Atish Patra
  Cc: open list:RISC-V, Anup Patel, Bin Meng,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 5/5] target/riscv: Enable privileged spec version 1.12
@ 2022-01-24 13:32     ` Christoph Müllner
  0 siblings, 0 replies; 34+ messages in thread
From: Christoph Müllner @ 2022-01-24 13:32 UTC (permalink / raw)
  To: Atish Patra
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Anup Patel,
	Bin Meng, Alistair Francis, Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
  2022-01-20 20:07   ` Atish Patra
@ 2022-01-26  8:37     ` Weiwei Li
  -1 siblings, 0 replies; 34+ messages in thread
From: Weiwei Li @ 2022-01-26  8:37 UTC (permalink / raw)
  To: Atish Patra, qemu-devel
  Cc: Anup Patel, Alistair Francis, Bin Meng, qemu-riscv, Palmer Dabbelt


在 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



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
@ 2022-01-26  8:37     ` Weiwei Li
  0 siblings, 0 replies; 34+ messages in thread
From: Weiwei Li @ 2022-01-26  8:37 UTC (permalink / raw)
  To: Atish Patra, qemu-devel
  Cc: qemu-riscv, Anup Patel, Bin Meng, Alistair Francis, Palmer Dabbelt


在 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



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
  2022-01-24  7:59     ` Richard Henderson
@ 2022-01-29  0:52       ` Atish Kumar Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-01-29  0:52 UTC (permalink / raw)
  To: Richard Henderson
  Cc: open list:RISC-V, Anup Patel, Bin Meng,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
@ 2022-01-29  0:52       ` Atish Kumar Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-01-29  0:52 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Anup Patel,
	Bin Meng, Alistair Francis, Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.
  2022-01-24  7:56     ` Richard Henderson
@ 2022-01-29  0:52       ` Atish Kumar Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-01-29  0:52 UTC (permalink / raw)
  To: Richard Henderson
  Cc: open list:RISC-V, Anup Patel, Bin Meng,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops.
@ 2022-01-29  0:52       ` Atish Kumar Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-01-29  0:52 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Anup Patel,
	Bin Meng, Alistair Francis, Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
  2022-01-26  8:37     ` Weiwei Li
@ 2022-01-29  1:28       ` Atish Patra
  -1 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-29  1:28 UTC (permalink / raw)
  To: Weiwei Li
  Cc: open list:RISC-V, Anup Patel, Bin Meng, Atish Patra,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
@ 2022-01-29  1:28       ` Atish Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Patra @ 2022-01-29  1:28 UTC (permalink / raw)
  To: Weiwei Li
  Cc: Atish Patra, qemu-devel@nongnu.org Developers, Anup Patel,
	Alistair Francis, Bin Meng, open list:RISC-V, Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
  2022-01-29  1:28       ` Atish Patra
  (?)
@ 2022-01-29  1:50       ` angell1518
  2022-01-31 20:05           ` Atish Kumar Patra
  -1 siblings, 1 reply; 34+ messages in thread
From: angell1518 @ 2022-01-29  1:50 UTC (permalink / raw)
  To: Atish Patra, Weiwei Li
  Cc: open list:RISC-V, Anup Patel, Bin Meng, Atish Patra,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
  2022-01-29  0:52       ` Atish Kumar Patra
@ 2022-01-31  6:33         ` Alistair Francis
  -1 siblings, 0 replies; 34+ messages in thread
From: Alistair Francis @ 2022-01-31  6:33 UTC (permalink / raw)
  To: Atish Kumar Patra
  Cc: open list:RISC-V, Anup Patel, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0
@ 2022-01-31  6:33         ` Alistair Francis
  0 siblings, 0 replies; 34+ messages in thread
From: Alistair Francis @ 2022-01-31  6:33 UTC (permalink / raw)
  To: Atish Kumar Patra
  Cc: Richard Henderson, open list:RISC-V, Anup Patel, Bin Meng,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
  2022-01-29  1:50       ` angell1518
@ 2022-01-31 20:05           ` Atish Kumar Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-01-31 20:05 UTC (permalink / raw)
  To: angell1518
  Cc: Weiwei Li, open list:RISC-V, Anup Patel, Bin Meng,
	qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
@ 2022-01-31 20:05           ` Atish Kumar Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-01-31 20:05 UTC (permalink / raw)
  To: angell1518
  Cc: Atish Patra, Weiwei Li, open list:RISC-V, Anup Patel, Bin Meng,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
  2022-01-20 20:07   ` Atish Patra
  (?)
  (?)
@ 2022-02-03 12:23   ` Heiko Stübner
  2022-02-03 20:39       ` Atish Kumar Patra
  -1 siblings, 1 reply; 34+ messages in thread
From: Heiko Stübner @ 2022-02-03 12:23 UTC (permalink / raw)
  To: qemu-devel, Atish Patra
  Cc: qemu-riscv, Anup Patel, Bin Meng, Atish Patra, Alistair Francis,
	Palmer Dabbelt

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




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
  2022-02-03 12:23   ` Heiko Stübner
@ 2022-02-03 20:39       ` Atish Kumar Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-02-03 20:39 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: open list:RISC-V, Anup Patel, Bin Meng,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
@ 2022-02-03 20:39       ` Atish Kumar Patra
  0 siblings, 0 replies; 34+ messages in thread
From: Atish Kumar Patra @ 2022-02-03 20:39 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Anup Patel,
	Bin Meng, Alistair Francis, Palmer Dabbelt

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2022-02-03 21:28 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20 20:07 [RFC 0/5] Privilege version update Atish Patra
2022-01-20 20:07 ` Atish Patra
2022-01-20 20:07 ` [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0 Atish Patra
2022-01-20 20:07   ` Atish Patra
2022-01-24  7:59   ` Richard Henderson
2022-01-24  7:59     ` Richard Henderson
2022-01-29  0:52     ` Atish Kumar Patra
2022-01-29  0:52       ` Atish Kumar Patra
2022-01-31  6:33       ` Alistair Francis
2022-01-31  6:33         ` Alistair Francis
2022-01-20 20:07 ` [RFC 2/5] target/riscv: Introduce privilege version field in the CSR ops Atish Patra
2022-01-20 20:07   ` Atish Patra
2022-01-24  7:56   ` Richard Henderson
2022-01-24  7:56     ` Richard Henderson
2022-01-29  0:52     ` Atish Kumar Patra
2022-01-29  0:52       ` Atish Kumar Patra
2022-01-20 20:07 ` [RFC 3/5] target/riscv: Add support for mconfigptr Atish Patra
2022-01-20 20:07   ` Atish Patra
2022-01-20 20:07 ` [RFC 4/5] target/riscv: Add *envcfg* CSRs support Atish Patra
2022-01-20 20:07   ` Atish Patra
2022-01-26  8:37   ` Weiwei Li
2022-01-26  8:37     ` Weiwei Li
2022-01-29  1:28     ` Atish Patra
2022-01-29  1:28       ` Atish Patra
2022-01-29  1:50       ` angell1518
2022-01-31 20:05         ` Atish Kumar Patra
2022-01-31 20:05           ` Atish Kumar Patra
2022-02-03 12:23   ` Heiko Stübner
2022-02-03 20:39     ` Atish Kumar Patra
2022-02-03 20:39       ` Atish Kumar Patra
2022-01-20 20:07 ` [RFC 5/5] target/riscv: Enable privileged spec version 1.12 Atish Patra
2022-01-20 20:07   ` Atish Patra
2022-01-24 13:32   ` Christoph Müllner
2022-01-24 13:32     ` Christoph Müllner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.