RISC-V Smstateen support

RFC PATCH Superseded Mayuresh Chitale 4 participants 22 messages Jun 3, 2022 – Jul 19, 2022 RISC-V

Share & tools

Mayuresh Chitale

author

2022-06-03 16:05 UTC #0 lore raw

v5 cover RISC-V Smstateen support

This series adds support for the Smstateen specification which provides a mechanism plug potential covert channels which are opened by extensions that add to processor state that may not get context-switched. Currently access to AIA registers, *envcfg registers and floating point(fcsr) is controlled via smstateen.

These patches can also be found on riscv_smstateen_v5 branch at:
https://github.com/mdchitale/qemu.git

This series depends on the following series from Anup:
https://lists.nongnu.org/archive/html/qemu-devel/2022-05/msg05231.html

Changes in v5:
- Fix the order in which smstateen extension is added to the isa_edata_arr as described in rule #3 the comment.

Changes in v4:
- Fix build issue with riscv32/riscv64-linux-user targets

Changes in v3:
- Fix coding style issues
- Fix *stateen0h index calculation

Changes in v2:
- Make h/s/envcfg bits in m/h/stateen registers as writeable by default.

Anup Patel (1):
target/riscv: Force disable extensions if priv spec version does not
match

Mayuresh Chitale (4):
target/riscv: Add smstateen support
target/riscv: smstateen check for h/senvcfg
target/riscv: smstateen check for fcsr
target/riscv: smstateen check for AIA/IMSIC

 target/riscv/cpu.c      |   2 +
 target/riscv/cpu.h      |   4 +
 target/riscv/cpu_bits.h |  36 +++
 target/riscv/csr.c      | 555 +++++++++++++++++++++++++++++++++++++++-
 target/riscv/machine.c  |  21 ++
 5 files changed, 615 insertions(+), 3 deletions(-)

Signature · 1 line

2.25.1

Mayuresh Chitale

author

2022-06-03 16:05 UTC #1 lore raw

RFC PATCH v5 1/4 target/riscv: Add smstateen support

Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.

This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.

Signed-off-by Mayuresh Chitale <[email protected]>

---
 target/riscv/cpu.c      |   2 +
 target/riscv/cpu.h      |   4 +
 target/riscv/cpu_bits.h |  36 +++++++
 target/riscv/csr.c      | 210 ++++++++++++++++++++++++++++++++++++++++
 target/riscv/machine.c  |  21 ++++
 5 files changed, 273 insertions(+)

target/riscv/cpu.c +2 -0

index e6e878ceb3..2d65ccd90f 100644
@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
     DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen, false),
 
     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
     DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
         ISA_EDATA_ENTRY(zve64f, ext_zve64f),
         ISA_EDATA_ENTRY(zhinx, ext_zhinx),
         ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
         ISA_EDATA_ENTRY(svinval, ext_svinval),
         ISA_EDATA_ENTRY(svnapot, ext_svnapot),
         ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
     /* CSRs for execution enviornment configuration */
     uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
     target_ulong senvcfg;
     uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
     bool ext_zhinxmin;
     bool ext_zve32f;
     bool ext_zve64f;
+    bool ext_smstateen;
 
     uint32_t mvendorid;
     uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +210 -0

228 lines not on this page · raw message

target/riscv/machine.c +21 -0

index 2a437b29a1..84a75dbb08 100644
@@ -262,6 +262,26 @@ static int riscv_cpu_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static bool smstateen_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+
+    return cpu->cfg.ext_smstateen;
+}
+
+static const VMStateDescription vmstate_smstateen = {
+    .name = "cpu/smtateen",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = smstateen_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.mstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.hstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.sstateen, RISCVCPU, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool envcfg_needed(void *opaque)
 {
     RISCVCPU *cpu = opaque;

@@ -347,6 +367,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_kvmtimer,
         &vmstate_envcfg,
         &vmstate_debug,
+        &vmstate_smstateen,
         NULL
     }
 };

Signature · 1 line

2.25.1

Alistair Francis

2022-06-16 05:49 UTC #2 lore raw

Re: [RFC PATCH v5 1/4] target/riscv: Add smstateen support

Quoted 21 lines · Mayuresh Chitale

Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.
This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.
Signed-off-by Mayuresh Chitale <[email protected]>
---
 target/riscv/cpu.c      |   2 +
 target/riscv/cpu.h      |   4 +
 target/riscv/cpu_bits.h |  36 +++++++
 target/riscv/csr.c      | 210 ++++++++++++++++++++++++++++++++++++++++
 target/riscv/machine.c  |  21 ++++
 5 files changed, 273 insertions(+)
target/riscv/cpu.c +0 -0
index e6e878ceb3..2d65ccd90f 100644

target/riscv/cpu.c static Property riscv_cpu_properties[] = { · line 916

Quoted 4 lines

target/riscv/cpu.c +1 -0

@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
     DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen, false),

This allows a user to enable this extension. It should be a seperate patch at the end of the series. The idea is that we add support, then once it's fully supported we allow a user to enable the extension.

Quoted 154 lines

target/riscv/cpu.c +1 -0

     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
     DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
         ISA_EDATA_ENTRY(zve64f, ext_zve64f),
         ISA_EDATA_ENTRY(zhinx, ext_zhinx),
         ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
         ISA_EDATA_ENTRY(svinval, ext_svinval),
         ISA_EDATA_ENTRY(svnapot, ext_svnapot),
         ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
     /* CSRs for execution enviornment configuration */
     uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
     target_ulong senvcfg;
     uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
     bool ext_zhinxmin;
     bool ext_zve32f;
     bool ext_zve64f;
+    bool ext_smstateen;
 
     uint32_t mvendorid;
     uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +45 -0

index 409a209f14..324fefce59 100644
@@ -247,6 +247,42 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
 
 }
 
