RISC-V Smstateen support
v2 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.
This series depends on the following series from Atish Patra:
https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00031.html
https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00142.html
Changes in v2:
- Make h/s/envcfg bits in m/h/stateen registers as writeable by default.
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 | 554 +++++++++++++++++++++++++++++++++++++++- target/riscv/machine.c | 22 +- 5 files changed, 614 insertions(+), 4 deletions(-)
Signature · 1 line
2.17.1
RFC PATCH v2 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 | 22 ++++- 5 files changed, 273 insertions(+), 1 deletion(-)
target/riscv/cpu.c +2 -0
@@ -807,6 +807,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 ("sstc" ,RISCVCPU , cfg.ext_sstc, true), +DEFINE_PROP_BOOL ("smstateen" ,RISCVCPU , cfg.ext_smstateen, true),DEFINE_PROP_STRING ("priv_spec" ,RISCVCPU , cfg.priv_spec), DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
@@ -970,6 +971,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)ISA_EDATA_ENTRY (svinval, ext_svinval),ISA_EDATA_ENTRY (svnapot, ext_svnapot),ISA_EDATA_ENTRY (svpbmt, ext_svpbmt), + ISA_EDATA_ENTRY(smstateen, ext_smstateen), };for (i =0 ; i <ARRAY_SIZE (isa_edata_arr); i++) {
target/riscv/cpu.h +4 -0
@@ -338,6 +338,9 @@ struct CPURISCVState {/* 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
@@ -416,6 +419,7 @@ struct RISCVCPUConfig {bool ext_zve32f;bool ext_zve64f;bool ext_sscofpmf; +bool ext_smstateen;/* Vendor-specific custom extensions */ bool ext_XVentanaCondOps;
target/riscv/cpu_bits.h +36 -0
@@ -205,6 +205,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
@@ -258,6 +264,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
@@ -309,6 +325,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 -1
@@ -232,6 +232,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;
@@ -331,7 +351,6 @@ const VMStateDescription vmstate_riscv_cpu = {VMSTATE_UINT64 (env.mtohost,RISCVCPU ),VMSTATE_UINT64 (env.mtimecmp,RISCVCPU ),VMSTATE_UINT64 (env.stimecmp,RISCVCPU ), -VMSTATE_END_OF_LIST () }, .subsections = (const VMStateDescription * []) {
@@ -342,6 +361,7 @@ const VMStateDescription vmstate_riscv_cpu = { &vmstate_rv128, &vmstate_kvmtimer, &vmstate_envcfg, + &vmstate_smstateen, NULL } };
Signature · 1 line
2.17.1
Re: [RFC PATCH v2 1/4] target/riscv: Add smstateen support
Quoted 166 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 | 22 ++++- 5 files changed, 273 insertions(+), 1 deletion(-)target/riscv/cpu.c +2
-0@@ -807,6 +807,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 ("sstc" ,RISCVCPU , cfg.ext_sstc, true), +DEFINE_PROP_BOOL ("smstateen" ,RISCVCPU , cfg.ext_smstateen, true),DEFINE_PROP_STRING ("priv_spec" ,RISCVCPU , cfg.priv_spec), DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),@@ -970,6 +971,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)ISA_EDATA_ENTRY (svinval, ext_svinval),ISA_EDATA_ENTRY (svnapot, ext_svnapot),ISA_EDATA_ENTRY (svpbmt, ext_svpbmt), + ISA_EDATA_ENTRY(smstateen, ext_smstateen), };for (i =0 ; i <ARRAY_SIZE (isa_edata_arr); i++) {target/riscv/cpu.h +4
-0@@ -338,6 +338,9 @@ struct CPURISCVState {/* 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 @@ -416,6 +419,7 @@ struct RISCVCPUConfig {bool ext_zve32f;bool ext_zve64f;bool ext_sscofpmf; +bool ext_smstateen;/* Vendor-specific custom extensions */ bool ext_XVentanaCondOps;target/riscv/cpu_bits.h +36
-0@@ -205,6 +205,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@@ -258,6 +264,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@@ -309,6 +325,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 0x757target/riscv/csr.c +36
-0@@ -265,6 +265,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 static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1922
Quoted 5 lines
target/riscv/csr.c +2
-0@@ -1882,6 +1918,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)
It's better that 'uint64_t' is aligned with 'CPURISCVState' here. The same to following similar cases.
Quoted 21 lines
target/riscv/csr.c +21
-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); +
target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1953
Quoted 10 lines
target/riscv/csr.c +10
-0+return RISCV_EXCP_NONE ; +} + +static RISCVException read_mstateenh (CPURISCVState *env,int csrno, +target_ulong *val) +{ + *val = env->mstateen[csrno -CSR_MSTATEEN0H -0x10 ] >>32 ; + +return RISCV_EXCP_NONE ; +}
Why minus 0x10 here?
when read CSR_MSTATEEN0H, the csrno should be CSR_MSTATEEN0H, and the expected result is mstateen[0] >> 32.
So, minus 0x10 seems unnecessary.
The same to following similar cases.
Quoted 94 lines
target/riscv/csr.c +90
-0+ +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 -0x10 ]; + 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 -0x10 ] >>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 -0x10 ; + + 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)
target/riscv/csr.c riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { · line 3831
Quoted 9 lines
target/riscv/csr.c +6
-0@@ -3664,6 +3823,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 },
It' better to aligned with CSR_MSTATEEN0 here.
Regards,
Weiwei Li
Quoted 95 lines
target/riscv/csr.c +45
-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@@ -232,6 +232,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;@@ -331,7 +351,6 @@ const VMStateDescription vmstate_riscv_cpu = {VMSTATE_UINT64 (env.mtohost,RISCVCPU ),VMSTATE_UINT64 (env.mtimecmp,RISCVCPU ),VMSTATE_UINT64 (env.stimecmp,RISCVCPU ), -VMSTATE_END_OF_LIST () }, .subsections = (const VMStateDescription * []) {@@ -342,6 +361,7 @@ const VMStateDescription vmstate_riscv_cpu = { &vmstate_rv128, &vmstate_kvmtimer, &vmstate_envcfg, + &vmstate_smstateen, NULL } };
Re: [RFC PATCH v2 1/4] target/riscv: Add smstateen support
Quoted 172 lines · Weiwei Li
Quoted 169 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 | 22 ++++- 5 files changed, 273 insertions(+), 1 deletion(-)target/riscv/cpu.c +2
-0@@ -807,6 +807,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 ("sstc" ,RISCVCPU , cfg.ext_sstc, true), +DEFINE_PROP_BOOL ("smstateen" ,RISCVCPU , cfg.ext_smstateen, true),DEFINE_PROP_STRING ("priv_spec" ,RISCVCPU , cfg.priv_spec), DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),@@ -970,6 +971,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)ISA_EDATA_ENTRY (svinval, ext_svinval),ISA_EDATA_ENTRY (svnapot, ext_svnapot),ISA_EDATA_ENTRY (svpbmt, ext_svpbmt), + ISA_EDATA_ENTRY(smstateen, ext_smstateen), };for (i =0 ; i <ARRAY_SIZE (isa_edata_arr); i++) {target/riscv/cpu.h +4
-0@@ -338,6 +338,9 @@ struct CPURISCVState {/* 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 @@ -416,6 +419,7 @@ struct RISCVCPUConfig {bool ext_zve32f;bool ext_zve64f;bool ext_sscofpmf; +bool ext_smstateen;/* Vendor-specific custom extensions */ bool ext_XVentanaCondOps;target/riscv/cpu_bits.h +36
-0@@ -205,6 +205,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@@ -258,6 +264,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@@ -309,6 +325,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 0x757target/riscv/csr.c +36
-0@@ -265,6 +265,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) {@@ -1882,6 +1918,129 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,return RISCV_EXCP_NONE ; }
Quoted 5 lines
target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1922
Quoted 2 lines
target/riscv/csr.c +2
-0+static inline void write_smstateen (CPURISCVState *env,uint64_t *reg, +uint64_t wr_mask,uint64_t new_val)It's better that '
uint64_t' is aligned with 'CPURISCVState' here. The same to following similar cases.
Ok. Will fix it in the next version.
Quoted 35 lines
Quoted 22 lines
target/riscv/csr.c +21
-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); +target/riscv/csr.c static RISCVException write_henvcfgh(CPURISCVState *env, int csrno, · line 1953
Quoted 10 lines
target/riscv/csr.c +10
-0+return RISCV_EXCP_NONE ; +} + +static RISCVException read_mstateenh (CPURISCVState *env,int csrno, +target_ulong *val) +{ + *val = env->mstateen[csrno -CSR_MSTATEEN0H -0x10 ] >>32 ; + +return RISCV_EXCP_NONE ; +}Why minus
0x10here?
Quoted 6 lines
when read
CSR_MSTATEEN0H, the csrno should beCSR_MSTATEEN0H, and the expected result is mstateen[0] >> 32.So, minus
0x10seems unnecessary.The same to following similar cases.
Correct. I will fix in the next version.
Quoted 207 lines
Quoted 95 lines
target/riscv/csr.c +90
-0+ +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 -0x10 ]; + 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 -0x10 ] >>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 -0x10 ; + + 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)target/riscv/csr.c riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { · line 3831
Quoted 9 lines
target/riscv/csr.c +6
-0@@ -3664,6 +3823,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 },It' better to aligned with
CSR_MSTATEEN0here.Regards,
Weiwei Li
Quoted 95 lines
target/riscv/csr.c +45
-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@@ -232,6 +232,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;@@ -331,7 +351,6 @@ const VMStateDescription vmstate_riscv_cpu = {VMSTATE_UINT64 (env.mtohost,RISCVCPU ),VMSTATE_UINT64 (env.mtimecmp,RISCVCPU ),VMSTATE_UINT64 (env.stimecmp,RISCVCPU ), -VMSTATE_END_OF_LIST () }, .subsections = (const VMStateDescription * []) {@@ -342,6 +361,7 @@ const VMStateDescription vmstate_riscv_cpu = { &vmstate_rv128, &vmstate_kvmtimer, &vmstate_envcfg, + &vmstate_smstateen, NULL } };
RFC PATCH v2 2/4 target/riscv: smstateen check for h/senvcfg
In reply to Mayuresh ChitaleAccesses 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 | 82 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 6 deletions(-)
target/riscv/csr.c +76 -6
@@ -39,6 +39,35 @@ 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 (!(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 ; + } + } + +return RISCV_EXCP_NONE ; +} +static RISCVException fs (CPURISCVState *env,int csrno) {#if !defined(CONFIG_USER_ONLY )
@@ -1865,6 +1894,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 ; }
@@ -1873,15 +1909,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 ; }
@@ -1890,6 +1938,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 ;
@@ -1903,6 +1957,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 ; }
@@ -1912,9 +1973,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 ; }
@@ -1936,7 +2002,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);
@@ -1957,7 +2024,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 -0x10 ]; val = (uint64_t )new_val <<32 ;
@@ -1979,7 +2047,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];
@@ -2002,8 +2071,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 -0x10 ; +uint64_t wr_mask = (1UL <<SMSTATEEN_STATEN ) | + (1UL <<SMSTATEEN0_HSENVCFG ); reg = &env->hstateen[index]; val = (uint64_t )new_val <<32 ;
Signature · 1 line
2.17.1
Re: [RFC PATCH v2 2/4] target/riscv: smstateen check for h/senvcfg
Quoted 21 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 | 82 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 6 deletions(-)target/riscv/csr.c +3
-0@@ -39,6 +39,35 @@ 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);
target/riscv/csr.c void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) · line 54
Quoted 10 lines
target/riscv/csr.c +10
-0+RISCVCPU *cpu =RISCV_CPU (cs); +bool virt =riscv_cpu_virt_enabled (env); + +if (!cpu->cfg.ext_smstateen) { +return RISCV_EXCP_NONE ; + } + +if (!(env->mstateen[0 ] &1UL << bit)) { +return RISCV_EXCP_ILLEGAL_INST ; + }
I think here should be " & (1UL << bit) " . The same for following similar cases.
Regards,
Weiwei Li
Re: [RFC PATCH v2 2/4] target/riscv: smstateen check for h/senvcfg
Quoted 29 lines · Weiwei Li
Quoted 26 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 | 82 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 6 deletions(-)target/riscv/csr.c +8
-0@@ -39,6 +39,35 @@ 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 ;
Quoted 8 lines
target/riscv/csr.c void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) · line 54
Quoted 5 lines
target/riscv/csr.c +5
-0+ } + +if (!(env->mstateen[0 ] &1UL << bit)) { +return RISCV_EXCP_ILLEGAL_INST ; + }I think here should be " & (1UL << bit) " . The same for following similar cases.
Ok. Will fix it in the next version.
Quoted 5 lines
Regards,
Weiwei Li
RFC PATCH v2 3/4 target/riscv: smstateen check for fcsr
In reply to Mayuresh ChitaleIf 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
@@ -75,6 +75,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 ; }
@@ -2006,6 +2010,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 ;
@@ -2030,6 +2038,10 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno, reg = &env->mstateen[csrno -CSR_MSTATEEN0H -0x10 ]; 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 ;
@@ -2051,6 +2063,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);
@@ -2075,6 +2091,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 ;
@@ -2100,6 +2120,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.17.1
RFC PATCH v2 4/4 target/riscv: smstateen check for AIA/IMSIC
In reply to Mayuresh ChitaleIf 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 | 248 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 246 insertions(+), 2 deletions(-)
target/riscv/csr.c +246 -2
@@ -68,6 +68,53 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)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); +} +static RISCVException fs (CPURISCVState *env,int csrno) {#if !defined(CONFIG_USER_ONLY )
@@ -1402,6 +1449,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);
@@ -1484,7 +1538,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);
@@ -1513,11 +1569,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 );
@@ -1551,6 +1616,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);
@@ -1669,6 +1741,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);
@@ -2014,6 +2093,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 ;
@@ -2042,6 +2127,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 ;
@@ -2067,6 +2158,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);
@@ -2095,6 +2192,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 ;
@@ -2288,6 +2391,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) {
@@ -2342,6 +2451,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) {
@@ -2504,6 +2619,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) {
@@ -2558,6 +2679,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) {
@@ -2748,6 +2875,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." );
@@ -2808,6 +2939,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) {
@@ -2854,6 +2991,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) {
@@ -2908,6 +3051,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; }
@@ -2917,6 +3067,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;
@@ -2956,6 +3113,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; }
@@ -3026,12 +3190,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 ; }
@@ -3090,41 +3270,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.17.1
Re: [RFC PATCH v2 4/4] target/riscv: smstateen check for AIA/IMSIC
Quoted 96 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 | 248 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 246 insertions(+), 2 deletions(-)target/riscv/csr.c +57
-1@@ -68,6 +68,53 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)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); +} +static RISCVException fs (CPURISCVState *env,int csrno) {#if !defined(CONFIG_USER_ONLY )@@ -1402,6 +1449,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);@@ -1484,7 +1538,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 static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val, · line 1574
Quoted 7 lines
target/riscv/csr.c +3
-1@@ -1513,11 +1569,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 )) {
It seems better that '(' aligned with '!'.
Quoted 43 lines
target/riscv/csr.c +21
-0+ 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 );@@ -1551,6 +1616,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);@@ -1669,6 +1741,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 static RISCVException write_mstateen(CPURISCVState *env, int csrno, · line 2099
Quoted 7 lines
target/riscv/csr.c +4
-0@@ -2014,6 +2093,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 );
I think it's better as follows:
+ wr_mask |= (1UL << SMSTATEEN0_IMSIC) |
+ (1UL << SMSTATEEN0_AIA) |
+ (1UL << SMSTATEEN0_SVSLCT);
Regards,
Weiwei Li
Re: [RFC PATCH v2 4/4] target/riscv: smstateen check for AIA/IMSIC
Quoted 153 lines · Weiwei Li
Quoted 96 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 | 248 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 246 insertions(+), 2 deletions(-)target/riscv/csr.c +57
-1@@ -68,6 +68,53 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)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); +} +static RISCVException fs (CPURISCVState *env,int csrno) {#if !defined(CONFIG_USER_ONLY )@@ -1402,6 +1449,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);@@ -1484,7 +1538,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 static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val, · line 1574
Quoted 7 lines
target/riscv/csr.c +3
-1@@ -1513,11 +1569,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 )) {It seems better that '(' aligned with '!'.
Quoted 46 lines
target/riscv/csr.c +21
-0+ 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 );@@ -1551,6 +1616,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);@@ -1669,6 +1741,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);@@ -2014,6 +2093,12 @@ static RISCVException write_mstateen(CPURISCVState *env, int csrno, wr_mask |=1UL <<SMSTATEEN0_FCSR ; }
Quoted 6 lines
target/riscv/csr.c static RISCVException write_mstateen(CPURISCVState *env, int csrno, · line 2099
Quoted 4 lines
target/riscv/csr.c +4
-0+if (riscv_feature (env,RISCV_FEATURE_AIA )) { + wr_mask |= (1UL <<SMSTATEEN0_IMSIC ) + | (1UL <<SMSTATEEN0_AIA ) + | (1UL <<SMSTATEEN0_SVSLCT );I think it's better as follows:
Ok. Will fix it in the next version.
Quoted 9 lines
(quoted diff) +3
-0+ wr_mask |= (1UL << SMSTATEEN0_IMSIC) | + (1UL << SMSTATEEN0_AIA) | + (1UL << SMSTATEEN0_SVSLCT);Regards,
Weiwei Li