+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return hmode(env, csrno);
+}
+
+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+
 /* Checks if PointerMasking registers could be accessed */
 static RISCVException pointer_masking(CPURISCVState *env, int csrno)
 {

@@ -1574,6 +1610,129 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+static inline void write_smstateen(CPURISCVState *env, uint64_t *reg,
+                                   uint64_t wr_mask, uint64_t new_val)
+{
+    *reg = (*reg & ~wr_mask) | (new_val & wr_mask);
+}
+
+static RISCVException read_mstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1631

Quoted 10 lines

target/riscv/csr.c +10 -0

+    *val = env->mstateen[csrno - CSR_MSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateen(CPURISCVState *env, int csrno,
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;

I'm not sure I understand, why is only bit 63 writable?

I guess you plan on expanding this, maybe make this a single const (or macro) that can be shared between them all?

Quoted 208 lines

target/riscv/csr.c +155 -0

+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_mstateenh(CPURISCVState *env, int csrno,
+                                     target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    write_smstateen(env, reg, wr_mask, val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_hstateen(CPURISCVState *env, int csrno,
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0;
+
+    reg = &env->hstateen[index];
+    wr_mask &= env->mstateen[index];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateenh(CPURISCVState *env, int csrno,
+                                     target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0H;
+
+    reg = &env->hstateen[index];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    wr_mask &= env->mstateen[index];
+
+    write_smstateen(env, reg, wr_mask, val);
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_sstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->sstateen[csrno - CSR_SSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_sstateen(CPURISCVState *env, int csrno,
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 0;
+    int index = csrno - CSR_SSTATEEN0;
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    reg = &env->sstateen[index];
+    if (virt) {
+        wr_mask &= env->mstateen[index];
+    } else {
+        wr_mask &= env->hstateen[index];
+    }
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException rmw_mip64(CPURISCVState *env, int csrno,
                                 uint64_t *ret_val,
                                 uint64_t new_val, uint64_t wr_mask)

@@ -3441,6 +3600,57 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh,
                                           .min_priv_ver = PRIV_VERSION_1_12_0 },
 
+    /* Smstateen extension CSRs */
+    [CSR_MSTATEEN0] = { "mstateen0", mstateen, read_mstateen, write_mstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN0H] = { "mstateen0h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN1] = { "mstateen1", mstateen, read_mstateen, write_mstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN1H] = { "mstateen1h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN2] = { "mstateen2", mstateen, read_mstateen, write_mstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN2H] = { "mstateen2h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN3] = { "mstateen3", mstateen, read_mstateen, write_mstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN3H] = { "mstateen3h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+
+    [CSR_HSTATEEN0] = { "hstateen0", hstateen, read_hstateen, write_hstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN0H] = { "hstateen0h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN1] = { "hstateen1", hstateen, read_hstateen, write_hstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN1H] = { "hstateen1h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN2] = { "hstateen2", hstateen, read_hstateen, write_hstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN2H] = { "hstateen2h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN3] = { "hstateen3", hstateen, read_hstateen, write_hstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN3H] = { "hstateen3h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+
+    [CSR_SSTATEEN0] = { "sstateen0", sstateen, read_sstateen, write_sstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_SSTATEEN1] = { "sstateen1", sstateen, read_sstateen, write_sstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_SSTATEEN2] = { "sstateen2", sstateen, read_sstateen, write_sstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_SSTATEEN3] = { "sstateen3", sstateen, read_sstateen, write_sstateen,
+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
     /* Supervisor Trap Setup */
     [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,    write_sstatus, NULL,
                                               read_sstatus_i128                 },

target/riscv/machine.c +21 -1

index 2a437b29a1..84a75dbb08 100644
@@ -262,6 +262,26 @@ static int riscv_cpu_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static bool smstateen_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+
+    return cpu->cfg.ext_smstateen;
+}
+
+static const VMStateDescription vmstate_smstateen = {
+    .name = "cpu/smtateen",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = smstateen_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.mstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.hstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.sstateen, RISCVCPU, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool envcfg_needed(void *opaque)
 {
     RISCVCPU *cpu = opaque;

@@ -347,6 +367,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_kvmtimer,
         &vmstate_envcfg,
         &vmstate_debug,
+        &vmstate_smstateen,
         NULL
     }
 };
--

2.25.1

Mayuresh Chitale

author

2022-07-07 15:42 UTC #3 lore raw

Re: [RFC PATCH v5 1/4] target/riscv: Add smstateen support

Quoted 206 lines · Alistair Francis

Quoted 21 lines · Mayuresh Chitale

Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.
This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.
Signed-off-by Mayuresh Chitale <[email protected]>
---
 target/riscv/cpu.c      |   2 +
 target/riscv/cpu.h      |   4 +
 target/riscv/cpu_bits.h |  36 +++++++
 target/riscv/csr.c      | 210
++++++++++++++++++++++++++++++++++++++++
 target/riscv/machine.c  |  21 ++++
 5 files changed, 273 insertions(+)
target/riscv/cpu.c +0 -0
index e6e878ceb3..2d65ccd90f 100644

target/riscv/cpu.c static Property riscv_cpu_properties[] = { · line 917

Quoted 5 lines

target/riscv/cpu.c +1 -0

@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
     DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen,
 false),

Quoted 166 lines

target/riscv/cpu.c +1 -0

     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
     DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU
 *cpu, char **isa_str, int max_str_len)
         ISA_EDATA_ENTRY(zve64f, ext_zve64f),
         ISA_EDATA_ENTRY(zhinx, ext_zhinx),
         ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
         ISA_EDATA_ENTRY(svinval, ext_svinval),
         ISA_EDATA_ENTRY(svnapot, ext_svnapot),

ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
     /* CSRs for execution enviornment configuration */
     uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
     target_ulong senvcfg;
     uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
     bool ext_zhinxmin;
     bool ext_zve32f;
     bool ext_zve64f;
+    bool ext_smstateen;
 
     uint32_t mvendorid;
     uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +36 -0

index 409a209f14..324fefce59 100644
@@ -247,6 +247,42 @@ static RISCVException hmode32(CPURISCVState
 *env, int csrno)
 
 }
 
+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return hmode(env, csrno);
+}
+
+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+
 /* Checks if PointerMasking registers could be accessed */
 static RISCVException pointer_masking(CPURISCVState *env, int

csrno)
{

target/riscv/csr.c +15 -0

@@ -1574,6 +1610,129 @@ static RISCVException
 write_henvcfgh(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+static inline void write_smstateen(CPURISCVState *env, uint64_t
 *reg,
+                                   uint64_t wr_mask, uint64_t
 new_val)
+{
+    *reg = (*reg & ~wr_mask) | (new_val & wr_mask);
+}
+
+static RISCVException read_mstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1631

Quoted 4 lines

target/riscv/csr.c +4 -0

+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;

I'm not sure I understand, why is only bit 63 writable?

Quoted 3 lines

I guess you plan on expanding this, maybe make this a single const (or
macro) that can be shared between them all?

Yes, it is expanded in the later patches depending on availability of other features.

Quoted 234 lines

target/riscv/csr.c +8 -0

+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_mstateenh(CPURISCVState *env, int

csrno,

target/riscv/csr.c +8 -0

+                                     target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateenh(CPURISCVState *env, int

csrno,

target/riscv/csr.c +23 -0

+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    write_smstateen(env, reg, wr_mask, val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_hstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c +14 -0

+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0;
+
+    reg = &env->hstateen[index];
+    wr_mask &= env->mstateen[index];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateenh(CPURISCVState *env, int

csrno,

target/riscv/csr.c +8 -0

+                                     target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_hstateenh(CPURISCVState *env, int

csrno,

target/riscv/csr.c +25 -0

+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0H;
+
+    reg = &env->hstateen[index];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    wr_mask &= env->mstateen[index];
+
+    write_smstateen(env, reg, wr_mask, val);
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_sstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->sstateen[csrno - CSR_SSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_sstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c +18 -0

+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 0;
+    int index = csrno - CSR_SSTATEEN0;
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    reg = &env->sstateen[index];
+    if (virt) {
+        wr_mask &= env->mstateen[index];
+    } else {
+        wr_mask &= env->hstateen[index];
+    }
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException rmw_mip64(CPURISCVState *env, int csrno,
                                 uint64_t *ret_val,
                                 uint64_t new_val, uint64_t

wr_mask)

target/riscv/csr.c +2 -0

@@ -3441,6 +3600,57 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE]
 = {
     [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
 write_henvcfgh,
                                           .min_priv_ver =
 PRIV_VERSION_1_12_0 },
 
+    /* Smstateen extension CSRs */
+    [CSR_MSTATEEN0] = { "mstateen0", mstateen, read_mstateen,

write_mstateen,

target/riscv/csr.c +5 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN0H] = { "mstateen0h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN1] = { "mstateen1", mstateen, read_mstateen,

write_mstateen,

target/riscv/csr.c +5 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN1H] = { "mstateen1h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN2] = { "mstateen2", mstateen, read_mstateen,

write_mstateen,

target/riscv/csr.c +5 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN2H] = { "mstateen2h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN3] = { "mstateen3", mstateen, read_mstateen,

write_mstateen,

target/riscv/csr.c +6 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_MSTATEEN3H] = { "mstateen3h", mstateen, read_mstateenh,
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+
+    [CSR_HSTATEEN0] = { "hstateen0", hstateen, read_hstateen,

write_hstateen,

target/riscv/csr.c +5 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN0H] = { "hstateen0h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN1] = { "hstateen1", hstateen, read_hstateen,

write_hstateen,

target/riscv/csr.c +5 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN1H] = { "hstateen1h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN2] = { "hstateen2", hstateen, read_hstateen,

write_hstateen,

target/riscv/csr.c +5 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN2H] = { "hstateen2h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN3] = { "hstateen3", hstateen, read_hstateen,

write_hstateen,

target/riscv/csr.c +6 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_HSTATEEN3H] = { "hstateen3h", hstateen, read_hstateenh,
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0 },
+
+    [CSR_SSTATEEN0] = { "sstateen0", sstateen, read_sstateen,

write_sstateen,

target/riscv/csr.c +2 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_SSTATEEN1] = { "sstateen1", sstateen, read_sstateen,

write_sstateen,

target/riscv/csr.c +2 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_SSTATEEN2] = { "sstateen2", sstateen, read_sstateen,

write_sstateen,

target/riscv/csr.c +2 -0

+                         .min_priv_ver = PRIV_VERSION_1_12_0 },
+    [CSR_SSTATEEN3] = { "sstateen3", sstateen, read_sstateen,

write_sstateen,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
/* Supervisor Trap Setup */
[CSR_SSTATUS] = { "sstatus", smode,
read_sstatus, write_sstatus, NULL,

                                               read_sstatus_i128   
              },

target/riscv/machine.c +20 -0

index 2a437b29a1..84a75dbb08 100644
@@ -262,6 +262,26 @@ static int riscv_cpu_post_load(void *opaque,
 int version_id)
     return 0;
 }
 
+static bool smstateen_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+
+    return cpu->cfg.ext_smstateen;
+}
+
+static const VMStateDescription vmstate_smstateen = {
+    .name = "cpu/smtateen",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = smstateen_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.mstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.hstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.sstateen, RISCVCPU, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool envcfg_needed(void *opaque)
 {

     RISCVCPU *cpu = opaque;

target/riscv/machine.c +1 -1

@@ -347,6 +367,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_kvmtimer,
         &vmstate_envcfg,
         &vmstate_debug,
+        &vmstate_smstateen,
         NULL
     }
 };
--

2.25.1

angell1518

2022-07-02 10:35 UTC #4 lore raw

Re:[RFC PATCH v5 1/4] target/riscv: Add smstateen support

In reply to Mayuresh Chitale

Quoted 140 lines · Mayuresh Chitale

Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.

This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.

Signed-off-by Mayuresh Chitale <[email protected]>

---

target/riscv/cpu.c      |   2 +
target/riscv/cpu.h      |   4 +
target/riscv/cpu_bits.h |  36 +++++++
target/riscv/csr.c      | 210 ++++++++++++++++++++++++++++++++++++++++
target/riscv/machine.c  |  21 ++++
5 files changed, 273 insertions(+)

target/riscv/cpu.c +2 -0

index e6e878ceb3..2d65ccd90f 100644
@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
    DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
    DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
    DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen, false),
 
    DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
    DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
        ISA_EDATA_ENTRY(zve64f, ext_zve64f),
        ISA_EDATA_ENTRY(zhinx, ext_zhinx),
        ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
        ISA_EDATA_ENTRY(svinval, ext_svinval),
        ISA_EDATA_ENTRY(svnapot, ext_svnapot),
        ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
    /* CSRs for execution enviornment configuration */
    uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
    target_ulong senvcfg;
    uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
    bool ext_zhinxmin;
    bool ext_zve32f;
    bool ext_zve64f;
+    bool ext_smstateen;
 
    uint32_t mvendorid;
    uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +14 -0

index 409a209f14..324fefce59 100644
@@ -247,6 +247,42 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
 
 }
 
+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{

target/riscv/csr.c static RISCVException hmode32(CPURISCVState *env, int csrno) · line 273

Quoted 10 lines

target/riscv/csr.c +10 -0

+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return hmode(env, csrno);
+}
+

I think we should take the stateen bit(bit 63) for matching mstateen into consideration here.

Similar to following sstateen function.

Quoted 66 lines

target/riscv/csr.c +59 -0

+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+
 /* Checks if PointerMasking registers could be accessed */
 static RISCVException pointer_masking(CPURISCVState *env, int csrno)
 {

@@ -1574,6 +1610,129 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
    return RISCV_EXCP_NONE;
 }
 
+static inline void write_smstateen(CPURISCVState *env, uint64_t *reg,
+                                   uint64_t wr_mask, uint64_t new_val)
+{
+    *reg = (*reg & ~wr_mask) | (new_val & wr_mask);
+}
+
+static RISCVException read_mstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateen(CPURISCVState *env, int csrno,
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_mstateenh(CPURISCVState *env, int csrno,
+                                     target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    write_smstateen(env, reg, wr_mask, val);
+
+    return RISCV_EXCP_NONE;

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1669

Quoted 10 lines

target/riscv/csr.c +10 -0

+}
+
+static RISCVException read_hstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+

I think we should also take mstateen into consideration here just like the following write function.
As the spec said, If we set certain bit in the matching mstateen csr to zero, the same bit in hstateen should be read-only zero. However, writes to mstateen will not affect the value in matching hstateen directly in current implementation,

so if we read hstateen just immediately after write zero to certain bit of mstateen, the same bit read from hstateen may not be zero.

Mayuresh Chitale

author

2022-07-07 16:53 UTC #5 lore raw

Re: [RFC PATCH v5 1/4] target/riscv: Add smstateen support

Quoted 152 lines · On Sat, 2022-07-02 at 18:19 +0800, angell1518 wrote:

At 2022-06-04 00:04:22, "Mayuresh Chitale" <[email protected]

Quoted 7 lines

wrote:
Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.
This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.

Quoted 6 lines

Signed-off-by Mayuresh Chitale <[email protected]>

---

target/riscv/cpu.c      |   2 +
target/riscv/cpu.h      |   4 +
target/riscv/cpu_bits.h |  36 +++++++
target/riscv/csr.c      | 210

++++++++++++++++++++++++++++++++++++++++

target/riscv/cpu.c static Property riscv_cpu_properties[] = { · line 916

Quoted 12 lines

target/riscv/machine.c | 21 ++++

5 files changed, 273 insertions(+)

target/riscv/cpu.c +1 -0

index e6e878ceb3..2d65ccd90f 100644
@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
    DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
    DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
    DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen,

false),

target/riscv/cpu.c static void riscv_isa_string_ext(RISCVCPU

Quoted 4 lines

target/riscv/cpu.c +0 -0

    DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
    DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU

*cpu, char **isa_str, int max_str_len)

target/riscv/csr.c static RISCVException hmode32(CPURISCVState

Quoted 95 lines

target/riscv/cpu.c +1 -0

        ISA_EDATA_ENTRY(zve64f, ext_zve64f),
        ISA_EDATA_ENTRY(zhinx, ext_zhinx),
        ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
        ISA_EDATA_ENTRY(svinval, ext_svinval),
        ISA_EDATA_ENTRY(svnapot, ext_svnapot),
        ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
    /* CSRs for execution enviornment configuration */
    uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
    target_ulong senvcfg;
    uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
    bool ext_zhinxmin;
    bool ext_zve32f;
    bool ext_zve64f;
+    bool ext_smstateen;
 
    uint32_t mvendorid;
    uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +0 -0

index 409a209f14..324fefce59 100644
@@ -247,6 +247,42 @@ static RISCVException hmode32(CPURISCVState

*env, int csrno)

Quoted 23 lines

target/riscv/csr.c +20 -0

 }
 
+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }

Quoted 8 lines

target/riscv/csr.c static RISCVException hmode32(CPURISCVState *env, int csrno) · line 273
Quoted 4 lines
target/riscv/csr.c +4 -0
+
+    return hmode(env, csrno);
+}
+
I think we should take the stateen bit(bit 63) for matching mstateen into consideration here.
Similar to following sstateen function.

ok.

Quoted 82 lines

Quoted 13 lines

target/riscv/csr.c +12 -0

+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+
 /* Checks if PointerMasking registers could be accessed */

Quoted 1 line

static RISCVException pointer_masking(CPURISCVState *env, int

csrno)

target/riscv/csr.c static RISCVException

Quoted 2 lines

{
target/riscv/csr.c +0 -0
@@ -1574,6 +1610,129 @@ static RISCVException

write_henvcfgh(CPURISCVState *env, int csrno,

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 4 lines

target/riscv/csr.c +1 -0

    return RISCV_EXCP_NONE;
 }
 
+static inline void write_smstateen(CPURISCVState *env, uint64_t

*reg,

target/riscv/csr.c static RISCVException · line 1614

Quoted 1 line

target/riscv/csr.c +1 -0

+                                   uint64_t wr_mask, uint64_t

new_val)

Quoted 23 lines

target/riscv/csr.c +23 -0

+{
+    *reg = (*reg & ~wr_mask) | (new_val & wr_mask);
+}
+
+static RISCVException read_mstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateen(CPURISCVState *env, int csrno,
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 10 lines

target/riscv/csr.c +10 -0

+
+static RISCVException read_mstateenh(CPURISCVState *env, int csrno,
+                                     target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateenh(CPURISCVState *env, int

csrno,

Quoted 12 lines

target/riscv/csr.c +12 -0

+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    write_smstateen(env, reg, wr_mask, val);
+
+    return RISCV_EXCP_NONE;

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1669

Quoted 10 lines

target/riscv/csr.c +10 -0

+}
+
+static RISCVException read_hstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+

Quoted 8 lines

I think we should also take mstateen into consideration here just like the following write function.
As the spec said, If we set certain bit in the matching mstateen csr to zero, the same bit in hstateen should be read-only zero. However, writes to mstateen will not affect the value in matching hstateen directly in current implementation,
so if we read hstateen just immediately after write zero to certain bit of mstateen, the same bit read from hstateen may not be zero.

I need to check this but as per my understanding this it is the responsibility of the software to clear the corresponding hstateen/stateen bits.

Quoted 93 lines

Weiwei Li

2022-07-07 23:45 UTC #6 lore raw

Re: [RFC PATCH v5 1/4] target/riscv: Add smstateen support

Quoted 249 lines · Mayuresh Chitale

Quoted 149 lines · On Sat, 2022-07-02 at 18:19 +0800, angell1518 wrote:

At 2022-06-04 00:04:22, "Mayuresh Chitale" <[email protected]

Quoted 7 lines

wrote:
Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.
This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.

Quoted 6 lines

Signed-off-by Mayuresh Chitale <[email protected]>

---

target/riscv/cpu.c      |   2 +
target/riscv/cpu.h      |   4 +
target/riscv/cpu_bits.h |  36 +++++++
target/riscv/csr.c      | 210

++++++++++++++++++++++++++++++++++++++++

target/riscv/cpu.c static Property riscv_cpu_properties[] = { · line 916

Quoted 12 lines

target/riscv/machine.c | 21 ++++

5 files changed, 273 insertions(+)

target/riscv/cpu.c +1 -0

index e6e878ceb3..2d65ccd90f 100644
@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
     DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen,

false),

target/riscv/cpu.c static void riscv_isa_string_ext(RISCVCPU

Quoted 3 lines

target/riscv/cpu.c +0 -0

     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
     DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU

*cpu, char **isa_str, int max_str_len)

target/riscv/csr.c static RISCVException hmode32(CPURISCVState

Quoted 95 lines

target/riscv/cpu.c +1 -0

         ISA_EDATA_ENTRY(zve64f, ext_zve64f),
         ISA_EDATA_ENTRY(zhinx, ext_zhinx),
         ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
         ISA_EDATA_ENTRY(svinval, ext_svinval),
         ISA_EDATA_ENTRY(svnapot, ext_svnapot),
         ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
     /* CSRs for execution enviornment configuration */
     uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
     target_ulong senvcfg;
     uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
     bool ext_zhinxmin;
     bool ext_zve32f;
     bool ext_zve64f;
+    bool ext_smstateen;
 
     uint32_t mvendorid;
     uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +0 -0

index 409a209f14..324fefce59 100644
@@ -247,6 +247,42 @@ static RISCVException hmode32(CPURISCVState

*env, int csrno)

Quoted 21 lines

target/riscv/csr.c +19 -0

 }
 
+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;

Quoted 8 lines

target/riscv/csr.c static RISCVException hmode32(CPURISCVState *env, int csrno) · line 273
Quoted 5 lines
target/riscv/csr.c +5 -0
+    }
+
+    return hmode(env, csrno);
+}
+
I think we should take the stateen bit(bit 63) for matching mstateen into consideration here.
Similar to following sstateen function.

ok.

Quoted 81 lines

Quoted 13 lines

target/riscv/csr.c +12 -0

+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+
 /* Checks if PointerMasking registers could be accessed */

Quoted 1 line

static RISCVException pointer_masking(CPURISCVState *env, int

csrno)

target/riscv/csr.c static RISCVException

Quoted 2 lines

{
target/riscv/csr.c +0 -0
@@ -1574,6 +1610,129 @@ static RISCVException

write_henvcfgh(CPURISCVState *env, int csrno,

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 4 lines

target/riscv/csr.c +1 -0

     return RISCV_EXCP_NONE;
 }
 
+static inline void write_smstateen(CPURISCVState *env, uint64_t

*reg,

target/riscv/csr.c static RISCVException · line 1614

Quoted 1 line

target/riscv/csr.c +1 -0

+                                   uint64_t wr_mask, uint64_t

new_val)

Quoted 23 lines

target/riscv/csr.c +23 -0

+{
+    *reg = (*reg & ~wr_mask) | (new_val & wr_mask);
+}
+
+static RISCVException read_mstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateen(CPURISCVState *env, int csrno,
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 10 lines

target/riscv/csr.c +10 -0

+
+static RISCVException read_mstateenh(CPURISCVState *env, int csrno,
+                                     target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateenh(CPURISCVState *env, int

csrno,

Quoted 12 lines

target/riscv/csr.c +12 -0

+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    write_smstateen(env, reg, wr_mask, val);
+
+    return RISCV_EXCP_NONE;

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1669

Quoted 10 lines

target/riscv/csr.c +10 -0

+}
+
+static RISCVException read_hstateen(CPURISCVState *env, int csrno,
+                                    target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+

Quoted 8 lines

I think we should also take mstateen into consideration here just like the following write function.
As the spec said, If we set certain bit in the matching mstateen csr to zero, the same bit in hstateen should be read-only zero. However, writes to mstateen will not affect the value in matching hstateen directly in current implementation,
so if we read hstateen just immediately after write zero to certain bit of mstateen, the same bit read from hstateen may not be zero.

Quoted 3 lines

I need to check this but as per my understanding this it is the responsibility of the software to clear the corresponding hstateen/stateen bits.

I have sent a issue for how the hstateen/sstateen become read-only zero for zero bits in mstateen:

https://github.com/riscv/riscv-state-enable/issues/9

Regards,

Weiwei Li

Quoted 201 lines

Quoted 91 lines

Mayuresh Chitale

author

2022-07-18 15:38 UTC #7 lore raw

Re: [RFC PATCH v5 1/4] target/riscv: Add smstateen support

Quoted 265 lines · Weiwei Li

Quoted 260 lines · Mayuresh Chitale

Quoted 152 lines · On Sat, 2022-07-02 at 18:19 +0800, angell1518 wrote:

At 2022-06-04 00:04:22, "Mayuresh Chitale" <
[email protected]

Quoted 7 lines

wrote:
Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.
This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.

Quoted 6 lines

Signed-off-by Mayuresh Chitale <[email protected]>

---

target/riscv/cpu.c      |   2 +
target/riscv/cpu.h      |   4 +
target/riscv/cpu_bits.h |  36 +++++++
target/riscv/csr.c      | 210

++++++++++++++++++++++++++++++++++++++++

target/riscv/cpu.c static Property riscv_cpu_properties[] = { · line 916

Quoted 12 lines

target/riscv/machine.c | 21 ++++

5 files changed, 273 insertions(+)

target/riscv/cpu.c +1 -0

index e6e878ceb3..2d65ccd90f 100644
@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
     DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen,

false),

target/riscv/cpu.c static void riscv_isa_string_ext(RISCVCPU

Quoted 3 lines

target/riscv/cpu.c +0 -0

     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
     DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU

*cpu, char **isa_str, int max_str_len)

Quoted 95 lines

target/riscv/cpu.c +1 -0

         ISA_EDATA_ENTRY(zve64f, ext_zve64f),
         ISA_EDATA_ENTRY(zhinx, ext_zhinx),
         ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
         ISA_EDATA_ENTRY(svinval, ext_svinval),
         ISA_EDATA_ENTRY(svnapot, ext_svnapot),
         ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
     /* CSRs for execution enviornment configuration */
     uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
     target_ulong senvcfg;
     uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
     bool ext_zhinxmin;
     bool ext_zve32f;
     bool ext_zve64f;
+    bool ext_smstateen;
 
     uint32_t mvendorid;
     uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +0 -0

index 409a209f14..324fefce59 100644
@@ -247,6 +247,42 @@ static RISCVException

Quoted 1 line

hmode32(CPURISCVState

*env, int csrno)

Quoted 22 lines

}

target/riscv/csr.c +20 -0

+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }

Quoted 8 lines

target/riscv/csr.c static RISCVException hmode32(CPURISCVState *env, int csrno) · line 273
Quoted 4 lines
target/riscv/csr.c +4 -0
+
+    return hmode(env, csrno);
+}
+
I think we should take the stateen bit(bit 63) for matching mstateen
into consideration here.
Similar to following sstateen function.

ok.

Quoted 89 lines

Quoted 12 lines

target/riscv/csr.c +12 -0

+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+

Quoted 2 lines

/* Checks if PointerMasking registers could be accessed */ static RISCVException pointer_masking(CPURISCVState *env, int

csrno)

target/riscv/csr.c static RISCVException

Quoted 2 lines

{
target/riscv/csr.c +0 -0
@@ -1574,6 +1610,129 @@ static RISCVException

write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 5 lines

target/riscv/csr.c +1 -0

     return RISCV_EXCP_NONE;
 }
 
+static inline void write_smstateen(CPURISCVState *env,

uint64_t

*reg,

Quoted 1 line

+ uint64_t wr_mask, uint64_t

new_val)

Quoted 28 lines

target/riscv/csr.c +5 -0

+{
+    *reg = (*reg & ~wr_mask) | (new_val & wr_mask);
+}
+
+static RISCVException read_mstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c +8 -0

+                                    target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c +12 -0

+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_mstateenh(CPURISCVState *env, int

csrno,

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 8 lines

target/riscv/csr.c +8 -0

+                                     target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateenh(CPURISCVState *env, int

csrno,

Quoted 16 lines

target/riscv/csr.c +15 -0

+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    write_smstateen(env, reg, wr_mask, val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1669

Quoted 7 lines

target/riscv/csr.c +7 -0

+                                    target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+

I think we should also take mstateen into consideration here just like the following write function.
As the spec said, If we set certain bit in the matching mstateen

Quoted 8 lines

csr
to zero, the same bit in hstateen should be read-only zero. However, writes to mstateen will not affect the value in matching hstateen directly in current implementation,
so if we read hstateen just immediately after write zero to certain
bit of mstateen, the same bit read from hstateen may not be zero.

Quoted 3 lines

I need to check this but as per my understanding this it is the responsibility of the software to clear the corresponding hstateen/stateen bits.

Quoted 5 lines

I have sent a issue for how the hstateen/sstateen become read-only zero
for zero bits in mstateen:
https://github.com/riscv/riscv-state-enable/issues/9

Thanks for the information. I think as per the dicussion on the above issue it looks like the current implementation is also valid.

Quoted 244 lines

Regards,
Weiwei Li
Quoted 239 lines
Quoted 100 lines
similar to read function of sstateen.
Quoted 17 lines
+static RISCVException write_hstateen(CPURISCVState *env, int csrno,
target/riscv/csr.c +14 -0
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0;
+
+    reg = &env->hstateen[index];
+    wr_mask &= env->mstateen[index];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateenh(CPURISCVState *env, int
csrno,
target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
Quoted 8 lines
target/riscv/csr.c +8 -0
+                                     target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_hstateenh(CPURISCVState *env, int
csrno,
Quoted 30 lines
target/riscv/csr.c +17 -0
+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0H;
+
+    reg = &env->hstateen[index];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    wr_mask &= env->mstateen[index];
+
+    write_smstateen(env, reg, wr_mask, val);
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_sstateen(CPURISCVState *env, int
csrno,
target/riscv/csr.c +8 -0
+                                    target_ulong *val)
+{
+    *val = env->sstateen[csrno - CSR_SSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_sstateen(CPURISCVState *env, int
csrno,
target/riscv/csr.c +3 -0
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1730
Quoted 10 lines
target/riscv/csr.c +10 -0
+    uint64_t wr_mask = 0;
+    int index = csrno - CSR_SSTATEEN0;
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    reg = &env->sstateen[index];
+    if (virt) {
+        wr_mask &= env->mstateen[index];
+    } else {
+        wr_mask &= env->hstateen[index];
+    }
Why this? I think hstateen works in virt mode, so this may be:
if (virt) {
   wr_mask &= env->mstateen[index] & env->hstateen[index];;
} else {
wr_mask &= env->mstateen[index];
}
Quoted 11 lines
target/riscv/csr.c +5 -0
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
static RISCVException rmw_mip64(CPURISCVState *env, int csrno,
                                 uint64_t *ret_val,
                                 uint64_t new_val, uint64_t
wr_mask)
target/riscv/csr.c +0 -0
@@ -3441,6 +3600,57 @@ riscv_csr_operations
csr_ops[CSR_TABLE_SIZE]
= {
Quoted 1 line
[CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
write_henvcfgh,
Quoted 1 line
                                           .min_priv_ver =
PRIV_VERSION_1_12_0 },
target/riscv/csr.c
Quoted 2 lines
target/riscv/csr.c +2 -0
+    /* Smstateen extension CSRs */
+    [CSR_MSTATEEN0] = { "mstateen0", mstateen, read_mstateen,
write_mstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN0H] = { "mstateen0h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN1] = { "mstateen1", mstateen, read_mstateen,
Quoted 8 lines
write_mstateen,
Quoted 2 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
It seems unreasonable for mstateen1~3 to share the same read/write
function with mstateen0 ,
since their field mask is different.
Similar to sstateen1~3 and hstateen1~3
I will fix it in the next version.
Quoted 130 lines
Regards,
Weiwei Li
Quoted 6 lines
+ [CSR_MSTATEEN1H] = { "mstateen1h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN2] = { "mstateen2", mstateen, read_mstateen,
write_mstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN2H] = { "mstateen2h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN3] = { "mstateen3", mstateen, read_mstateen,
write_mstateen,
target/riscv/csr.c
Quoted 9 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN3H] = { "mstateen3h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
target/riscv/csr.c +2 -0
+
+    [CSR_HSTATEEN0] = { "hstateen0", hstateen, read_hstateen,
write_hstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN0H] = { "hstateen0h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN1] = { "hstateen1", hstateen, read_hstateen,
write_hstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN1H] = { "hstateen1h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN2] = { "hstateen2", hstateen, read_hstateen,
write_hstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN2H] = { "hstateen2h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN3] = { "hstateen3", hstateen, read_hstateen,
write_hstateen,
target/riscv/csr.c
Quoted 9 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN3H] = { "hstateen3h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
target/riscv/csr.c +2 -0
+
+    [CSR_SSTATEEN0] = { "sstateen0", sstateen, read_sstateen,
write_sstateen,
Quoted 3 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_SSTATEEN1] = { "sstateen1", sstateen, read_sstateen,
write_sstateen,
Quoted 3 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_SSTATEEN2] = { "sstateen2", sstateen, read_sstateen,
write_sstateen,
Quoted 3 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_SSTATEEN3] = { "sstateen3", sstateen, read_sstateen,
write_sstateen,
Quoted 4 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
/* Supervisor Trap Setup */
[CSR_SSTATUS] = { "sstatus", smode,
read_sstatus, write_sstatus, NULL,
Quoted 2 lines
read_sstatus_i12
8
              },
target/riscv/machine.c static int riscv_cpu_post_load(void · line 262
Quoted 6 lines
target/riscv/machine.c +0 -0
index 2a437b29a1..84a75dbb08 100644
@@ -262,6 +262,26 @@ static int riscv_cpu_post_load(void
 *opaque,
int version_id)
Quoted 38 lines
target/riscv/machine.c +20 -0
     return 0;
 }
 
+static bool smstateen_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+
+    return cpu->cfg.ext_smstateen;
+}
+
+static const VMStateDescription vmstate_smstateen = {
+    .name = "cpu/smtateen",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = smstateen_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.mstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.hstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.sstateen, RISCVCPU, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool envcfg_needed(void *opaque)
 {
     RISCVCPU *cpu = opaque;
target/riscv/machine.c +1 -0
@@ -347,6 +367,7 @@ const VMStateDescription vmstate_riscv_cpu
 = {
         &vmstate_kvmtimer,
         &vmstate_envcfg,
         &vmstate_debug,
+        &vmstate_smstateen,
         NULL
     }
};
Signature · 1 line
2.25.1

Weiwei Li

2022-07-19 01:03 UTC #8 lore raw

Re: [RFC PATCH v5 1/4] target/riscv: Add smstateen support

Quoted 265 lines · Mayuresh Chitale

Quoted 264 lines · Weiwei Li

Quoted 263 lines · Mayuresh Chitale

Quoted 152 lines · On Sat, 2022-07-02 at 18:19 +0800, angell1518 wrote:

At 2022-06-04 00:04:22, "Mayuresh Chitale" <
[email protected]

Quoted 7 lines

wrote:
Smstateen extension specifies a mechanism to close the potential covert channels that could cause security issues.
This patch adds the CSRs defined in the specification and the corresponding predicates and read/write functions.

Quoted 6 lines

Signed-off-by Mayuresh Chitale <[email protected]>

---

target/riscv/cpu.c      |   2 +
target/riscv/cpu.h      |   4 +
target/riscv/cpu_bits.h |  36 +++++++
target/riscv/csr.c      | 210

++++++++++++++++++++++++++++++++++++++++

target/riscv/cpu.c static Property riscv_cpu_properties[] = { · line 916

Quoted 12 lines

target/riscv/machine.c | 21 ++++

5 files changed, 273 insertions(+)

target/riscv/cpu.c +1 -0

index e6e878ceb3..2d65ccd90f 100644
@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
      DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
      DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
      DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen,

false),

target/riscv/cpu.c static void riscv_isa_string_ext(RISCVCPU

Quoted 3 lines

target/riscv/cpu.c +0 -0

      DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
      DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),

@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU

*cpu, char **isa_str, int max_str_len)

Quoted 95 lines

target/riscv/cpu.c +1 -0

          ISA_EDATA_ENTRY(zve64f, ext_zve64f),
          ISA_EDATA_ENTRY(zhinx, ext_zhinx),
          ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+        ISA_EDATA_ENTRY(smstateen, ext_smstateen),
          ISA_EDATA_ENTRY(svinval, ext_svinval),
          ISA_EDATA_ENTRY(svnapot, ext_svnapot),
          ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),

target/riscv/cpu.h +4 -0

index f08c3e8813..1c269b77bd 100644
@@ -327,6 +327,9 @@ struct CPUArchState {
 
      /* CSRs for execution enviornment configuration */
      uint64_t menvcfg;
+    uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+    uint64_t sstateen[SMSTATEEN_MAX_COUNT];
      target_ulong senvcfg;
      uint64_t henvcfg;
 #endif

@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
      bool ext_zhinxmin;
      bool ext_zve32f;
      bool ext_zve64f;
+    bool ext_smstateen;
 
      uint32_t mvendorid;
      uint64_t marchid;

target/riscv/cpu_bits.h +36 -0

index 4a55c6a709..2a3ef26d21 100644
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG         0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0       0x10C
+#define CSR_SSTATEEN1       0x10D
+#define CSR_SSTATEEN2       0x10E
+#define CSR_SSTATEEN3       0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141

@@ -257,6 +263,16 @@
 #define CSR_HENVCFG         0x60A
 #define CSR_HENVCFGH        0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0       0x60C
+#define CSR_HSTATEEN0H      0x61C
+#define CSR_HSTATEEN1       0x60D
+#define CSR_HSTATEEN1H      0x61D
+#define CSR_HSTATEEN2       0x60E
+#define CSR_HSTATEEN2H      0x61E
+#define CSR_HSTATEEN3       0x60F
+#define CSR_HSTATEEN3H      0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204

@@ -304,6 +320,26 @@
 #define CSR_MENVCFG         0x30A
 #define CSR_MENVCFGH        0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0       0x30C
+#define CSR_MSTATEEN0H      0x31C
+#define CSR_MSTATEEN1       0x30D
+#define CSR_MSTATEEN1H      0x31D
+#define CSR_MSTATEEN2       0x30E
+#define CSR_MSTATEEN2H      0x31E
+#define CSR_MSTATEEN3       0x30F
+#define CSR_MSTATEEN3H      0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS       0
+#define SMSTATEEN0_FCSR     0
+#define SMSTATEEN0_IMSIC    58
+#define SMSTATEEN0_AIA      59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN    63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757

target/riscv/csr.c +0 -0

index 409a209f14..324fefce59 100644
@@ -247,6 +247,42 @@ static RISCVException

Quoted 1 line

hmode32(CPURISCVState

*env, int csrno)

Quoted 22 lines

}

target/riscv/csr.c +20 -0

+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }

Quoted 8 lines

target/riscv/csr.c static RISCVException hmode32(CPURISCVState *env, int csrno) · line 273
Quoted 4 lines
target/riscv/csr.c +4 -0
+
+    return hmode(env, csrno);
+}
+
I think we should take the stateen bit(bit 63) for matching mstateen
into consideration here.
Similar to following sstateen function.

ok.

Quoted 89 lines

Quoted 12 lines

target/riscv/csr.c +12 -0

+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+

Quoted 2 lines

/* Checks if PointerMasking registers could be accessed */ static RISCVException pointer_masking(CPURISCVState *env, int

csrno)

target/riscv/csr.c static RISCVException

Quoted 2 lines

{
target/riscv/csr.c +0 -0
@@ -1574,6 +1610,129 @@ static RISCVException

write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 5 lines

target/riscv/csr.c +1 -0

      return RISCV_EXCP_NONE;
 }
 
+static inline void write_smstateen(CPURISCVState *env,

uint64_t

*reg,

Quoted 1 line

+ uint64_t wr_mask, uint64_t

new_val)

Quoted 28 lines

target/riscv/csr.c +5 -0

+{
+    *reg = (*reg & ~wr_mask) | (new_val & wr_mask);
+}
+
+static RISCVException read_mstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c +8 -0

+                                    target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c +12 -0

+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_mstateenh(CPURISCVState *env, int

csrno,

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,

Quoted 8 lines

target/riscv/csr.c +8 -0

+                                     target_ulong *val)
+{
+    *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mstateenh(CPURISCVState *env, int

csrno,

Quoted 16 lines

target/riscv/csr.c +15 -0

+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+
+    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    write_smstateen(env, reg, wr_mask, val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateen(CPURISCVState *env, int

csrno,

target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1669

Quoted 7 lines

target/riscv/csr.c +7 -0

+                                    target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+

I think we should also take mstateen into consideration here just like the following write function.
As the spec said, If we set certain bit in the matching mstateen

Quoted 8 lines

csr
to zero, the same bit in hstateen should be read-only zero. However, writes to mstateen will not affect the value in matching hstateen directly in current implementation,
so if we read hstateen just immediately after write zero to certain
bit of mstateen, the same bit read from hstateen may not be zero.

I need to check this but as per my understanding this it is the responsibility of the software to clear the corresponding hstateen/stateen bits.

Quoted 7 lines

Quoted 5 lines
I have sent a issue for how the hstateen/sstateen become read-only zero
for zero bits in mstateen:
https://github.com/riscv/riscv-state-enable/issues/9
Thanks for the information. I think as per the dicussion on the above issue it looks like the current implementation is also valid.

I think the way to implement the read-only zero is unspecified. However, the read out value for

the read-only zero fields should be zero.

Regards,

Weiwei Li

Quoted 243 lines

Quoted 243 lines
Regards,
Weiwei Li
Quoted 239 lines
Quoted 100 lines
similar to read function of sstateen.
Quoted 17 lines
+static RISCVException write_hstateen(CPURISCVState *env, int csrno,
target/riscv/csr.c +14 -0
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0;
+
+    reg = &env->hstateen[index];
+    wr_mask &= env->mstateen[index];
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_hstateenh(CPURISCVState *env, int
csrno,
target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
Quoted 8 lines
target/riscv/csr.c +8 -0
+                                     target_ulong *val)
+{
+    *val = env->hstateen[csrno - CSR_HSTATEEN0H] >> 32;
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_hstateenh(CPURISCVState *env, int
csrno,
Quoted 30 lines
target/riscv/csr.c +17 -0
+                                      target_ulong new_val)
+{
+    uint64_t *reg;
+    uint64_t val;
+    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    int index = csrno - CSR_HSTATEEN0H;
+
+    reg = &env->hstateen[index];
+    val = (uint64_t)new_val << 32;
+    val |= *reg & 0xFFFFFFFF;
+    wr_mask &= env->mstateen[index];
+
+    write_smstateen(env, reg, wr_mask, val);
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_sstateen(CPURISCVState *env, int
csrno,
target/riscv/csr.c +8 -0
+                                    target_ulong *val)
+{
+    *val = env->sstateen[csrno - CSR_SSTATEEN0];
+
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_sstateen(CPURISCVState *env, int
csrno,
target/riscv/csr.c +3 -0
+                                     target_ulong new_val)
+{
+    uint64_t *reg;
target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1730
Quoted 10 lines
target/riscv/csr.c +10 -0
+    uint64_t wr_mask = 0;
+    int index = csrno - CSR_SSTATEEN0;
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    reg = &env->sstateen[index];
+    if (virt) {
+        wr_mask &= env->mstateen[index];
+    } else {
+        wr_mask &= env->hstateen[index];
+    }
Why this? I think hstateen works in virt mode, so this may be:
if (virt) {
    wr_mask &= env->mstateen[index] & env->hstateen[index];;
} else {
    wr_mask &= env->mstateen[index];
}
Quoted 11 lines
target/riscv/csr.c +5 -0
+    write_smstateen(env, reg, wr_mask, new_val);
+
+    return RISCV_EXCP_NONE;
+}
+
static RISCVException rmw_mip64(CPURISCVState *env, int csrno,
                                  uint64_t *ret_val,
                                  uint64_t new_val, uint64_t
wr_mask)
target/riscv/csr.c +0 -0
@@ -3441,6 +3600,57 @@ riscv_csr_operations
csr_ops[CSR_TABLE_SIZE]
= {
Quoted 1 line
[CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
write_henvcfgh,
Quoted 1 line
                                            .min_priv_ver =
PRIV_VERSION_1_12_0 },
target/riscv/csr.c
Quoted 2 lines
target/riscv/csr.c +2 -0
+    /* Smstateen extension CSRs */
+    [CSR_MSTATEEN0] = { "mstateen0", mstateen, read_mstateen,
write_mstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN0H] = { "mstateen0h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN1] = { "mstateen1", mstateen, read_mstateen,
Quoted 8 lines
write_mstateen,
Quoted 2 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
It seems unreasonable for mstateen1~3 to share the same read/write
function with mstateen0 ,
since their field mask is different.
Similar to sstateen1~3 and hstateen1~3
I will fix it in the next version.
Quoted 130 lines
Regards,
Weiwei Li
Quoted 6 lines
+ [CSR_MSTATEEN1H] = { "mstateen1h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN2] = { "mstateen2", mstateen, read_mstateen,
write_mstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN2H] = { "mstateen2h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN3] = { "mstateen3", mstateen, read_mstateen,
write_mstateen,
target/riscv/csr.c
Quoted 9 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_MSTATEEN3H] = { "mstateen3h", mstateen,
read_mstateenh,
target/riscv/csr.c +2 -0
+                          write_mstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
target/riscv/csr.c +2 -0
+
+    [CSR_HSTATEEN0] = { "hstateen0", hstateen, read_hstateen,
write_hstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN0H] = { "hstateen0h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN1] = { "hstateen1", hstateen, read_hstateen,
write_hstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN1H] = { "hstateen1h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN2] = { "hstateen2", hstateen, read_hstateen,
write_hstateen,
Quoted 8 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN2H] = { "hstateen2h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN3] = { "hstateen3", hstateen, read_hstateen,
write_hstateen,
target/riscv/csr.c
Quoted 9 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_HSTATEEN3H] = { "hstateen3h", hstateen,
read_hstateenh,
target/riscv/csr.c +2 -0
+                          write_hstateenh,
+                          .min_priv_ver = PRIV_VERSION_1_12_0
},
target/riscv/csr.c +2 -0
+
+    [CSR_SSTATEEN0] = { "sstateen0", sstateen, read_sstateen,
write_sstateen,
Quoted 3 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_SSTATEEN1] = { "sstateen1", sstateen, read_sstateen,
write_sstateen,
Quoted 3 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_SSTATEEN2] = { "sstateen2", sstateen, read_sstateen,
write_sstateen,
Quoted 3 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
+ [CSR_SSTATEEN3] = { "sstateen3", sstateen, read_sstateen,
write_sstateen,
Quoted 4 lines
+ .min_priv_ver = PRIV_VERSION_1_12_0
},
/* Supervisor Trap Setup */
[CSR_SSTATUS] = { "sstatus", smode,
read_sstatus, write_sstatus, NULL,
Quoted 2 lines
read_sstatus_i12
8
               },
target/riscv/machine.c static int riscv_cpu_post_load(void · line 262
Quoted 6 lines
target/riscv/machine.c +0 -0
index 2a437b29a1..84a75dbb08 100644
@@ -262,6 +262,26 @@ static int riscv_cpu_post_load(void
 *opaque,
int version_id)
Quoted 38 lines
target/riscv/machine.c +20 -0
      return 0;
 }
 
+static bool smstateen_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+
+    return cpu->cfg.ext_smstateen;
+}
+
+static const VMStateDescription vmstate_smstateen = {
+    .name = "cpu/smtateen",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = smstateen_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.mstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.hstateen, RISCVCPU, 4),
+        VMSTATE_UINT64_ARRAY(env.sstateen, RISCVCPU, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool envcfg_needed(void *opaque)
 {
      RISCVCPU *cpu = opaque;
target/riscv/machine.c +1 -0
@@ -347,6 +367,7 @@ const VMStateDescription vmstate_riscv_cpu
 = {
          &vmstate_kvmtimer,
          &vmstate_envcfg,
          &vmstate_debug,
+        &vmstate_smstateen,
          NULL
      }
};
Signature · 1 line
2.25.1

Mayuresh Chitale

author

2022-06-03 16:05 UTC #9 lore raw

RFC PATCH v5 2/4 target/riscv: smstateen check for h/senvcfg

In reply to Mayuresh Chitale

Accesses to henvcfg, henvcfgh and senvcfg are allowed only if corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an illegal instruction trap is generated.

Signed-off-by Mayuresh Chitale <[email protected]>

---
 target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 6 deletions(-)

target/riscv/csr.c +78 -6

index 324fefce59..ae91ae1f7e 100644
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+#if !defined(CONFIG_USER_ONLY)
+    if (!(env->mstateen[0] & 1UL << bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    if (virt) {
+        if (!(env->hstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (mode == PRV_U) {
+        if (!(env->sstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)

@@ -1557,6 +1588,13 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->senvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1565,15 +1603,27 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+    RISCVException ret;
 
-    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1582,6 +1632,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
     if (riscv_cpu_mxl(env) == MXL_RV64) {
         mask |= HENVCFG_PBMTE | HENVCFG_STCE;

@@ -1595,6 +1651,13 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg >> 32;
     return RISCV_EXCP_NONE;
 }

@@ -1604,9 +1667,14 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
 {
     uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
     uint64_t valh = (uint64_t)val << 32;
+    RISCVException ret;
 
-    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
     return RISCV_EXCP_NONE;
 }

@@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;

@@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
     reg = &env->hstateen[index];

@@ -1694,8 +1765,9 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
     int index = csrno - CSR_HSTATEEN0H;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;

Signature · 1 line

2.25.1

Alistair Francis

2022-06-16 06:54 UTC #10 lore raw

Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg

Quoted 54 lines · Mayuresh Chitale

Accesses to henvcfg, henvcfgh and senvcfg are allowed only if corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an illegal instruction trap is generated.

Signed-off-by Mayuresh Chitale <[email protected]>

---
 target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 6 deletions(-)

target/riscv/csr.c +31 -0

index 324fefce59..ae91ae1f7e 100644
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+#if !defined(CONFIG_USER_ONLY)
+    if (!(env->mstateen[0] & 1UL << bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    if (virt) {
+        if (!(env->hstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (mode == PRV_U) {
+        if (!(env->sstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)

target/riscv/csr.c static RISCVException write_menvcfgh(CPURISCVState *env, int csrno, · line 1593

Quoted 6 lines

target/riscv/csr.c +3 -0

@@ -1557,6 +1588,13 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);

Couldn't this be part of the original permission check so we don't need a second check?

Alistair

Quoted 124 lines

target/riscv/csr.c +44 -7

+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->senvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1565,15 +1603,27 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+    RISCVException ret;
 
-    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1582,6 +1632,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
     if (riscv_cpu_mxl(env) == MXL_RV64) {
         mask |= HENVCFG_PBMTE | HENVCFG_STCE;

@@ -1595,6 +1651,13 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg >> 32;
     return RISCV_EXCP_NONE;
 }

@@ -1604,9 +1667,14 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
 {
     uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
     uint64_t valh = (uint64_t)val << 32;
+    RISCVException ret;
 
-    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
     return RISCV_EXCP_NONE;
 }

@@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;

@@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
     reg = &env->hstateen[index];

@@ -1694,8 +1765,9 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
     int index = csrno - CSR_HSTATEEN0H;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
--

2.25.1

Alistair Francis

2022-06-16 06:56 UTC #11 lore raw

Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg

Quoted 58 lines · Alistair Francis

Quoted 55 lines · Mayuresh Chitale

Accesses to henvcfg, henvcfgh and senvcfg are allowed only if corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an illegal instruction trap is generated.

Signed-off-by Mayuresh Chitale <[email protected]>

---
 target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 6 deletions(-)

target/riscv/csr.c +31 -0

index 324fefce59..ae91ae1f7e 100644
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+#if !defined(CONFIG_USER_ONLY)
+    if (!(env->mstateen[0] & 1UL << bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    if (virt) {
+        if (!(env->hstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (mode == PRV_U) {
+        if (!(env->sstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)

@@ -1557,6 +1588,13 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,

Quoted 8 lines

target/riscv/csr.c static RISCVException write_menvcfgh(CPURISCVState *env, int csrno, · line 1593
Quoted 5 lines
target/riscv/csr.c +3 -0
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
Couldn't this be part of the original permission check so we don't need a second check?

Whoops, misread the function. Ignore that

Alistair

Quoted 127 lines

Alistair

Quoted 124 lines

target/riscv/csr.c +44 -7

+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->senvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1565,15 +1603,27 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+    RISCVException ret;
 
-    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1582,6 +1632,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
     if (riscv_cpu_mxl(env) == MXL_RV64) {
         mask |= HENVCFG_PBMTE | HENVCFG_STCE;

@@ -1595,6 +1651,13 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg >> 32;
     return RISCV_EXCP_NONE;
 }

@@ -1604,9 +1667,14 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
 {
     uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
     uint64_t valh = (uint64_t)val << 32;
+    RISCVException ret;
 
-    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
     return RISCV_EXCP_NONE;
 }

@@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;

@@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
     reg = &env->hstateen[index];

@@ -1694,8 +1765,9 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
     int index = csrno - CSR_HSTATEEN0H;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
--

2.25.1

Alistair Francis

Reviewed-by on v5 2/4

2022-06-16 07:01 UTC #12 lore raw

Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg

In reply to Mayuresh Chitale

Quoted 7 lines · Mayuresh Chitale

Accesses to henvcfg, henvcfgh and senvcfg are allowed only if corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an illegal instruction trap is generated.
Signed-off-by Mayuresh Chitale <[email protected]>

Reviewed-by Alistair Francis <[email protected]>

Alistair

Quoted 177 lines

---
 target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 6 deletions(-)

target/riscv/csr.c +78 -7

index 324fefce59..ae91ae1f7e 100644
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+#if !defined(CONFIG_USER_ONLY)
+    if (!(env->mstateen[0] & 1UL << bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    if (virt) {
+        if (!(env->hstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (mode == PRV_U) {
+        if (!(env->sstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)

@@ -1557,6 +1588,13 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->senvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1565,15 +1603,27 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+    RISCVException ret;
 
-    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg;
     return RISCV_EXCP_NONE;
 }

@@ -1582,6 +1632,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
     uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
     if (riscv_cpu_mxl(env) == MXL_RV64) {
         mask |= HENVCFG_PBMTE | HENVCFG_STCE;

@@ -1595,6 +1651,13 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg >> 32;
     return RISCV_EXCP_NONE;
 }

@@ -1604,9 +1667,14 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
 {
     uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
     uint64_t valh = (uint64_t)val << 32;
+    RISCVException ret;
 
-    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
     return RISCV_EXCP_NONE;
 }

@@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;

@@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
     reg = &env->hstateen[index];

@@ -1694,8 +1765,9 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
     int index = csrno - CSR_HSTATEEN0H;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
--

2.25.1

angell1518

2022-07-02 10:34 UTC #13 lore raw

Re:[RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg

In reply to Mayuresh Chitale

Quoted 23 lines · Mayuresh Chitale

Accesses to henvcfg, henvcfgh and senvcfg are allowed only if corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an illegal instruction trap is generated.
Signed-off-by Mayuresh Chitale <[email protected]>
---
target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 78 insertions(+), 6 deletions(-)
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +5 -0
index 324fefce59..ae91ae1f7e 100644
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    bool virt = riscv_cpu_virt_enabled(env);

target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) · line 56

Quoted 10 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +10 -0

+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+#if !defined(CONFIG_USER_ONLY)
+    if (!(env->mstateen[0] & 1UL << bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+

I think mstateen only control access in priv mode under M mode.

So we should take the current priv node into consideration here.

target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) · line 67

Quoted 11 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +10 -0

+    if (virt) {
+        if (!(env->hstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (mode == PRV_U) {
+        if (!(env->sstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
 
+ }

I think there are two problem here:

The first is that we should also take virt mode into consideration here too. As the spec said:

"if executing in VS or VU mode and the circumstances for a virtual instruction exception apply, raises a virtual instruction exception instead of an illegal instruction exception"

So it will generate RISCV_EXCP_VIRT_INSTRUCTION_FAULT in VU mode.

The second is that sstateen csr works only when 'S' extension is enabled

Regards,
Weiwei Li

Quoted 139 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +6 -0

+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {

#if !defined(CONFIG_USER_ONLY)

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +47 -6

@@ -1557,6 +1588,13 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
                                 target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
    *val = env->senvcfg;
    return RISCV_EXCP_NONE;
 }

@@ -1565,15 +1603,27 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
                                  target_ulong val)
 {
    uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+    RISCVException ret;
 
-    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
    return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
                                 target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
    *val = env->henvcfg;
    return RISCV_EXCP_NONE;
 }

@@ -1582,6 +1632,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
                                  target_ulong val)
 {
    uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
    if (riscv_cpu_mxl(env) == MXL_RV64) {
        mask |= HENVCFG_PBMTE | HENVCFG_STCE;

@@ -1595,6 +1651,13 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
                                 target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
    *val = env->henvcfg >> 32;
    return RISCV_EXCP_NONE;
 }

@@ -1604,9 +1667,14 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
 {
    uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
    uint64_t valh = (uint64_t)val << 32;
+    RISCVException ret;
 
-    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
    return RISCV_EXCP_NONE;
 }

@@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
                                     target_ulong new_val)
 {
    uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
    write_smstateen(env, reg, wr_mask, new_val);

@@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
 {
    uint64_t *reg;
    uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
    val = (uint64_t)new_val << 32;

@@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
                                     target_ulong new_val)
 {
    uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
    int index = csrno - CSR_HSTATEEN0;
 
    reg = &env->hstateen[index];

@@ -1694,8 +1765,9 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
 {
    uint64_t *reg;
    uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
    int index = csrno - CSR_HSTATEEN0H;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
    reg = &env->hstateen[index];
    val = (uint64_t)new_val << 32;

Signature · 1 line

2.25.1

Mayuresh Chitale

author

2022-07-07 17:20 UTC #14 lore raw

Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg

Quoted 24 lines · On Sat, 2022-07-02 at 18:33 +0800, angell1518 wrote:

At 2022-06-04 00:04:23, "Mayuresh Chitale" <[email protected]
Quoted 9 lines
wrote:
Accesses to henvcfg, henvcfgh and senvcfg are allowed only if corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an illegal instruction trap is generated.
Signed-off-by Mayuresh Chitale <[email protected]>
---
target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++-
---
target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno,
Quoted 7 lines
1 file changed, 78 insertions(+), 6 deletions(-)
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +0 -0
index 324fefce59..ae91ae1f7e 100644
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno,
riscv_csr_operations *ops)
target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno, · line 42
Quoted 4 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +1 -0
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int
mode, int bit)

Quoted 16 lines

Quoted 4 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +4 -0
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    bool virt = riscv_cpu_virt_enabled(env);
target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno, · line 56
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +10 -0
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+#if !defined(CONFIG_USER_ONLY)
+    if (!(env->mstateen[0] & 1UL << bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
I think mstateen only control access in priv mode under M mode. So we should take the current priv node into consideration here.

For any curent priv mode if the required bit is not set in mstateen we can return here itself. For e.g if current priv mode is S then we only check the required bit in mstateen. If current priv mode is 'U', we need to check the required bit in mstateen and sstateen

Quoted 10 lines

target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno, · line 66
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +10 -0
+    if (virt) {
+        if (!(env->hstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (mode == PRV_U) {
+        if (!(env->sstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+ }

Quoted 3 lines

I think there are two problem here:
The first is that we should also take virt mode into consideration here too.

Actually virt mode is considered above for both priv modes S and U.

Quoted 11 lines

As the spec said:
"if executing in VS or VU mode and the circumstances for a virtual instruction
exception apply, raises a virtual instruction exception instead of an illegal instruction exception"
So it will generate RISCV_EXCP_VIRT_INSTRUCTION_FAULT in VU mode.
The second is that sstateen csr works only when 'S' extension is enabled

I will fix it in the next version.

Quoted 152 lines

Regards,
Weiwei Li

target/riscv/csr.ccounteren static RISCVException

Quoted 10 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +6 -0

+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)

@@ -1557,6 +1588,13 @@ static RISCVException

write_menvcfgh(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException

Quoted 14 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +7 -0

 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
                                 target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
    *val = env->senvcfg;
    return RISCV_EXCP_NONE;
 }

@@ -1565,15 +1603,27 @@ static RISCVException

write_senvcfg(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException · line 1605

Quoted 3 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +0 -0

                                  target_ulong val)
 {
    uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE |

SENVCFG_CBZE;

target/riscv/csr.ccounteren static RISCVException

Quoted 26 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +13 -1

+    RISCVException ret;
 
-    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
    return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
                                 target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
    *val = env->henvcfg;
    return RISCV_EXCP_NONE;
 }

@@ -1582,6 +1632,12 @@ static RISCVException

write_henvcfg(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException · line 1634

Quoted 3 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +0 -0

                                  target_ulong val)
 {
    uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE |

HENVCFG_CBZE;

target/riscv/csr.ccounteren static RISCVException

Quoted 10 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +6 -0

+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
    if (riscv_cpu_mxl(env) == MXL_RV64) {
        mask |= HENVCFG_PBMTE | HENVCFG_STCE;

@@ -1595,6 +1651,13 @@ static RISCVException

write_henvcfg(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException

Quoted 14 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +7 -0

 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
                                 target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
    *val = env->henvcfg >> 32;
    return RISCV_EXCP_NONE;
 }

@@ -1604,9 +1667,14 @@ static RISCVException

write_henvcfgh(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException

Quoted 16 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +6 -1

 {
    uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
    uint64_t valh = (uint64_t)val << 32;
+    RISCVException ret;
 
-    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
    return RISCV_EXCP_NONE;
 }

@@ -1628,7 +1696,8 @@ static RISCVException

write_mstateen(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException

Quoted 10 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1

                                     target_ulong new_val)
 {
    uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
    reg = &env->mstateen[csrno - CSR_MSTATEEN0];
    write_smstateen(env, reg, wr_mask, new_val);

@@ -1649,7 +1718,8 @@ static RISCVException

write_mstateenh(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException

Quoted 10 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1

 {
    uint64_t *reg;
    uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
    reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
    val = (uint64_t)new_val << 32;

@@ -1671,7 +1741,8 @@ static RISCVException

write_hstateen(CPURISCVState *env, int csrno,

target/riscv/csr.ccounteren static RISCVException

Quoted 10 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1

                                     target_ulong new_val)
 {
    uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
    int index = csrno - CSR_HSTATEEN0;
 
    reg = &env->hstateen[index];

@@ -1694,8 +1765,9 @@ static RISCVException

write_hstateenh(CPURISCVState *env, int csrno,

Quoted 13 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1

 {
    uint64_t *reg;
    uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
    int index = csrno - CSR_HSTATEEN0H;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
    reg = &env->hstateen[index];
    val = (uint64_t)new_val << 32;

Signature · 1 line

2.25.1

Weiwei Li

2022-07-07 23:36 UTC #15 lore raw

Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg

Quoted 41 lines · Mayuresh Chitale

Quoted 24 lines · On Sat, 2022-07-02 at 18:33 +0800, angell1518 wrote:
At 2022-06-04 00:04:23, "Mayuresh Chitale" <[email protected]
Quoted 9 lines
wrote:
Accesses to henvcfg, henvcfgh and senvcfg are allowed only if corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an illegal instruction trap is generated.
Signed-off-by Mayuresh Chitale <[email protected]>
---
target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++-
---
target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno,
Quoted 7 lines
1 file changed, 78 insertions(+), 6 deletions(-)
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +0 -0
index 324fefce59..ae91ae1f7e 100644
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno,
riscv_csr_operations *ops)
target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno, · line 42
Quoted 4 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +1 -0
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int
mode, int bit)
Quoted 16 lines
Quoted 4 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +4 -0
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    bool virt = riscv_cpu_virt_enabled(env);
target/riscv/csr.ccounteren void riscv_set_csr_ops(int csrno, · line 56
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +10 -0
+
+    if (!cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+#if !defined(CONFIG_USER_ONLY)
+    if (!(env->mstateen[0] & 1UL << bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
I think mstateen only control access in priv mode under M mode. So we should take the current priv node into consideration here.

Quoted 5 lines

For any curent priv mode if the required bit is not set in mstateen we can return here itself. For e.g if current priv mode is S then we only check the required bit in mstateen. If current priv mode is 'U', we need to check the required bit in mstateen and sstateen

mstateen only controls the access from less-privilege mode. So do hstateen and sstateen. we should pass all of the check, if current priv is PRV_M. like this:
+ if (env->priv == PRV_M) {
+ return RISCV_EXCP_NONE;
+ }

Quoted 10 lines

target/riscv/csr.ccounteren renamed from target/riscv/csr.c +10 -0

+    if (virt) {
+        if (!(env->hstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (mode == PRV_U) {
+        if (!(env->sstateen[0] & 1UL << bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+ }

Quoted 4 lines

Quoted 3 lines
I think there are two problem here:
The first is that we should also take virt mode into consideration here too.
Actually virt mode is considered above for both priv modes S and U.

Yeah(we also should pass this check for current priv is PRV_M). However, there's still a problem

here: the mode here is the mode for csr not the current priv mode. Actually, Sstateen will control the access

for user mode csr(such as fcsr) from U mode. So taking the following question into consideration, the total check

may be:

+ if (mode == PRV_U &&env->priv == PRV_U) {
+ if (riscv_has_ext(env, RVS) && !(env->sstateen[0] & 1UL << bit)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }

Regards,
Weiwei Li

Quoted 163 lines

Quoted 9 lines
As the spec said:
"if executing in VS or VU mode and the circumstances for a virtual instruction
exception apply, raises a virtual instruction exception instead of an illegal instruction exception"
So it will generate RISCV_EXCP_VIRT_INSTRUCTION_FAULT in VU mode. The second is that sstateen csr works only when 'S' extension is enabled
I will fix it in the next version.
Quoted 152 lines
Regards,
Weiwei Li
target/riscv/csr.ccounteren static RISCVException
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +6 -0
+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -1557,6 +1588,13 @@ static RISCVException
write_menvcfgh(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException
Quoted 14 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +7 -0
 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->senvcfg;
     return RISCV_EXCP_NONE;
 }
@@ -1565,15 +1603,27 @@ static RISCVException
write_senvcfg(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException · line 1605
Quoted 3 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +0 -0
                                   target_ulong val)
 {
     uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE |
SENVCFG_CBZE;
target/riscv/csr.ccounteren static RISCVException
Quoted 26 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +13 -1
+    RISCVException ret;
 
-    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg;
     return RISCV_EXCP_NONE;
 }
@@ -1582,6 +1632,12 @@ static RISCVException
write_henvcfg(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException · line 1634
Quoted 3 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +0 -0
                                   target_ulong val)
 {
     uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE |
HENVCFG_CBZE;
target/riscv/csr.ccounteren static RISCVException
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +6 -0
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
     if (riscv_cpu_mxl(env) == MXL_RV64) {
         mask |= HENVCFG_PBMTE | HENVCFG_STCE;
@@ -1595,6 +1651,13 @@ static RISCVException
write_henvcfg(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException
Quoted 14 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +7 -0
 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->henvcfg >> 32;
     return RISCV_EXCP_NONE;
 }
@@ -1604,9 +1667,14 @@ static RISCVException
write_henvcfgh(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException
Quoted 16 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +6 -1
 {
     uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
     uint64_t valh = (uint64_t)val << 32;
+    RISCVException ret;
 
-    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
+    env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
     return RISCV_EXCP_NONE;
 }
 
@@ -1628,7 +1696,8 @@ static RISCVException
write_mstateen(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
     write_smstateen(env, reg, wr_mask, new_val);
@@ -1649,7 +1718,8 @@ static RISCVException
write_mstateenh(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;
@@ -1671,7 +1741,8 @@ static RISCVException
write_hstateen(CPURISCVState *env, int csrno,
target/riscv/csr.ccounteren static RISCVException
Quoted 10 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1
                                      target_ulong new_val)
 {
     uint64_t *reg;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
     reg = &env->hstateen[index];
@@ -1694,8 +1765,9 @@ static RISCVException
write_hstateenh(CPURISCVState *env, int csrno,
Quoted 13 lines
target/riscv/csr.ccounteren renamed from target/riscv/csr.c +2 -1
 {
     uint64_t *reg;
     uint64_t val;
-    uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
     int index = csrno - CSR_HSTATEEN0H;
+    uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+                       (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
Signature · 1 line
2.25.1

Mayuresh Chitale

author

2022-06-03 16:05 UTC #16 lore raw

RFC PATCH v5 3/4 target/riscv: smstateen check for fcsr

In reply to Mayuresh Chitale

If smstateen is implemented and sstateen0.fcsr is clear then the floating point operations must return illegal instruction exception.

Signed-off-by Mayuresh Chitale <[email protected]>

---
 target/riscv/csr.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

target/riscv/csr.c +24 -0

index ae91ae1f7e..8bbbed38ff 100644
@@ -77,6 +77,10 @@ static RISCVException fs(CPURISCVState *env, int csrno)
         !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
+
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+        return smstateen_acc_ok(env, PRV_U, SMSTATEEN0_FCSR);
+    }
 #endif
     return RISCV_EXCP_NONE;
 }

@@ -1700,6 +1704,10 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
                        (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     write_smstateen(env, reg, wr_mask, new_val);
 
     return RISCV_EXCP_NONE;

@@ -1724,6 +1732,10 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;
     val |= *reg & 0xFFFFFFFF;
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     write_smstateen(env, reg, wr_mask, val);
 
     return RISCV_EXCP_NONE;

@@ -1745,6 +1757,10 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
                        (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->hstateen[index];
     wr_mask &= env->mstateen[index];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1769,6 +1785,10 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
     uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
                        (1UL << SMSTATEEN0_HSENVCFG);
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
     val |= *reg & 0xFFFFFFFF;

@@ -1794,6 +1814,10 @@ static RISCVException write_sstateen(CPURISCVState *env, int csrno,
     int index = csrno - CSR_SSTATEEN0;
     bool virt = riscv_cpu_virt_enabled(env);
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->sstateen[index];
     if (virt) {
         wr_mask &= env->mstateen[index];

Signature · 1 line

2.25.1

Alistair Francis

2022-06-16 07:17 UTC #17 lore raw

Re: [RFC PATCH v5 3/4] target/riscv: smstateen check for fcsr

Quoted 15 lines · Mayuresh Chitale

If smstateen is implemented and sstateen0.fcsr is clear then the floating point operations must return illegal instruction exception.
Signed-off-by Mayuresh Chitale <[email protected]>
---
 target/riscv/csr.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
target/riscv/csr.c +0 -0
index ae91ae1f7e..8bbbed38ff 100644

target/riscv/csr.c static RISCVException fs(CPURISCVState *env, int csrno) · line 83

Quoted 7 lines

target/riscv/csr.c +4 -0

@@ -77,6 +77,10 @@ static RISCVException fs(CPURISCVState *env, int csrno)
         !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
+
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+        return smstateen_acc_ok(env, PRV_U, SMSTATEEN0_FCSR);
+    }

This only checks access to the CSRs. Shouldn't we also be throwing errors if any instruction operates on an x register?

target/riscv/csr.c static RISCVException write_mstateen(CPURISCVState *env, int csrno, · line 1709

Quoted 10 lines

target/riscv/csr.c +3 -0

 #endif
     return RISCV_EXCP_NONE;
 }

@@ -1700,6 +1704,10 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
                        (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }

This doesn't look right.

"Whenever misa.F = 1, bit 1 of mstateen0 is read-only zero". Shouldn't that mean we don't allow writes if we have the RVF extension?

Alistair

Quoted 52 lines

target/riscv/csr.c +17 -1

+
     write_smstateen(env, reg, wr_mask, new_val);
 
     return RISCV_EXCP_NONE;

@@ -1724,6 +1732,10 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;
     val |= *reg & 0xFFFFFFFF;
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     write_smstateen(env, reg, wr_mask, val);
 
     return RISCV_EXCP_NONE;

@@ -1745,6 +1757,10 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
                        (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->hstateen[index];
     wr_mask &= env->mstateen[index];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1769,6 +1785,10 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
     uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
                        (1UL << SMSTATEEN0_HSENVCFG);
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
     val |= *reg & 0xFFFFFFFF;

@@ -1794,6 +1814,10 @@ static RISCVException write_sstateen(CPURISCVState *env, int csrno,
     int index = csrno - CSR_SSTATEEN0;
     bool virt = riscv_cpu_virt_enabled(env);
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->sstateen[index];
     if (virt) {
         wr_mask &= env->mstateen[index];
--

2.25.1

Mayuresh Chitale

author

2022-07-07 16:12 UTC #18 lore raw

Re: [RFC PATCH v5 3/4] target/riscv: smstateen check for fcsr

Quoted 11 lines · Alistair Francis

Quoted 9 lines · Mayuresh Chitale
If smstateen is implemented and sstateen0.fcsr is clear then the floating point operations must return illegal instruction exception.
Signed-off-by Mayuresh Chitale <[email protected]>
---
 target/riscv/csr.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Quoted 16 lines

target/riscv/csr.c static RISCVException fs(CPURISCVState *env, int · line 84
Quoted 13 lines
target/riscv/csr.c +4 -0
index ae91ae1f7e..8bbbed38ff 100644
@@ -77,6 +77,10 @@ static RISCVException fs(CPURISCVState *env, int
 csrno)
         !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
+
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+        return smstateen_acc_ok(env, PRV_U, SMSTATEEN0_FCSR);
+    }
This only checks access to the CSRs. Shouldn't we also be throwing errors if any instruction operates on an x register?

Yes.

Quoted 15 lines

target/riscv/csr.c static RISCVException · line 1710

Quoted 12 lines

target/riscv/csr.c +0 -0

 #endif
     return RISCV_EXCP_NONE;

}

target/riscv/csr.c +3 -0

@@ -1700,6 +1704,10 @@ static RISCVException
 write_mstateen(CPURISCVState *env, int csrno,
                        (1UL << SMSTATEEN0_HSENVCFG);
 
     reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }

This doesn't look right.

Quoted 3 lines

"Whenever misa.F = 1, bit 1 of mstateen0 is read-only zero". Shouldn't
that mean we don't allow writes if we have the RVF extension?

I will fix it in the next version.

Quoted 59 lines

Alistair

Quoted 56 lines

target/riscv/csr.c +1 -0

+
     write_smstateen(env, reg, wr_mask, new_val);

     return RISCV_EXCP_NONE;

target/riscv/csr.c +4 -0

@@ -1724,6 +1732,10 @@ static RISCVException
 write_mstateenh(CPURISCVState *env, int csrno,
     reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
     val = (uint64_t)new_val << 32;
     val |= *reg & 0xFFFFFFFF;
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     write_smstateen(env, reg, wr_mask, val);

     return RISCV_EXCP_NONE;

target/riscv/csr.c +4 -0

@@ -1745,6 +1757,10 @@ static RISCVException
 write_hstateen(CPURISCVState *env, int csrno,
                        (1UL << SMSTATEEN0_HSENVCFG);
     int index = csrno - CSR_HSTATEEN0;
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->hstateen[index];
     wr_mask &= env->mstateen[index];

     write_smstateen(env, reg, wr_mask, new_val);

target/riscv/csr.c +4 -0

@@ -1769,6 +1785,10 @@ static RISCVException
 write_hstateenh(CPURISCVState *env, int csrno,
     uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
                        (1UL << SMSTATEEN0_HSENVCFG);
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;

     val |= *reg & 0xFFFFFFFF;

target/riscv/csr.c +4 -0

@@ -1794,6 +1814,10 @@ static RISCVException
 write_sstateen(CPURISCVState *env, int csrno,
     int index = csrno - CSR_SSTATEEN0;
     bool virt = riscv_cpu_virt_enabled(env);
 
+    if (riscv_has_ext(env, RVF)) {
+        wr_mask |= 1UL << SMSTATEEN0_FCSR;
+    }
+
     reg = &env->sstateen[index];
     if (virt) {

         wr_mask &= env->mstateen[index];

--
2.25.1

Mayuresh Chitale

author

2022-06-03 16:04 UTC #19 lore raw

RFC PATCH v5 4/4 target/riscv: smstateen check for AIA/IMSIC

In reply to Mayuresh Chitale

If smstateen is implemented then accesses to AIA registers CSRS, IMSIC CSRs and other IMSIC registers is controlled by setting of corresponding bits in mstateen/hstateen registers. Otherwise an illegal instruction trap or virtual instruction trap is generated.

Signed-off-by Mayuresh Chitale <[email protected]>

---
 target/riscv/csr.c | 253 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 248 insertions(+), 5 deletions(-)

target/riscv/csr.c +248 -5

index 8bbbed38ff..213b3c17ff 100644
@@ -39,6 +39,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+#if !defined(CONFIG_USER_ONLY)
 static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
 {
     CPUState *cs = env_cpu(env);

@@ -49,7 +50,6 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
         return RISCV_EXCP_NONE;
     }
 
-#if !defined(CONFIG_USER_ONLY)
     if (!(env->mstateen[0] & 1UL << bit)) {
         return RISCV_EXCP_ILLEGAL_INST;
     }

@@ -65,11 +65,57 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
             return RISCV_EXCP_ILLEGAL_INST;
         }
     }
-#endif
-
     return RISCV_EXCP_NONE;
 }
 
+static RISCVException smstateen_aia_acc_ok(CPURISCVState *env, int csrno)
+{
+    int bit, mode;
+
+    switch (csrno) {
+    case CSR_SSETEIPNUM:
+    case CSR_SCLREIPNUM:
+    case CSR_SSETEIENUM:
+    case CSR_SCLREIENUM:
+    case CSR_STOPEI:
+    case CSR_VSSETEIPNUM:
+    case CSR_VSCLREIPNUM:
+    case CSR_VSSETEIENUM:
+    case CSR_VSCLREIENUM:
+    case CSR_VSTOPEI:
+    case CSR_HSTATUS:
+        mode = PRV_S;
+        bit = SMSTATEEN0_IMSIC;
+        break;
+
+    case CSR_SIEH:
+    case CSR_SIPH:
+    case CSR_HVIPH:
+    case CSR_HVICTL:
+    case CSR_HVIPRIO1:
+    case CSR_HVIPRIO2:
+    case CSR_HVIPRIO1H:
+    case CSR_HVIPRIO2H:
+    case CSR_VSIEH:
+    case CSR_VSIPH:
+        mode = PRV_S;
+        bit = SMSTATEEN0_AIA;
+        break;
+
+    case CSR_SISELECT:
+    case CSR_VSISELECT:
+        mode = PRV_S;
+        bit = SMSTATEEN0_SVSLCT;
+        break;
+
+    default:
+        return RISCV_EXCP_NONE;
+    }
+
+    return smstateen_acc_ok(env, mode, bit);
+}
+#endif
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)

@@ -1130,6 +1176,13 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
                         target_ulong new_val, target_ulong wr_mask)
 {
     target_ulong *iselect;
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1212,7 +1265,9 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
     bool virt;
     uint8_t *iprio;
     int ret = -EINVAL;
-    target_ulong priv, isel, vgein;
+    target_ulong priv, isel, vgein = 0;
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1241,11 +1296,20 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
     };
 
     /* Find the selected guest interrupt file */
-    vgein = (virt) ? get_field(env->hstatus, HSTATUS_VGEIN) : 0;
+    if (virt) {
+        if (!cpu->cfg.ext_smstateen ||
+                (env->hstateen[0] & 1UL << SMSTATEEN0_IMSIC)) {
+            vgein = get_field(env->hstatus, HSTATUS_VGEIN);
+        }
+    }
 
     if (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) {
         /* Local interrupt priority registers not available for VS-mode */
         if (!virt) {
+            if (priv == PRV_S && cpu->cfg.ext_smstateen &&
+                !(env->hstateen[0] & 1UL << SMSTATEEN0_AIA)) {
+                goto done;
+            }
             ret = rmw_iprio(riscv_cpu_mxl_bits(env),
                             isel, iprio, val, new_val, wr_mask,
                             (priv == PRV_M) ? IRQ_M_EXT : IRQ_S_EXT);

@@ -1279,6 +1343,13 @@ static int rmw_xsetclreinum(CPURISCVState *env, int csrno, target_ulong *val,
     int ret = -EINVAL;
     bool set, pend, virt;
     target_ulong priv, isel, vgein, xlen, nval, wmask;
+    RISCVException excp;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    excp = smstateen_aia_acc_ok(env, csrno);
+    if (excp != RISCV_EXCP_NONE) {
+        return excp;
+    }
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1397,6 +1468,13 @@ static int rmw_xtopei(CPURISCVState *env, int csrno, target_ulong *val,
     bool virt;
     int ret = -EINVAL;
     target_ulong priv, vgein;
+    RISCVException excp;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    excp = smstateen_aia_acc_ok(env, csrno);
+    if (excp != RISCV_EXCP_NONE) {
+        return excp;
+    }
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1708,6 +1786,12 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     write_smstateen(env, reg, wr_mask, new_val);
 
     return RISCV_EXCP_NONE;

@@ -1736,6 +1820,12 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     write_smstateen(env, reg, wr_mask, val);
 
     return RISCV_EXCP_NONE;

@@ -1761,6 +1851,12 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     reg = &env->hstateen[index];
     wr_mask &= env->mstateen[index];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1789,6 +1885,12 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
     val |= *reg & 0xFFFFFFFF;

@@ -1979,6 +2081,12 @@ static RISCVException rmw_vsieh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_vsie64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2033,6 +2141,12 @@ static RISCVException rmw_sieh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_sie64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2195,6 +2309,12 @@ static RISCVException rmw_vsiph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_vsip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2249,6 +2369,12 @@ static RISCVException rmw_siph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_sip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2441,6 +2567,10 @@ static RISCVException read_hstatus(CPURISCVState *env, int csrno,
 static RISCVException write_hstatus(CPURISCVState *env, int csrno,
                                     target_ulong val)
 {
+    if (smstateen_aia_acc_ok(env, csrno) != RISCV_EXCP_NONE) {
+        val &= ~HSTATUS_VGEIN;
+    }
+
     env->hstatus = val;
     if (riscv_cpu_mxl(env) != MXL_RV32 && get_field(val, HSTATUS_VSXL) != 2) {
         qemu_log_mask(LOG_UNIMP, "QEMU does not support mixed HSXLEN options.");

@@ -2501,6 +2631,12 @@ static RISCVException rmw_hidelegh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_hideleg64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2547,6 +2683,12 @@ static RISCVException rmw_hviph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_hvip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2601,6 +2743,13 @@ static RISCVException write_hcounteren(CPURISCVState *env, int csrno,
 static RISCVException read_hgeie(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     if (val) {
         *val = env->hgeie;
     }

@@ -2610,6 +2759,13 @@ static RISCVException read_hgeie(CPURISCVState *env, int csrno,
 static RISCVException write_hgeie(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     /* Only GEILEN:1 bits implemented and BIT0 is never implemented */
     val &= ((((target_ulong)1) << env->geilen) - 1) << 1;
     env->hgeie = val;

@@ -2649,6 +2805,13 @@ static RISCVException write_htinst(CPURISCVState *env, int csrno,
 static RISCVException read_hgeip(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     if (val) {
         *val = env->hgeip;
     }

@@ -2719,12 +2882,28 @@ static RISCVException write_htimedeltah(CPURISCVState *env, int csrno,
 
 static int read_hvictl(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->hvictl;
     return RISCV_EXCP_NONE;
 }
 
 static int write_hvictl(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret = RISCV_EXCP_NONE;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     env->hvictl = val & HVICTL_VALID_MASK;
     return RISCV_EXCP_NONE;
 }

@@ -2783,41 +2962,105 @@ static int write_hvipriox(CPURISCVState *env, int first_index,
 
 static int read_hviprio1(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 0, env->hviprio, val);
 }
 
 static int write_hviprio1(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 0, env->hviprio, val);
 }
 
 static int read_hviprio1h(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 4, env->hviprio, val);
 }
 
 static int write_hviprio1h(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 4, env->hviprio, val);
 }
 
 static int read_hviprio2(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 8, env->hviprio, val);
 }
 
 static int write_hviprio2(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 8, env->hviprio, val);
 }
 
 static int read_hviprio2h(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 12, env->hviprio, val);
 }
 
 static int write_hviprio2h(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 12, env->hviprio, val);
 }

Signature · 1 line

2.25.1

Alistair Francis

2022-06-16 07:19 UTC #20 lore raw

Re: [RFC PATCH v5 4/4] target/riscv: smstateen check for AIA/IMSIC

Quoted 18 lines · Mayuresh Chitale

If smstateen is implemented then accesses to AIA registers CSRS, IMSIC CSRs and other IMSIC registers is controlled by setting of corresponding bits in mstateen/hstateen registers. Otherwise an illegal instruction trap or virtual instruction trap is generated.
Signed-off-by Mayuresh Chitale <[email protected]>
---
 target/riscv/csr.c | 253 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 248 insertions(+), 5 deletions(-)
target/riscv/csr.c +0 -0
index 8bbbed38ff..213b3c17ff 100644

target/riscv/csr.c void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) · line 42

Quoted 4 lines

target/riscv/csr.c +1 -0

@@ -39,6 +39,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+#if !defined(CONFIG_USER_ONLY)

This should just be in the original patch.

Quoted 12 lines

target/riscv/csr.c +0 -1

 static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
 {
     CPUState *cs = env_cpu(env);

@@ -49,7 +50,6 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
         return RISCV_EXCP_NONE;
     }
 
-#if !defined(CONFIG_USER_ONLY)
     if (!(env->mstateen[0] & 1UL << bit)) {
         return RISCV_EXCP_ILLEGAL_INST;
     }

target/riscv/csr.c static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit) · line 71

Quoted 9 lines

target/riscv/csr.c +1 -2

@@ -65,11 +65,57 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
             return RISCV_EXCP_ILLEGAL_INST;
         }
     }
-#endif
-
     return RISCV_EXCP_NONE;
 }
 
+static RISCVException smstateen_aia_acc_ok(CPURISCVState *env, int csrno)

The spec doesn't mention the effects on AIA, it just says that some bits are reserved. How do you know what should happen here?

Alistair

Quoted 447 lines

target/riscv/csr.c +246 -3

+{
+    int bit, mode;
+
+    switch (csrno) {
+    case CSR_SSETEIPNUM:
+    case CSR_SCLREIPNUM:
+    case CSR_SSETEIENUM:
+    case CSR_SCLREIENUM:
+    case CSR_STOPEI:
+    case CSR_VSSETEIPNUM:
+    case CSR_VSCLREIPNUM:
+    case CSR_VSSETEIENUM:
+    case CSR_VSCLREIENUM:
+    case CSR_VSTOPEI:
+    case CSR_HSTATUS:
+        mode = PRV_S;
+        bit = SMSTATEEN0_IMSIC;
+        break;
+
+    case CSR_SIEH:
+    case CSR_SIPH:
+    case CSR_HVIPH:
+    case CSR_HVICTL:
+    case CSR_HVIPRIO1:
+    case CSR_HVIPRIO2:
+    case CSR_HVIPRIO1H:
+    case CSR_HVIPRIO2H:
+    case CSR_VSIEH:
+    case CSR_VSIPH:
+        mode = PRV_S;
+        bit = SMSTATEEN0_AIA;
+        break;
+
+    case CSR_SISELECT:
+    case CSR_VSISELECT:
+        mode = PRV_S;
+        bit = SMSTATEEN0_SVSLCT;
+        break;
+
+    default:
+        return RISCV_EXCP_NONE;
+    }
+
+    return smstateen_acc_ok(env, mode, bit);
+}
+#endif
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)

@@ -1130,6 +1176,13 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
                         target_ulong new_val, target_ulong wr_mask)
 {
     target_ulong *iselect;
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1212,7 +1265,9 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
     bool virt;
     uint8_t *iprio;
     int ret = -EINVAL;
-    target_ulong priv, isel, vgein;
+    target_ulong priv, isel, vgein = 0;
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1241,11 +1296,20 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
     };
 
     /* Find the selected guest interrupt file */
-    vgein = (virt) ? get_field(env->hstatus, HSTATUS_VGEIN) : 0;
+    if (virt) {
+        if (!cpu->cfg.ext_smstateen ||
+                (env->hstateen[0] & 1UL << SMSTATEEN0_IMSIC)) {
+            vgein = get_field(env->hstatus, HSTATUS_VGEIN);
+        }
+    }
 
     if (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) {
         /* Local interrupt priority registers not available for VS-mode */
         if (!virt) {
+            if (priv == PRV_S && cpu->cfg.ext_smstateen &&
+                !(env->hstateen[0] & 1UL << SMSTATEEN0_AIA)) {
+                goto done;
+            }
             ret = rmw_iprio(riscv_cpu_mxl_bits(env),
                             isel, iprio, val, new_val, wr_mask,
                             (priv == PRV_M) ? IRQ_M_EXT : IRQ_S_EXT);

@@ -1279,6 +1343,13 @@ static int rmw_xsetclreinum(CPURISCVState *env, int csrno, target_ulong *val,
     int ret = -EINVAL;
     bool set, pend, virt;
     target_ulong priv, isel, vgein, xlen, nval, wmask;
+    RISCVException excp;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    excp = smstateen_aia_acc_ok(env, csrno);
+    if (excp != RISCV_EXCP_NONE) {
+        return excp;
+    }
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1397,6 +1468,13 @@ static int rmw_xtopei(CPURISCVState *env, int csrno, target_ulong *val,
     bool virt;
     int ret = -EINVAL;
     target_ulong priv, vgein;
+    RISCVException excp;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    excp = smstateen_aia_acc_ok(env, csrno);
+    if (excp != RISCV_EXCP_NONE) {
+        return excp;
+    }
 
     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

@@ -1708,6 +1786,12 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     write_smstateen(env, reg, wr_mask, new_val);
 
     return RISCV_EXCP_NONE;

@@ -1736,6 +1820,12 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     write_smstateen(env, reg, wr_mask, val);
 
     return RISCV_EXCP_NONE;

@@ -1761,6 +1851,12 @@ static RISCVException write_hstateen(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     reg = &env->hstateen[index];
     wr_mask &= env->mstateen[index];
     write_smstateen(env, reg, wr_mask, new_val);

@@ -1789,6 +1885,12 @@ static RISCVException write_hstateenh(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;
     val |= *reg & 0xFFFFFFFF;

@@ -1979,6 +2081,12 @@ static RISCVException rmw_vsieh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_vsie64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2033,6 +2141,12 @@ static RISCVException rmw_sieh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_sie64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2195,6 +2309,12 @@ static RISCVException rmw_vsiph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_vsip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2249,6 +2369,12 @@ static RISCVException rmw_siph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_sip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2441,6 +2567,10 @@ static RISCVException read_hstatus(CPURISCVState *env, int csrno,
 static RISCVException write_hstatus(CPURISCVState *env, int csrno,
                                     target_ulong val)
 {
+    if (smstateen_aia_acc_ok(env, csrno) != RISCV_EXCP_NONE) {
+        val &= ~HSTATUS_VGEIN;
+    }
+
     env->hstatus = val;
     if (riscv_cpu_mxl(env) != MXL_RV32 && get_field(val, HSTATUS_VSXL) != 2) {
         qemu_log_mask(LOG_UNIMP, "QEMU does not support mixed HSXLEN options.");

@@ -2501,6 +2631,12 @@ static RISCVException rmw_hidelegh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_hideleg64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2547,6 +2683,12 @@ static RISCVException rmw_hviph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_hvip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);
     if (ret_val) {

@@ -2601,6 +2743,13 @@ static RISCVException write_hcounteren(CPURISCVState *env, int csrno,
 static RISCVException read_hgeie(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     if (val) {
         *val = env->hgeie;
     }

@@ -2610,6 +2759,13 @@ static RISCVException read_hgeie(CPURISCVState *env, int csrno,
 static RISCVException write_hgeie(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     /* Only GEILEN:1 bits implemented and BIT0 is never implemented */
     val &= ((((target_ulong)1) << env->geilen) - 1) << 1;
     env->hgeie = val;

@@ -2649,6 +2805,13 @@ static RISCVException write_htinst(CPURISCVState *env, int csrno,
 static RISCVException read_hgeip(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     if (val) {
         *val = env->hgeip;
     }

@@ -2719,12 +2882,28 @@ static RISCVException write_htimedeltah(CPURISCVState *env, int csrno,
 
 static int read_hvictl(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->hvictl;
     return RISCV_EXCP_NONE;
 }
 
 static int write_hvictl(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret = RISCV_EXCP_NONE;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     env->hvictl = val & HVICTL_VALID_MASK;
     return RISCV_EXCP_NONE;
 }

@@ -2783,41 +2962,105 @@ static int write_hvipriox(CPURISCVState *env, int first_index,
 
 static int read_hviprio1(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 0, env->hviprio, val);
 }
 
 static int write_hviprio1(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 0, env->hviprio, val);
 }
 
 static int read_hviprio1h(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 4, env->hviprio, val);
 }
 
 static int write_hviprio1h(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 4, env->hviprio, val);
 }
 
 static int read_hviprio2(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 8, env->hviprio, val);
 }
 
 static int write_hviprio2(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 8, env->hviprio, val);
 }
 
 static int read_hviprio2h(CPURISCVState *env, int csrno, target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 12, env->hviprio, val);
 }
 
 static int write_hviprio2h(CPURISCVState *env, int csrno, target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 12, env->hviprio, val);
 }
 
--

2.25.1

Mayuresh Chitale

author

2022-07-07 16:22 UTC #21 lore raw

Re: [RFC PATCH v5 4/4] target/riscv: smstateen check for AIA/IMSIC

Quoted 23 lines · Alistair Francis

Quoted 21 lines · Mayuresh Chitale
If smstateen is implemented then accesses to AIA registers CSRS, IMSIC CSRs and other IMSIC registers is controlled by setting of corresponding bits in mstateen/hstateen registers. Otherwise an illegal instruction trap or virtual instruction trap is generated.
Signed-off-by Mayuresh Chitale <[email protected]>
---
 target/riscv/csr.c | 253
++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 248 insertions(+), 5 deletions(-)
target/riscv/csr.c +0 -0
index 8bbbed38ff..213b3c17ff 100644
@@ -39,6 +39,7 @@ void riscv_set_csr_ops(int csrno,
 riscv_csr_operations *ops)
 }
 

Quoted 4 lines

target/riscv/csr.c void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) · line 42
Quoted 2 lines
target/riscv/csr.c +1 -0
 /* Predicates */
+#if !defined(CONFIG_USER_ONLY)
This should just be in the original patch.

I will modify the second patch.

Quoted 24 lines

target/riscv/csr.c +0 -0

 static RISCVException smstateen_acc_ok(CPURISCVState *env, int
 mode, int bit)

 {
     CPUState *cs = env_cpu(env);

target/riscv/csr.c +0 -1

@@ -49,7 +50,6 @@ static RISCVException
 smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
         return RISCV_EXCP_NONE;
     }
 
-#if !defined(CONFIG_USER_ONLY)
     if (!(env->mstateen[0] & 1UL << bit)) {
         return RISCV_EXCP_ILLEGAL_INST;

target/riscv/csr.c +0 -2

@@ -65,11 +65,57 @@ static RISCVException
 smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
             return RISCV_EXCP_ILLEGAL_INST;
         }
     }
-#endif
-
     return RISCV_EXCP_NONE;
 }

Quoted 5 lines

target/riscv/csr.c static RISCVException · line 73
Quoted 2 lines
target/riscv/csr.c +1 -0
+static RISCVException smstateen_aia_acc_ok(CPURISCVState *env, int
 csrno)
The spec doesn't mention the effects on AIA, it just says that some bits are reserved. How do you know what should happen here?

Actually these bits were defined in an earlier version of the spec but I can drop this patch for now and resend once those bits get defined again.

Quoted 487 lines

Alistair

Quoted 484 lines

target/riscv/csr.c +47 -0

+{
+    int bit, mode;
+
+    switch (csrno) {
+    case CSR_SSETEIPNUM:
+    case CSR_SCLREIPNUM:
+    case CSR_SSETEIENUM:
+    case CSR_SCLREIENUM:
+    case CSR_STOPEI:
+    case CSR_VSSETEIPNUM:
+    case CSR_VSCLREIPNUM:
+    case CSR_VSSETEIENUM:
+    case CSR_VSCLREIENUM:
+    case CSR_VSTOPEI:
+    case CSR_HSTATUS:
+        mode = PRV_S;
+        bit = SMSTATEEN0_IMSIC;
+        break;
+
+    case CSR_SIEH:
+    case CSR_SIPH:
+    case CSR_HVIPH:
+    case CSR_HVICTL:
+    case CSR_HVIPRIO1:
+    case CSR_HVIPRIO2:
+    case CSR_HVIPRIO1H:
+    case CSR_HVIPRIO2H:
+    case CSR_VSIEH:
+    case CSR_VSIPH:
+        mode = PRV_S;
+        bit = SMSTATEEN0_AIA;
+        break;
+
+    case CSR_SISELECT:
+    case CSR_VSISELECT:
+        mode = PRV_S;
+        bit = SMSTATEEN0_SVSLCT;
+        break;
+
+    default:
+        return RISCV_EXCP_NONE;
+    }
+
+    return smstateen_acc_ok(env, mode, bit);
+}
+#endif
+
 static RISCVException fs(CPURISCVState *env, int csrno)

{
#if !defined(CONFIG_USER_ONLY)

target/riscv/csr.c +7 -0

@@ -1130,6 +1176,13 @@ static int rmw_xiselect(CPURISCVState *env,
 int csrno, target_ulong *val,
                         target_ulong new_val, target_ulong
 wr_mask)
 {
     target_ulong *iselect;
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }

     /* Translate CSR number for VS-mode */
     csrno = aia_xlate_vs_csrno(env, csrno);

target/riscv/csr.c +3 -1

@@ -1212,7 +1265,9 @@ static int rmw_xireg(CPURISCVState *env, int
 csrno, target_ulong *val,
     bool virt;
     uint8_t *iprio;
     int ret = -EINVAL;
-    target_ulong priv, isel, vgein;
+    target_ulong priv, isel, vgein = 0;
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
 
     /* Translate CSR number for VS-mode */

     csrno = aia_xlate_vs_csrno(env, csrno);

target/riscv/csr.c +6 -1

@@ -1241,11 +1296,20 @@ static int rmw_xireg(CPURISCVState *env,
 int csrno, target_ulong *val,
     };
 
     /* Find the selected guest interrupt file */
-    vgein = (virt) ? get_field(env->hstatus, HSTATUS_VGEIN) : 0;
+    if (virt) {
+        if (!cpu->cfg.ext_smstateen ||
+                (env->hstateen[0] & 1UL << SMSTATEEN0_IMSIC)) {
+            vgein = get_field(env->hstatus, HSTATUS_VGEIN);
+        }
+    }
 
     if (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) {
         /* Local interrupt priority registers not available for

VS-mode */

         if (!virt) {

target/riscv/csr.c +4 -0

+            if (priv == PRV_S && cpu->cfg.ext_smstateen &&
+                !(env->hstateen[0] & 1UL << SMSTATEEN0_AIA)) {
+                goto done;
+            }
             ret = rmw_iprio(riscv_cpu_mxl_bits(env),
                             isel, iprio, val, new_val, wr_mask,
                             (priv == PRV_M) ? IRQ_M_EXT :

IRQ_S_EXT);

target/riscv/csr.c +7 -0

@@ -1279,6 +1343,13 @@ static int rmw_xsetclreinum(CPURISCVState
 *env, int csrno, target_ulong *val,
     int ret = -EINVAL;
     bool set, pend, virt;
     target_ulong priv, isel, vgein, xlen, nval, wmask;
+    RISCVException excp;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    excp = smstateen_aia_acc_ok(env, csrno);
+    if (excp != RISCV_EXCP_NONE) {
+        return excp;
+    }
 
     /* Translate CSR number for VS-mode */

     csrno = aia_xlate_vs_csrno(env, csrno);

target/riscv/csr.c +7 -0

@@ -1397,6 +1468,13 @@ static int rmw_xtopei(CPURISCVState *env,
 int csrno, target_ulong *val,
     bool virt;
     int ret = -EINVAL;
     target_ulong priv, vgein;
+    RISCVException excp;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    excp = smstateen_aia_acc_ok(env, csrno);
+    if (excp != RISCV_EXCP_NONE) {
+        return excp;
+    }
 
     /* Translate CSR number for VS-mode */

     csrno = aia_xlate_vs_csrno(env, csrno);

target/riscv/csr.c +6 -0

@@ -1708,6 +1786,12 @@ static RISCVException
 write_mstateen(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     write_smstateen(env, reg, wr_mask, new_val);

     return RISCV_EXCP_NONE;

target/riscv/csr.c +6 -0

@@ -1736,6 +1820,12 @@ static RISCVException
 write_mstateenh(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     write_smstateen(env, reg, wr_mask, val);

     return RISCV_EXCP_NONE;

target/riscv/csr.c +6 -0

@@ -1761,6 +1851,12 @@ static RISCVException
 write_hstateen(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     reg = &env->hstateen[index];
     wr_mask &= env->mstateen[index];

     write_smstateen(env, reg, wr_mask, new_val);

target/riscv/csr.c +6 -0

@@ -1789,6 +1885,12 @@ static RISCVException
 write_hstateenh(CPURISCVState *env, int csrno,
         wr_mask |= 1UL << SMSTATEEN0_FCSR;
     }
 
+    if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+        wr_mask |= (1UL << SMSTATEEN0_IMSIC)
+                | (1UL << SMSTATEEN0_AIA)
+                | (1UL << SMSTATEEN0_SVSLCT);
+    }
+
     reg = &env->hstateen[index];
     val = (uint64_t)new_val << 32;

     val |= *reg & 0xFFFFFFFF;

target/riscv/csr.c +6 -0

@@ -1979,6 +2081,12 @@ static RISCVException
 rmw_vsieh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_vsie64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);

     if (ret_val) {

target/riscv/csr.c +6 -0

@@ -2033,6 +2141,12 @@ static RISCVException rmw_sieh(CPURISCVState
 *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_sie64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);

     if (ret_val) {

target/riscv/csr.c +6 -0

@@ -2195,6 +2309,12 @@ static RISCVException
 rmw_vsiph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_vsip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);

     if (ret_val) {

target/riscv/csr.c +6 -0

@@ -2249,6 +2369,12 @@ static RISCVException rmw_siph(CPURISCVState
 *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_sip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);

     if (ret_val) {

target/riscv/csr.c +4 -0

@@ -2441,6 +2567,10 @@ static RISCVException
 read_hstatus(CPURISCVState *env, int csrno,
 static RISCVException write_hstatus(CPURISCVState *env, int csrno,
                                     target_ulong val)
 {
+    if (smstateen_aia_acc_ok(env, csrno) != RISCV_EXCP_NONE) {
+        val &= ~HSTATUS_VGEIN;
+    }
+
     env->hstatus = val;
     if (riscv_cpu_mxl(env) != MXL_RV32 && get_field(val,

HSTATUS_VSXL) != 2) {
qemu_log_mask(LOG_UNIMP, "QEMU does not support mixed
HSXLEN options.");

target/riscv/csr.c +6 -0

@@ -2501,6 +2631,12 @@ static RISCVException
 rmw_hidelegh(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_hideleg64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);

     if (ret_val) {

target/riscv/csr.c +6 -0

@@ -2547,6 +2683,12 @@ static RISCVException
 rmw_hviph(CPURISCVState *env, int csrno,
     uint64_t rval;
     RISCVException ret;
 
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     ret = rmw_hvip64(env, csrno, &rval,
         ((uint64_t)new_val) << 32, ((uint64_t)wr_mask) << 32);

     if (ret_val) {

target/riscv/csr.c +7 -0

@@ -2601,6 +2743,13 @@ static RISCVException
 write_hcounteren(CPURISCVState *env, int csrno,
 static RISCVException read_hgeie(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     if (val) {
         *val = env->hgeie;

target/riscv/csr.c +7 -0

@@ -2610,6 +2759,13 @@ static RISCVException
 read_hgeie(CPURISCVState *env, int csrno,
 static RISCVException write_hgeie(CPURISCVState *env, int csrno,
                                   target_ulong val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     /* Only GEILEN:1 bits implemented and BIT0 is never

implemented */

     val &= ((((target_ulong)1) << env->geilen) - 1) << 1;
     env->hgeie = val;

target/riscv/csr.c +7 -0

@@ -2649,6 +2805,13 @@ static RISCVException
 write_htinst(CPURISCVState *env, int csrno,
 static RISCVException read_hgeip(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
+    RISCVException ret;
+
+    ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_IMSIC);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     if (val) {
         *val = env->hgeip;

target/riscv/csr.c +16 -0

@@ -2719,12 +2882,28 @@ static RISCVException
 write_htimedeltah(CPURISCVState *env, int csrno,
 
 static int read_hvictl(CPURISCVState *env, int csrno, target_ulong
 *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     *val = env->hvictl;
     return RISCV_EXCP_NONE;
 }
 
 static int write_hvictl(CPURISCVState *env, int csrno,
 target_ulong val)
 {
+    RISCVException ret = RISCV_EXCP_NONE;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+

     env->hvictl = val & HVICTL_VALID_MASK;
     return RISCV_EXCP_NONE;

}

target/riscv/csr.c +56 -0

@@ -2783,41 +2962,105 @@ static int write_hvipriox(CPURISCVState
 *env, int first_index,
 
 static int read_hviprio1(CPURISCVState *env, int csrno,
 target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 0, env->hviprio, val);
 }
 
 static int write_hviprio1(CPURISCVState *env, int csrno,
 target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 0, env->hviprio, val);
 }
 
 static int read_hviprio1h(CPURISCVState *env, int csrno,
 target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 4, env->hviprio, val);
 }
 
 static int write_hviprio1h(CPURISCVState *env, int csrno,
 target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 4, env->hviprio, val);
 }
 
 static int read_hviprio2(CPURISCVState *env, int csrno,
 target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 8, env->hviprio, val);
 }
 
 static int write_hviprio2(CPURISCVState *env, int csrno,
 target_ulong val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 8, env->hviprio, val);
 }
 
 static int read_hviprio2h(CPURISCVState *env, int csrno,
 target_ulong *val)
 {
+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return read_hvipriox(env, 12, env->hviprio, val);
 }
 
 static int write_hviprio2h(CPURISCVState *env, int csrno,

target_ulong val)
{

target/riscv/csr.c +8 -1

+    RISCVException ret;
+
+    /* Check if smstateen is enabled and this access is allowed */
+    ret = smstateen_aia_acc_ok(env, csrno);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
     return write_hvipriox(env, 12, env->hviprio, val);
 }
 
--

2.25.1