* [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions
@ 2024-09-25 11:57 Clément Léger
2024-09-25 11:57 ` [PATCH v2 1/8] target/riscv: Add Ssdbltrp CSRs handling Clément Léger
` (7 more replies)
0 siblings, 8 replies; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:57 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
A double trap typically arises during a sensitive phase in trap handling
operations — when an exception or interrupt occurs while the trap
handler (the component responsible for managing these events) is in a
non-reentrant state. This non-reentrancy usually occurs in the early
phase of trap handling, wherein the trap handler has not yet preserved
the necessary state to handle and resume from the trap. The occurrence
of such event is unlikely but can happen when dealing with hardware
errors.
This series adds support for Ssdbltrp and Smdbltrp ratified ISA
extensions [1]. It is based on the Smrnmi series [6].
Ssdbltrp can be tested using qemu[2], opensbi[3], linux[4] and
kvm-unit-tests[5]. Assuming you have a riscv environment available and
configured (CROSS_COMPILE), it can be built for riscv64 using the
following instructions:
Qemu:
$ git clone https://github.com/rivosinc/qemu.git
$ cd qemu
$ git switch -C dbltrp_v2 dev/cleger/dbltrp_v2
$ mkdir build && cd build
$ ../configure --target-list=riscv64-softmmu
$ make
OpenSBI:
$ git clone https://github.com/rivosinc/opensbi.git
$ cd opensbi
$ git switch -C dbltrp_v3 dev/cleger/dbltrp_v3
$ make O=build PLATFORM_RISCV_XLEN=64 PLATFORM=generic
Linux:
$ git clone https://github.com/rivosinc/linux.git
$ cd linux
$ git switch -C dbltrp_v1 dev/cleger/dbltrp_v1
$ export ARCH=riscv
$ make O=build defconfig
$ ./script/config --file build/.config --enable RISCV_DBLTRP
$ make O=build
kvm-unit-tests:
$ git clone https://github.com/clementleger/kvm-unit-tests.git
$ cd kvm-unit-tests
$ git switch -C dbltrp_v1 dev/cleger/dbltrp_v1
$ ./configure --arch=riscv64 --cross-prefix=$CROSS_COMPILE
$ make
You will also need kvmtool in your rootfs.
Run with kvm-unit-test test as kernel:
$ qemu-system-riscv64 \
-M virt \
-cpu rv64,ssdbltrp=true,smdbltrp=true \
-nographic \
-serial mon:stdio \
-bios opensbi/build/platform/generic/firmware/fw_jump.bin \
-kernel kvm-unit-tests-dbltrp/riscv/sbi_dbltrp.flat
...
[OpenSBI boot partially elided]
Boot HART ISA Extensions : sscofpmf,sstc,zicntr,zihpm,zicboz,zicbom,sdtrig,svadu,ssdbltrp
...
##########################################################################
# kvm-unit-tests
##########################################################################
PASS: sbi: fwft: FWFT extension probing no error
PASS: sbi: fwft: FWFT extension is present
PASS: sbi: fwft: dbltrp: Get double trap enable feature value
PASS: sbi: fwft: dbltrp: Set double trap enable feature value == 0
PASS: sbi: fwft: dbltrp: Get double trap enable feature value == 0
PASS: sbi: fwft: dbltrp: Double trap disabled, trap first time ok
PASS: sbi: fwft: dbltrp: Set double trap enable feature value == 1
PASS: sbi: fwft: dbltrp: Get double trap enable feature value == 1
PASS: sbi: fwft: dbltrp: Trapped twice allowed ok
INFO: sbi: fwft: dbltrp: Should generate a double trap and crash !
sbi_trap_error: hart0: trap0: double trap handler failed (error -10)
sbi_trap_error: hart0: trap0: mcause=0x0000000000000010 mtval=0x0000000000000000
sbi_trap_error: hart0: trap0: mtval2=0x0000000000000003 mtinst=0x0000000000000000
sbi_trap_error: hart0: trap0: mepc=0x00000000802000d8 mstatus=0x8000000a01006900
sbi_trap_error: hart0: trap0: ra=0x00000000802001fc sp=0x0000000080213e70
sbi_trap_error: hart0: trap0: gp=0x0000000000000000 tp=0x0000000080088000
sbi_trap_error: hart0: trap0: s0=0x0000000080213e80 s1=0x0000000000000001
sbi_trap_error: hart0: trap0: a0=0x0000000080213e80 a1=0x0000000080208193
sbi_trap_error: hart0: trap0: a2=0x000000008020dc20 a3=0x000000000000000f
sbi_trap_error: hart0: trap0: a4=0x0000000080210cd8 a5=0x00000000802110d0
sbi_trap_error: hart0: trap0: a6=0x00000000802136e4 a7=0x0000000046574654
sbi_trap_error: hart0: trap0: s2=0x0000000080210cd9 s3=0x0000000000000000
sbi_trap_error: hart0: trap0: s4=0x0000000000000000 s5=0x0000000000000000
sbi_trap_error: hart0: trap0: s6=0x0000000000000000 s7=0x0000000000000001
sbi_trap_error: hart0: trap0: s8=0x0000000000002000 s9=0x0000000080083700
sbi_trap_error: hart0: trap0: s10=0x0000000000000000 s11=0x0000000000000000
sbi_trap_error: hart0: trap0: t0=0x0000000000000000 t1=0x0000000080213ed8
sbi_trap_error: hart0: trap0: t2=0x0000000000001000 t3=0x0000000080213ee0
sbi_trap_error: hart0: trap0: t4=0x0000000000000000 t5=0x000000008020f8d0
sbi_trap_error: hart0: trap0: t6=0x0000000000000000
Run with linux and kvm-unit-test test in kvm (testing VS-mode):
$ qemu-system-riscv64 \
-M virt \
-cpu rv64,ssdbltrp=true,smdbltrp=true \
-nographic \
-serial mon:stdio \
-bios opensbi/build/platform/generic/firmware/fw_jump.bin \
-kernel linux/build/arch/riscv/boot/Image
...
[Linux boot partially elided]
[ 0.735079] riscv-dbltrp: Double trap handling registered
...
$ lkvm run -k sbi_dbltrp.flat -m 128 -c 2
##########################################################################
# kvm-unit-tests
##########################################################################
PASS: sbi: fwft: FWFT extension probing no error
PASS: sbi: fwft: FWFT extension is present
PASS: sbi: fwft: dbltrp: Get double trap enable feature value
PASS: sbi: fwft: dbltrp: Set double trap enable feature value == 0
PASS: sbi: fwft: dbltrp: Get double trap enable feature value == 0
PASS: sbi: fwft: dbltrp: Double trap disabled, trap first time ok
PASS: sbi: fwft: dbltrp: Set double trap enable feature value == 1
PASS: sbi: fwft: dbltrp: Get double trap enable feature value == 1
PASS: sbi: fwft: dbltrp: Trapped twice allowed ok
INFO: sbi: fwft: dbltrp: Should generate a double trap and crash !
[ 51.939077] Guest double trap
[ 51.939323] kvm [93]: VCPU exit error -95
[ 51.939683] kvm [93]: SEPC=0x802000d8 SSTATUS=0x200004520 HSTATUS=0x200200180
[ 51.939947] kvm [93]: SCAUSE=0x10 STVAL=0x0 HTVAL=0x3 HTINST=0x0
KVM_RUN failed: Operation not supported
$
Testing Smbdbltrp can be done using gdb and trigger some trap. For
instance, interrupt M-mode firmware at some point, set mstatus.mdt = 1
and corrupt some register to generate a NULL pointer exception.
Link: https://github.com/riscv/riscv-isa-manual/commit/52a5742d5ab5a0792019033631b2035a493ad981 [1]
Link: https://github.com/rivosinc/qemu/tree/dev/cleger/dbltrp_v2 [2]
Link: https://github.com/rivosinc/opensbi/tree/dev/cleger/dbltrp_v3 [3]
Link: https://github.com/rivosinc/linux/tree/dev/cleger/dbltrp_v1 [4]
Link: https://github.com/clementleger/kvm-unit-tests/tree/dev/cleger/dbltrp_v1 [5]
Link: https://lore.kernel.org/all/CAKmqyKO9k93OLX_pxtkHcUAvC=NUNxtp6mDCb2J2ZWHMnfbNYQ@mail.gmail.com/T/ [6]
---
V2:
- Squashed commits that added ext_s{s|m}dbltrp as suggested by Daniel
Clément Léger (8):
target/riscv: Add Ssdbltrp CSRs handling
target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
target/riscv: Implement Ssdbltrp exception handling
target/riscv: Add Ssdbltrp ISA extension enable switch
target/riscv: Add Smdbltrp CSRs handling
target/riscv: Implement Smdbltrp sret, mret and mnret behavior
target/riscv: Implement Smdbltrp behavior
target/riscv: Add Smdbltrp ISA extension enable switch
target/riscv/cpu.c | 9 +++-
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 8 ++++
target/riscv/cpu_cfg.h | 2 +
target/riscv/cpu_helper.c | 98 +++++++++++++++++++++++++++++++++------
target/riscv/csr.c | 60 ++++++++++++++++++++----
target/riscv/op_helper.c | 47 ++++++++++++++++++-
7 files changed, 198 insertions(+), 27 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/8] target/riscv: Add Ssdbltrp CSRs handling
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
@ 2024-09-25 11:57 ` Clément Léger
2024-10-11 2:49 ` Alistair Francis
2024-09-25 11:58 ` [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior Clément Léger
` (6 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:57 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
Add ext_ssdbltrp in RISCVCPUConfig and implement MSTATUS.SDT,
{H|M}ENVCFG.DTE and modify the availability of MTVAL2 based on the
presence of the Ssdbltrp ISA extension.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 6 ++++++
target/riscv/cpu_cfg.h | 1 +
target/riscv/cpu_helper.c | 16 ++++++++++++++
target/riscv/csr.c | 45 ++++++++++++++++++++++++++++++---------
5 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a84e719d3f..ee984bf270 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -553,6 +553,7 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong geilen);
bool riscv_cpu_vector_enabled(CPURISCVState *env);
void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
int riscv_env_mmu_index(CPURISCVState *env, bool ifetch);
+bool riscv_env_smode_dbltrp_enabled(CPURISCVState *env, bool virt);
G_NORETURN void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
MMUAccessType access_type,
int mmu_idx, uintptr_t retaddr);
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index da1723496c..3a5588d4df 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -558,6 +558,7 @@
#define MSTATUS_TVM 0x00100000 /* since: priv-1.10 */
#define MSTATUS_TW 0x00200000 /* since: priv-1.10 */
#define MSTATUS_TSR 0x00400000 /* since: priv-1.10 */
+#define MSTATUS_SDT 0x01000000
#define MSTATUS_GVA 0x4000000000ULL
#define MSTATUS_MPV 0x8000000000ULL
@@ -588,6 +589,7 @@ typedef enum {
#define SSTATUS_XS 0x00018000
#define SSTATUS_SUM 0x00040000 /* since: priv-1.10 */
#define SSTATUS_MXR 0x00080000
+#define SSTATUS_SDT 0x01000000
#define SSTATUS64_UXL 0x0000000300000000ULL
@@ -777,11 +779,13 @@ typedef enum RISCVException {
#define MENVCFG_CBIE (3UL << 4)
#define MENVCFG_CBCFE BIT(6)
#define MENVCFG_CBZE BIT(7)
+#define MENVCFG_DTE (1ULL << 59)
#define MENVCFG_ADUE (1ULL << 61)
#define MENVCFG_PBMTE (1ULL << 62)
#define MENVCFG_STCE (1ULL << 63)
/* For RV32 */
+#define MENVCFGH_DTE BIT(27)
#define MENVCFGH_ADUE BIT(29)
#define MENVCFGH_PBMTE BIT(30)
#define MENVCFGH_STCE BIT(31)
@@ -795,11 +799,13 @@ typedef enum RISCVException {
#define HENVCFG_CBIE MENVCFG_CBIE
#define HENVCFG_CBCFE MENVCFG_CBCFE
#define HENVCFG_CBZE MENVCFG_CBZE
+#define HENVCFG_DTE MENVCFG_DTE
#define HENVCFG_ADUE MENVCFG_ADUE
#define HENVCFG_PBMTE MENVCFG_PBMTE
#define HENVCFG_STCE MENVCFG_STCE
/* For RV32 */
+#define HENVCFGH_DTE MENVCFGH_DTE
#define HENVCFGH_ADUE MENVCFGH_ADUE
#define HENVCFGH_PBMTE MENVCFGH_PBMTE
#define HENVCFGH_STCE MENVCFGH_STCE
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index ae2a945b5f..dd804f95d4 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -77,6 +77,7 @@ struct RISCVCPUConfig {
bool ext_smstateen;
bool ext_sstc;
bool ext_smcntrpmf;
+ bool ext_ssdbltrp;
bool ext_svadu;
bool ext_svinval;
bool ext_svnapot;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9d0400035f..395d8235ce 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -63,6 +63,22 @@ int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
#endif
}
+bool riscv_env_smode_dbltrp_enabled(CPURISCVState *env, bool virt)
+{
+#ifdef CONFIG_USER_ONLY
+ return false;
+#else
+ if (!riscv_cpu_cfg(env)->ext_ssdbltrp) {
+ return false;
+ }
+ if (virt) {
+ return (env->henvcfg & HENVCFG_DTE) != 0;
+ } else {
+ return (env->menvcfg & MENVCFG_DTE) != 0;
+ }
+#endif
+}
+
void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
uint64_t *cs_base, uint32_t *pflags)
{
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e5de72453c..d8280ec956 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -540,6 +540,15 @@ static RISCVException aia_hmode32(CPURISCVState *env, int csrno)
return hmode32(env, csrno);
}
+static RISCVException dbltrp_hmode(CPURISCVState *env, int csrno)
+{
+ if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+ return RISCV_EXCP_NONE;
+ }
+
+ return hmode(env, csrno);
+}
+
static RISCVException pmp(CPURISCVState *env, int csrno)
{
if (riscv_cpu_cfg(env)->pmp) {
@@ -1402,7 +1411,7 @@ static const target_ulong vs_delegable_excps = DELEGABLE_EXCPS &
(1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)));
static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
- SSTATUS_SUM | SSTATUS_MXR | SSTATUS_VS;
+ SSTATUS_SUM | SSTATUS_MXR | SSTATUS_VS | SSTATUS_SDT;
/*
* Spec allows for bits 13:63 to be either read-only or writable.
@@ -1600,6 +1609,14 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
mask |= MSTATUS_VS;
}
+ if (riscv_env_smode_dbltrp_enabled(env, env->virt_enabled)) {
+ mask |= MSTATUS_SDT;
+ if ((val & MSTATUS_SDT) != 0) {
+ mstatus &= ~MSTATUS_SIE;
+ val &= ~MSTATUS_SIE;
+ }
+ }
+
if (xl != MXL_RV32 || env->debugger) {
if (riscv_has_ext(env, RVH)) {
mask |= MSTATUS_MPV | MSTATUS_GVA;
@@ -2354,7 +2371,8 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
if (riscv_cpu_mxl(env) == MXL_RV64) {
mask |= (cfg->ext_svpbmt ? MENVCFG_PBMTE : 0) |
(cfg->ext_sstc ? MENVCFG_STCE : 0) |
- (cfg->ext_svadu ? MENVCFG_ADUE : 0);
+ (cfg->ext_svadu ? MENVCFG_ADUE : 0) |
+ (cfg->ext_ssdbltrp ? MENVCFG_DTE : 0);
}
env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
@@ -2374,7 +2392,8 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
uint64_t mask = (cfg->ext_svpbmt ? MENVCFG_PBMTE : 0) |
(cfg->ext_sstc ? MENVCFG_STCE : 0) |
- (cfg->ext_svadu ? MENVCFG_ADUE : 0);
+ (cfg->ext_svadu ? MENVCFG_ADUE : 0) |
+ (cfg->ext_ssdbltrp ? MENVCFG_DTE : 0);
uint64_t valh = (uint64_t)val << 32;
env->menvcfg = (env->menvcfg & ~mask) | (valh & mask);
@@ -2425,9 +2444,10 @@ static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
* henvcfg.pbmte is read_only 0 when menvcfg.pbmte = 0
* henvcfg.stce is read_only 0 when menvcfg.stce = 0
* henvcfg.adue is read_only 0 when menvcfg.adue = 0
+ * henvcfg.dte is read_only 0 when menvcfg.dte = 0
*/
- *val = env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE) |
- env->menvcfg);
+ *val = env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE |
+ HENVCFG_DTE) | env->menvcfg);
return RISCV_EXCP_NONE;
}
@@ -2435,6 +2455,7 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
target_ulong val)
{
uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+ uint64_t menvcfg_mask;
RISCVException ret;
ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
@@ -2443,7 +2464,11 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
}
if (riscv_cpu_mxl(env) == MXL_RV64) {
- mask |= env->menvcfg & (HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE);
+ menvcfg_mask = HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE;
+ if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+ menvcfg_mask |= HENVCFG_DTE;
+ }
+ mask |= env->menvcfg & menvcfg_mask;
}
env->henvcfg = (env->henvcfg & ~mask) | (val & mask);
@@ -2461,8 +2486,8 @@ static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
return ret;
}
- *val = (env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE) |
- env->menvcfg)) >> 32;
+ *val = (env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE |
+ HENVCFG_DTE) | env->menvcfg)) >> 32;
return RISCV_EXCP_NONE;
}
@@ -2470,7 +2495,7 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
target_ulong val)
{
uint64_t mask = env->menvcfg & (HENVCFG_PBMTE | HENVCFG_STCE |
- HENVCFG_ADUE);
+ HENVCFG_ADUE | HENVCFG_DTE);
uint64_t valh = (uint64_t)val << 32;
RISCVException ret;
@@ -5246,7 +5271,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_VSATP] = { "vsatp", hmode, read_vsatp, write_vsatp,
.min_priv_ver = PRIV_VERSION_1_12_0 },
- [CSR_MTVAL2] = { "mtval2", hmode, read_mtval2, write_mtval2,
+ [CSR_MTVAL2] = { "mtval2", dbltrp_hmode, read_mtval2, write_mtval2,
.min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MTINST] = { "mtinst", hmode, read_mtinst, write_mtinst,
.min_priv_ver = PRIV_VERSION_1_12_0 },
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
2024-09-25 11:57 ` [PATCH v2 1/8] target/riscv: Add Ssdbltrp CSRs handling Clément Léger
@ 2024-09-25 11:58 ` Clément Léger
2024-10-11 3:13 ` Alistair Francis
2024-09-25 11:58 ` [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling Clément Léger
` (5 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:58 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
When the Ssdbltrp extension is enabled, SSTATUS.SDT field is cleared
when executing sret. When executing mret/mnret, SSTATUS.SDT is cleared
when returning to U, VS or VU and VSSTATUS.SDT is cleared when returning
to VU from HS.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/op_helper.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 6895c7596b..00b6f75102 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -287,6 +287,18 @@ target_ulong helper_sret(CPURISCVState *env)
get_field(mstatus, MSTATUS_SPIE));
mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
+
+ if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+ if (riscv_has_ext(env, RVH)) {
+ target_ulong prev_vu = get_field(env->hstatus, HSTATUS_SPV) &&
+ prev_priv == PRV_U;
+ /* Returning to VU from HS, vsstatus.sdt = 0 */
+ if (!env->virt_enabled && prev_vu) {
+ env->vsstatus = set_field(env->vsstatus, MSTATUS_SDT, 0);
+ }
+ }
+ mstatus = set_field(mstatus, MSTATUS_SDT, 0);
+ }
if (env->priv_ver >= PRIV_VERSION_1_12_0) {
mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
}
@@ -297,7 +309,6 @@ target_ulong helper_sret(CPURISCVState *env)
target_ulong hstatus = env->hstatus;
prev_virt = get_field(hstatus, HSTATUS_SPV);
-
hstatus = set_field(hstatus, HSTATUS_SPV, 0);
env->hstatus = hstatus;
@@ -328,6 +339,22 @@ static void check_ret_from_m_mode(CPURISCVState *env, target_ulong retpc,
riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, GETPC());
}
}
+static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,
+ target_ulong prev_priv,
+ target_ulong prev_virt)
+{
+ /* If returning to U, VS or VU, sstatus.sdt = 0 */
+ if (prev_priv == PRV_U || (prev_virt &&
+ (prev_priv == PRV_S || prev_priv == PRV_U))) {
+ mstatus = set_field(mstatus, MSTATUS_SDT, 0);
+ /* If returning to VU, vsstatus.sdt = 0 */
+ if (prev_virt && prev_priv == PRV_U) {
+ env->vsstatus = set_field(env->vsstatus, MSTATUS_SDT, 0);
+ }
+ }
+
+ return mstatus;
+}
target_ulong helper_mret(CPURISCVState *env)
{
@@ -345,6 +372,9 @@ target_ulong helper_mret(CPURISCVState *env)
mstatus = set_field(mstatus, MSTATUS_MPP,
riscv_has_ext(env, RVU) ? PRV_U : PRV_M);
mstatus = set_field(mstatus, MSTATUS_MPV, 0);
+ if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+ mstatus = ssdbltrp_mxret(env, mstatus, prev_priv, prev_virt);
+ }
if ((env->priv_ver >= PRIV_VERSION_1_12_0) && (prev_priv != PRV_M)) {
mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
}
@@ -382,6 +412,9 @@ target_ulong helper_mnret(CPURISCVState *env)
if (prev_priv < PRV_M) {
env->mstatus = set_field(env->mstatus, MSTATUS_MPRV, false);
}
+ if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+ env->mstatus = ssdbltrp_mxret(env, env->mstatus, prev_priv, prev_virt);
+ }
if (riscv_has_ext(env, RVH) && prev_virt) {
riscv_cpu_swap_hypervisor_regs(env);
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
2024-09-25 11:57 ` [PATCH v2 1/8] target/riscv: Add Ssdbltrp CSRs handling Clément Léger
2024-09-25 11:58 ` [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior Clément Léger
@ 2024-09-25 11:58 ` Clément Léger
2024-10-11 3:22 ` Alistair Francis
2024-09-25 11:58 ` [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch Clément Léger
` (4 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:58 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
while SSTATUS.SDT isn't cleared, generate a double trap exception to
M-mode.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/cpu.c | 2 +-
target/riscv/cpu_bits.h | 1 +
target/riscv/cpu_helper.c | 47 ++++++++++++++++++++++++++++++++++-----
3 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cf06cd741a..65347ccd5a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
"load_page_fault",
"reserved",
"store_page_fault",
- "reserved",
+ "double_trap",
"reserved",
"reserved",
"reserved",
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 3a5588d4df..5557a86348 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -699,6 +699,7 @@ typedef enum RISCVException {
RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
+ RISCV_EXCP_DOUBLE_TRAP = 0x10,
RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 395d8235ce..69da3c3384 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
mstatus_mask |= MSTATUS_FS;
}
bool current_virt = env->virt_enabled;
-
+ if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
+ mstatus_mask |= MSTATUS_SDT;
+ }
g_assert(riscv_has_ext(env, RVH));
if (current_virt) {
@@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
CPURISCVState *env = &cpu->env;
bool virt = env->virt_enabled;
bool write_gva = false;
+ bool vsmode_exc;
uint64_t s;
int mode;
@@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
!(env->mip & (1 << cause));
bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
!(env->mip & (1 << cause));
+ bool smode_double_trap = false;
+ uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
target_ulong tval = 0;
target_ulong tinst = 0;
target_ulong htval = 0;
@@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
!async &&
mode == PRV_M;
+ vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
+ /*
+ * Check double trap condition only if already in S-mode and targeting
+ * S-mode
+ */
+ if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
+ bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
+ bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
+ /* In VS or HS */
+ if (riscv_has_ext(env, RVH)) {
+ if (vsmode_exc) {
+ /* VS -> VS */
+ /* Stay in VS mode, use henvcfg instead of menvcfg*/
+ dte = (env->henvcfg & HENVCFG_DTE) != 0;
+ } else if (env->virt_enabled) {
+ /* VS -> HS */
+ dte = false;
+ }
+ }
+ smode_double_trap = dte && sdt;
+ if (smode_double_trap) {
+ mode = PRV_M;
+ }
+ }
+
if (mode == PRV_S) {
/* handle the trap in S-mode */
if (riscv_has_ext(env, RVH)) {
- uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
-
- if (env->virt_enabled &&
- (((hdeleg >> cause) & 1) || vs_injected)) {
+ if (vsmode_exc) {
/* Trap to VS mode */
/*
* See if we need to adjust cause. Yes if its VS mode interrupt
@@ -1876,6 +1903,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
s = set_field(s, MSTATUS_SPIE, get_field(s, MSTATUS_SIE));
s = set_field(s, MSTATUS_SPP, env->priv);
s = set_field(s, MSTATUS_SIE, 0);
+ if (riscv_env_smode_dbltrp_enabled(env, virt)) {
+ s = set_field(s, MSTATUS_SDT, 1);
+ }
env->mstatus = s;
env->scause = cause | ((target_ulong)async << (TARGET_LONG_BITS - 1));
env->sepc = env->pc;
@@ -1909,9 +1939,14 @@ void riscv_cpu_do_interrupt(CPUState *cs)
s = set_field(s, MSTATUS_MIE, 0);
env->mstatus = s;
env->mcause = cause | ~(((target_ulong)-1) >> async);
+ if (smode_double_trap) {
+ env->mtval2 = env->mcause;
+ env->mcause = RISCV_EXCP_DOUBLE_TRAP;
+ } else {
+ env->mtval2 = mtval2;
+ }
env->mepc = env->pc;
env->mtval = tval;
- env->mtval2 = mtval2;
env->mtinst = tinst;
if (cpu->cfg.ext_smrnmi && nmi_execp) {
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
` (2 preceding siblings ...)
2024-09-25 11:58 ` [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling Clément Léger
@ 2024-09-25 11:58 ` Clément Léger
2024-10-11 3:24 ` Alistair Francis
2024-09-25 11:58 ` [PATCH v2 5/8] target/riscv: Add Smdbltrp CSRs handling Clément Léger
` (3 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:58 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
Add the switch to enable the Ssdbltrp ISA extension.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 65347ccd5a..4f52cf7ac0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -190,6 +190,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, has_priv_1_11),
ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, has_priv_1_12),
+ ISA_EXT_DATA_ENTRY(ssdbltrp, PRIV_VERSION_1_12_0, ext_ssdbltrp),
ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
@@ -1492,6 +1493,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("smrnmi", ext_smrnmi, false),
MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
MULTI_EXT_CFG_BOOL("ssaia", ext_ssaia, false),
+ MULTI_EXT_CFG_BOOL("ssdbltrp", ext_ssdbltrp, false),
MULTI_EXT_CFG_BOOL("svade", ext_svade, false),
MULTI_EXT_CFG_BOOL("svadu", ext_svadu, true),
MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 5/8] target/riscv: Add Smdbltrp CSRs handling
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
` (3 preceding siblings ...)
2024-09-25 11:58 ` [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch Clément Léger
@ 2024-09-25 11:58 ` Clément Léger
2024-10-11 3:30 ` Alistair Francis
2024-09-25 11:58 ` [PATCH v2 6/8] target/riscv: Implement Smdbltrp sret, mret and mnret behavior Clément Léger
` (2 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:58 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
Add `ext_smdbltrp`in RISCVCPUConfig and implement MSTATUS.MDT behavior.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/cpu_bits.h | 1 +
target/riscv/cpu_cfg.h | 1 +
target/riscv/csr.c | 15 +++++++++++++++
3 files changed, 17 insertions(+)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5557a86348..62bab1bf55 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -561,6 +561,7 @@
#define MSTATUS_SDT 0x01000000
#define MSTATUS_GVA 0x4000000000ULL
#define MSTATUS_MPV 0x8000000000ULL
+#define MSTATUS_MDT 0x40000000000ULL /* Smdbltrp extension */
#define MSTATUS64_UXL 0x0000000300000000ULL
#define MSTATUS64_SXL 0x0000000C00000000ULL
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index dd804f95d4..4c4caa2b39 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -78,6 +78,7 @@ struct RISCVCPUConfig {
bool ext_sstc;
bool ext_smcntrpmf;
bool ext_ssdbltrp;
+ bool ext_smdbltrp;
bool ext_svadu;
bool ext_svinval;
bool ext_svnapot;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d8280ec956..cc1940447a 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1617,6 +1617,14 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
}
}
+ if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+ mask |= MSTATUS_MDT;
+ if ((val & MSTATUS_MDT) != 0) {
+ mstatus &= ~MSTATUS_MIE;
+ val &= ~MSTATUS_MIE;
+ }
+ }
+
if (xl != MXL_RV32 || env->debugger) {
if (riscv_has_ext(env, RVH)) {
mask |= MSTATUS_MPV | MSTATUS_GVA;
@@ -1655,6 +1663,13 @@ static RISCVException write_mstatush(CPURISCVState *env, int csrno,
uint64_t valh = (uint64_t)val << 32;
uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0;
+ if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+ mask |= MSTATUS_MDT;
+ if ((val & MSTATUS_MDT) != 0) {
+ env->mstatus &= ~MSTATUS_MIE;
+ val &= ~MSTATUS_MIE;
+ }
+ }
env->mstatus = (env->mstatus & ~mask) | (valh & mask);
return RISCV_EXCP_NONE;
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 6/8] target/riscv: Implement Smdbltrp sret, mret and mnret behavior
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
` (4 preceding siblings ...)
2024-09-25 11:58 ` [PATCH v2 5/8] target/riscv: Add Smdbltrp CSRs handling Clément Léger
@ 2024-09-25 11:58 ` Clément Léger
2024-09-25 11:58 ` [PATCH v2 7/8] target/riscv: Implement Smdbltrp behavior Clément Léger
2024-09-25 11:58 ` [PATCH v2 8/8] target/riscv: Add Smdbltrp ISA extension enable switch Clément Léger
7 siblings, 0 replies; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:58 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
When the Ssdbltrp extension is enabled, SSTATUS.MDT field is cleared
when executing sret if executed in M-mode. When executing mret/mnret,
SSTATUS.MDT is cleared.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/op_helper.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 00b6f75102..9d0911f697 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -299,6 +299,9 @@ target_ulong helper_sret(CPURISCVState *env)
}
mstatus = set_field(mstatus, MSTATUS_SDT, 0);
}
+ if (riscv_cpu_cfg(env)->ext_smdbltrp && env->priv >= PRV_M) {
+ mstatus = set_field(mstatus, MSTATUS_MDT, 0);
+ }
if (env->priv_ver >= PRIV_VERSION_1_12_0) {
mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
}
@@ -375,6 +378,9 @@ target_ulong helper_mret(CPURISCVState *env)
if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
mstatus = ssdbltrp_mxret(env, mstatus, prev_priv, prev_virt);
}
+ if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+ mstatus = set_field(mstatus, MSTATUS_MDT, 0);
+ }
if ((env->priv_ver >= PRIV_VERSION_1_12_0) && (prev_priv != PRV_M)) {
mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
}
@@ -416,6 +422,12 @@ target_ulong helper_mnret(CPURISCVState *env)
env->mstatus = ssdbltrp_mxret(env, env->mstatus, prev_priv, prev_virt);
}
+ if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+ if (prev_priv < PRV_M) {
+ env->mstatus = set_field(env->mstatus, MSTATUS_MDT, false);
+ }
+ }
+
if (riscv_has_ext(env, RVH) && prev_virt) {
riscv_cpu_swap_hypervisor_regs(env);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 7/8] target/riscv: Implement Smdbltrp behavior
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
` (5 preceding siblings ...)
2024-09-25 11:58 ` [PATCH v2 6/8] target/riscv: Implement Smdbltrp sret, mret and mnret behavior Clément Léger
@ 2024-09-25 11:58 ` Clément Léger
2024-09-25 11:58 ` [PATCH v2 8/8] target/riscv: Add Smdbltrp ISA extension enable switch Clément Léger
7 siblings, 0 replies; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:58 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
When the Smsdbltrp ISA extension is enabled, MSTATUS.MDT bit is enabled
at reset and set upon trap. If a trap happens while MSTATUS.MDT is
already set, it will trigger an abort or an NMI is the Smrnmi extension
is available.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/cpu.c | 3 +++
target/riscv/cpu_helper.c | 35 ++++++++++++++++++++++++++---------
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4f52cf7ac0..dd3351832a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -946,6 +946,9 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
env->mstatus_hs = set_field(env->mstatus_hs,
MSTATUS64_UXL, env->misa_mxl);
}
+ if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+ env->mstatus = set_field(env->mstatus, MSTATUS_MDT, 1);
+ }
}
env->mcause = 0;
env->miclaim = MIP_SGEIP;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 69da3c3384..5a30d1b8a8 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1697,6 +1697,17 @@ static target_ulong riscv_transformed_insn(CPURISCVState *env,
return xinsn;
}
+static void riscv_do_nmi(CPURISCVState *env, target_ulong cause, bool virt)
+{
+ env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
+ env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV, virt);
+ env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP, env->priv);
+ env->mncause = cause;
+ env->mnepc = env->pc;
+ env->pc = env->rnmi_irqvec;
+ riscv_cpu_set_mode(env, PRV_M, virt);
+}
+
/*
* Handle Traps
*
@@ -1733,15 +1744,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
bool nmi_execp = false;
if (cpu->cfg.ext_smrnmi && env->rnmip && async) {
- env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
- env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV,
- env->virt_enabled);
- env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP,
- env->priv);
- env->mncause = cause | ((target_ulong)1U << (TARGET_LONG_BITS - 1));
- env->mnepc = env->pc;
- env->pc = env->rnmi_irqvec;
- riscv_cpu_set_mode(env, PRV_M, virt);
+ riscv_do_nmi(env, cause | ((target_ulong)1U << (TARGET_LONG_BITS - 1)),
+ virt);
return;
}
@@ -1937,6 +1941,19 @@ void riscv_cpu_do_interrupt(CPUState *cs)
s = set_field(s, MSTATUS_MPIE, get_field(s, MSTATUS_MIE));
s = set_field(s, MSTATUS_MPP, env->priv);
s = set_field(s, MSTATUS_MIE, 0);
+ if (cpu->cfg.ext_smdbltrp) {
+ if (env->mstatus & MSTATUS_MDT) {
+ assert(env->priv == PRV_M);
+ if (!cpu->cfg.ext_smrnmi || nmi_execp) {
+ cpu_abort(CPU(cpu), "M-mode double trap\n");
+ } else {
+ riscv_do_nmi(env, cause, false);
+ return;
+ }
+ }
+
+ s = set_field(s, MSTATUS_MDT, 1);
+ }
env->mstatus = s;
env->mcause = cause | ~(((target_ulong)-1) >> async);
if (smode_double_trap) {
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 8/8] target/riscv: Add Smdbltrp ISA extension enable switch
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
` (6 preceding siblings ...)
2024-09-25 11:58 ` [PATCH v2 7/8] target/riscv: Implement Smdbltrp behavior Clément Léger
@ 2024-09-25 11:58 ` Clément Léger
7 siblings, 0 replies; 26+ messages in thread
From: Clément Léger @ 2024-09-25 11:58 UTC (permalink / raw)
To: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: Clément Léger, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Ved Shanbhogue, Atish Patra, qemu-devel
Add the switch to enable the Smdbltrp ISA extension.
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
target/riscv/cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index dd3351832a..44da17bc9a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -183,6 +183,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
ISA_EXT_DATA_ENTRY(smcntrpmf, PRIV_VERSION_1_12_0, ext_smcntrpmf),
+ ISA_EXT_DATA_ENTRY(smdbltrp, PRIV_VERSION_1_12_0, ext_smdbltrp),
ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
ISA_EXT_DATA_ENTRY(smrnmi, PRIV_VERSION_1_12_0, ext_smrnmi),
ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
@@ -1492,6 +1493,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
+ MULTI_EXT_CFG_BOOL("smdbltrp", ext_smdbltrp, false),
MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
MULTI_EXT_CFG_BOOL("smrnmi", ext_smrnmi, false),
MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
--
2.45.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/8] target/riscv: Add Ssdbltrp CSRs handling
2024-09-25 11:57 ` [PATCH v2 1/8] target/riscv: Add Ssdbltrp CSRs handling Clément Léger
@ 2024-10-11 2:49 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2024-10-11 2:49 UTC (permalink / raw)
To: Clément Léger
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On Wed, Sep 25, 2024 at 10:02 PM Clément Léger <cleger@rivosinc.com> wrote:
>
> Add ext_ssdbltrp in RISCVCPUConfig and implement MSTATUS.SDT,
> {H|M}ENVCFG.DTE and modify the availability of MTVAL2 based on the
> presence of the Ssdbltrp ISA extension.
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 6 ++++++
> target/riscv/cpu_cfg.h | 1 +
> target/riscv/cpu_helper.c | 16 ++++++++++++++
> target/riscv/csr.c | 45 ++++++++++++++++++++++++++++++---------
> 5 files changed, 59 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a84e719d3f..ee984bf270 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -553,6 +553,7 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong geilen);
> bool riscv_cpu_vector_enabled(CPURISCVState *env);
> void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
> int riscv_env_mmu_index(CPURISCVState *env, bool ifetch);
> +bool riscv_env_smode_dbltrp_enabled(CPURISCVState *env, bool virt);
> G_NORETURN void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> MMUAccessType access_type,
> int mmu_idx, uintptr_t retaddr);
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index da1723496c..3a5588d4df 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -558,6 +558,7 @@
> #define MSTATUS_TVM 0x00100000 /* since: priv-1.10 */
> #define MSTATUS_TW 0x00200000 /* since: priv-1.10 */
> #define MSTATUS_TSR 0x00400000 /* since: priv-1.10 */
> +#define MSTATUS_SDT 0x01000000
> #define MSTATUS_GVA 0x4000000000ULL
> #define MSTATUS_MPV 0x8000000000ULL
>
> @@ -588,6 +589,7 @@ typedef enum {
> #define SSTATUS_XS 0x00018000
> #define SSTATUS_SUM 0x00040000 /* since: priv-1.10 */
> #define SSTATUS_MXR 0x00080000
> +#define SSTATUS_SDT 0x01000000
>
> #define SSTATUS64_UXL 0x0000000300000000ULL
>
> @@ -777,11 +779,13 @@ typedef enum RISCVException {
> #define MENVCFG_CBIE (3UL << 4)
> #define MENVCFG_CBCFE BIT(6)
> #define MENVCFG_CBZE BIT(7)
> +#define MENVCFG_DTE (1ULL << 59)
> #define MENVCFG_ADUE (1ULL << 61)
> #define MENVCFG_PBMTE (1ULL << 62)
> #define MENVCFG_STCE (1ULL << 63)
>
> /* For RV32 */
> +#define MENVCFGH_DTE BIT(27)
> #define MENVCFGH_ADUE BIT(29)
> #define MENVCFGH_PBMTE BIT(30)
> #define MENVCFGH_STCE BIT(31)
> @@ -795,11 +799,13 @@ typedef enum RISCVException {
> #define HENVCFG_CBIE MENVCFG_CBIE
> #define HENVCFG_CBCFE MENVCFG_CBCFE
> #define HENVCFG_CBZE MENVCFG_CBZE
> +#define HENVCFG_DTE MENVCFG_DTE
> #define HENVCFG_ADUE MENVCFG_ADUE
> #define HENVCFG_PBMTE MENVCFG_PBMTE
> #define HENVCFG_STCE MENVCFG_STCE
>
> /* For RV32 */
> +#define HENVCFGH_DTE MENVCFGH_DTE
> #define HENVCFGH_ADUE MENVCFGH_ADUE
> #define HENVCFGH_PBMTE MENVCFGH_PBMTE
> #define HENVCFGH_STCE MENVCFGH_STCE
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index ae2a945b5f..dd804f95d4 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -77,6 +77,7 @@ struct RISCVCPUConfig {
> bool ext_smstateen;
> bool ext_sstc;
> bool ext_smcntrpmf;
> + bool ext_ssdbltrp;
> bool ext_svadu;
> bool ext_svinval;
> bool ext_svnapot;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9d0400035f..395d8235ce 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -63,6 +63,22 @@ int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
> #endif
> }
>
> +bool riscv_env_smode_dbltrp_enabled(CPURISCVState *env, bool virt)
> +{
> +#ifdef CONFIG_USER_ONLY
> + return false;
> +#else
> + if (!riscv_cpu_cfg(env)->ext_ssdbltrp) {
> + return false;
> + }
> + if (virt) {
> + return (env->henvcfg & HENVCFG_DTE) != 0;
> + } else {
> + return (env->menvcfg & MENVCFG_DTE) != 0;
> + }
> +#endif
> +}
> +
> void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
> uint64_t *cs_base, uint32_t *pflags)
> {
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e5de72453c..d8280ec956 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -540,6 +540,15 @@ static RISCVException aia_hmode32(CPURISCVState *env, int csrno)
> return hmode32(env, csrno);
> }
>
> +static RISCVException dbltrp_hmode(CPURISCVState *env, int csrno)
> +{
> + if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
> + return RISCV_EXCP_NONE;
> + }
> +
> + return hmode(env, csrno);
> +}
> +
> static RISCVException pmp(CPURISCVState *env, int csrno)
> {
> if (riscv_cpu_cfg(env)->pmp) {
> @@ -1402,7 +1411,7 @@ static const target_ulong vs_delegable_excps = DELEGABLE_EXCPS &
> (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)));
> static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
> SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
> - SSTATUS_SUM | SSTATUS_MXR | SSTATUS_VS;
> + SSTATUS_SUM | SSTATUS_MXR | SSTATUS_VS | SSTATUS_SDT;
>
> /*
> * Spec allows for bits 13:63 to be either read-only or writable.
> @@ -1600,6 +1609,14 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
> mask |= MSTATUS_VS;
> }
>
> + if (riscv_env_smode_dbltrp_enabled(env, env->virt_enabled)) {
> + mask |= MSTATUS_SDT;
> + if ((val & MSTATUS_SDT) != 0) {
> + mstatus &= ~MSTATUS_SIE;
> + val &= ~MSTATUS_SIE;
> + }
> + }
> +
> if (xl != MXL_RV32 || env->debugger) {
> if (riscv_has_ext(env, RVH)) {
> mask |= MSTATUS_MPV | MSTATUS_GVA;
> @@ -2354,7 +2371,8 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> if (riscv_cpu_mxl(env) == MXL_RV64) {
> mask |= (cfg->ext_svpbmt ? MENVCFG_PBMTE : 0) |
> (cfg->ext_sstc ? MENVCFG_STCE : 0) |
> - (cfg->ext_svadu ? MENVCFG_ADUE : 0);
> + (cfg->ext_svadu ? MENVCFG_ADUE : 0) |
> + (cfg->ext_ssdbltrp ? MENVCFG_DTE : 0);
> }
> env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
>
> @@ -2374,7 +2392,8 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
> uint64_t mask = (cfg->ext_svpbmt ? MENVCFG_PBMTE : 0) |
> (cfg->ext_sstc ? MENVCFG_STCE : 0) |
> - (cfg->ext_svadu ? MENVCFG_ADUE : 0);
> + (cfg->ext_svadu ? MENVCFG_ADUE : 0) |
> + (cfg->ext_ssdbltrp ? MENVCFG_DTE : 0);
> uint64_t valh = (uint64_t)val << 32;
>
> env->menvcfg = (env->menvcfg & ~mask) | (valh & mask);
> @@ -2425,9 +2444,10 @@ static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> * henvcfg.pbmte is read_only 0 when menvcfg.pbmte = 0
> * henvcfg.stce is read_only 0 when menvcfg.stce = 0
> * henvcfg.adue is read_only 0 when menvcfg.adue = 0
> + * henvcfg.dte is read_only 0 when menvcfg.dte = 0
> */
> - *val = env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE) |
> - env->menvcfg);
> + *val = env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE |
> + HENVCFG_DTE) | env->menvcfg);
> return RISCV_EXCP_NONE;
> }
>
> @@ -2435,6 +2455,7 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
> + uint64_t menvcfg_mask;
> RISCVException ret;
>
> ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> @@ -2443,7 +2464,11 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> }
>
> if (riscv_cpu_mxl(env) == MXL_RV64) {
> - mask |= env->menvcfg & (HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE);
> + menvcfg_mask = HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE;
> + if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
> + menvcfg_mask |= HENVCFG_DTE;
> + }
> + mask |= env->menvcfg & menvcfg_mask;
> }
>
> env->henvcfg = (env->henvcfg & ~mask) | (val & mask);
> @@ -2461,8 +2486,8 @@ static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> return ret;
> }
>
> - *val = (env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE) |
> - env->menvcfg)) >> 32;
> + *val = (env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE |
> + HENVCFG_DTE) | env->menvcfg)) >> 32;
> return RISCV_EXCP_NONE;
> }
>
> @@ -2470,7 +2495,7 @@ static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> uint64_t mask = env->menvcfg & (HENVCFG_PBMTE | HENVCFG_STCE |
> - HENVCFG_ADUE);
> + HENVCFG_ADUE | HENVCFG_DTE);
> uint64_t valh = (uint64_t)val << 32;
> RISCVException ret;
>
> @@ -5246,7 +5271,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_VSATP] = { "vsatp", hmode, read_vsatp, write_vsatp,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
>
> - [CSR_MTVAL2] = { "mtval2", hmode, read_mtval2, write_mtval2,
> + [CSR_MTVAL2] = { "mtval2", dbltrp_hmode, read_mtval2, write_mtval2,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> [CSR_MTINST] = { "mtinst", hmode, read_mtinst, write_mtinst,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
2024-09-25 11:58 ` [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior Clément Léger
@ 2024-10-11 3:13 ` Alistair Francis
2024-10-11 18:52 ` Ved Shanbhogue
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-11 3:13 UTC (permalink / raw)
To: Clément Léger
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On Wed, Sep 25, 2024 at 9:58 PM Clément Léger <cleger@rivosinc.com> wrote:
>
> When the Ssdbltrp extension is enabled, SSTATUS.SDT field is cleared
> when executing sret. When executing mret/mnret, SSTATUS.SDT is cleared
> when returning to U, VS or VU and VSSTATUS.SDT is cleared when returning
> to VU from HS.
I don't see mret being mentioned in the spec. Where do you see that
V/SSTATUS.SDT should be cleared?
Alistair
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
> target/riscv/op_helper.c | 35 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 6895c7596b..00b6f75102 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -287,6 +287,18 @@ target_ulong helper_sret(CPURISCVState *env)
> get_field(mstatus, MSTATUS_SPIE));
> mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> +
> + if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
> + if (riscv_has_ext(env, RVH)) {
> + target_ulong prev_vu = get_field(env->hstatus, HSTATUS_SPV) &&
> + prev_priv == PRV_U;
> + /* Returning to VU from HS, vsstatus.sdt = 0 */
> + if (!env->virt_enabled && prev_vu) {
> + env->vsstatus = set_field(env->vsstatus, MSTATUS_SDT, 0);
> + }
> + }
> + mstatus = set_field(mstatus, MSTATUS_SDT, 0);
> + }
> if (env->priv_ver >= PRIV_VERSION_1_12_0) {
> mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
> }
> @@ -297,7 +309,6 @@ target_ulong helper_sret(CPURISCVState *env)
> target_ulong hstatus = env->hstatus;
>
> prev_virt = get_field(hstatus, HSTATUS_SPV);
> -
> hstatus = set_field(hstatus, HSTATUS_SPV, 0);
>
> env->hstatus = hstatus;
> @@ -328,6 +339,22 @@ static void check_ret_from_m_mode(CPURISCVState *env, target_ulong retpc,
> riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, GETPC());
> }
> }
> +static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,
> + target_ulong prev_priv,
> + target_ulong prev_virt)
> +{
> + /* If returning to U, VS or VU, sstatus.sdt = 0 */
> + if (prev_priv == PRV_U || (prev_virt &&
> + (prev_priv == PRV_S || prev_priv == PRV_U))) {
> + mstatus = set_field(mstatus, MSTATUS_SDT, 0);
> + /* If returning to VU, vsstatus.sdt = 0 */
> + if (prev_virt && prev_priv == PRV_U) {
> + env->vsstatus = set_field(env->vsstatus, MSTATUS_SDT, 0);
> + }
> + }
> +
> + return mstatus;
> +}
>
> target_ulong helper_mret(CPURISCVState *env)
> {
> @@ -345,6 +372,9 @@ target_ulong helper_mret(CPURISCVState *env)
> mstatus = set_field(mstatus, MSTATUS_MPP,
> riscv_has_ext(env, RVU) ? PRV_U : PRV_M);
> mstatus = set_field(mstatus, MSTATUS_MPV, 0);
> + if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
> + mstatus = ssdbltrp_mxret(env, mstatus, prev_priv, prev_virt);
> + }
> if ((env->priv_ver >= PRIV_VERSION_1_12_0) && (prev_priv != PRV_M)) {
> mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
> }
> @@ -382,6 +412,9 @@ target_ulong helper_mnret(CPURISCVState *env)
> if (prev_priv < PRV_M) {
> env->mstatus = set_field(env->mstatus, MSTATUS_MPRV, false);
> }
> + if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
> + env->mstatus = ssdbltrp_mxret(env, env->mstatus, prev_priv, prev_virt);
> + }
>
> if (riscv_has_ext(env, RVH) && prev_virt) {
> riscv_cpu_swap_hypervisor_regs(env);
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling
2024-09-25 11:58 ` [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling Clément Léger
@ 2024-10-11 3:22 ` Alistair Francis
2024-10-14 7:43 ` Clément Léger
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-11 3:22 UTC (permalink / raw)
To: Clément Léger
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
>
> When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
> while SSTATUS.SDT isn't cleared, generate a double trap exception to
> M-mode.
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
> target/riscv/cpu.c | 2 +-
> target/riscv/cpu_bits.h | 1 +
> target/riscv/cpu_helper.c | 47 ++++++++++++++++++++++++++++++++++-----
> 3 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index cf06cd741a..65347ccd5a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
> "load_page_fault",
> "reserved",
> "store_page_fault",
> - "reserved",
> + "double_trap",
> "reserved",
> "reserved",
> "reserved",
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 3a5588d4df..5557a86348 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -699,6 +699,7 @@ typedef enum RISCVException {
> RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
> RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
> RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
> + RISCV_EXCP_DOUBLE_TRAP = 0x10,
> RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
> RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
> RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 395d8235ce..69da3c3384 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
> mstatus_mask |= MSTATUS_FS;
> }
> bool current_virt = env->virt_enabled;
> -
> + if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
> + mstatus_mask |= MSTATUS_SDT;
> + }
> g_assert(riscv_has_ext(env, RVH));
>
> if (current_virt) {
> @@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> CPURISCVState *env = &cpu->env;
> bool virt = env->virt_enabled;
> bool write_gva = false;
> + bool vsmode_exc;
> uint64_t s;
> int mode;
>
> @@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> !(env->mip & (1 << cause));
> bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
> !(env->mip & (1 << cause));
> + bool smode_double_trap = false;
> + uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
> target_ulong tval = 0;
> target_ulong tinst = 0;
> target_ulong htval = 0;
> @@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> !async &&
> mode == PRV_M;
>
> + vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
> + /*
> + * Check double trap condition only if already in S-mode and targeting
> + * S-mode
> + */
> + if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
> + bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
> + bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
> + /* In VS or HS */
> + if (riscv_has_ext(env, RVH)) {
> + if (vsmode_exc) {
> + /* VS -> VS */
> + /* Stay in VS mode, use henvcfg instead of menvcfg*/
> + dte = (env->henvcfg & HENVCFG_DTE) != 0;
> + } else if (env->virt_enabled) {
> + /* VS -> HS */
> + dte = false;
I don't follow why this is false
Alistair
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch
2024-09-25 11:58 ` [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch Clément Léger
@ 2024-10-11 3:24 ` Alistair Francis
2024-10-14 7:43 ` Clément Léger
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-11 3:24 UTC (permalink / raw)
To: Clément Léger
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
>
> Add the switch to enable the Ssdbltrp ISA extension.
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
> target/riscv/cpu.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 65347ccd5a..4f52cf7ac0 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -190,6 +190,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, has_priv_1_11),
> ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
> ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, has_priv_1_12),
> + ISA_EXT_DATA_ENTRY(ssdbltrp, PRIV_VERSION_1_12_0, ext_ssdbltrp),
Shouldn't this be PRIV_VERSION_1_13_0?
Alistair
> ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
> ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
> ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
> @@ -1492,6 +1493,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> MULTI_EXT_CFG_BOOL("smrnmi", ext_smrnmi, false),
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> MULTI_EXT_CFG_BOOL("ssaia", ext_ssaia, false),
> + MULTI_EXT_CFG_BOOL("ssdbltrp", ext_ssdbltrp, false),
> MULTI_EXT_CFG_BOOL("svade", ext_svade, false),
> MULTI_EXT_CFG_BOOL("svadu", ext_svadu, true),
> MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/8] target/riscv: Add Smdbltrp CSRs handling
2024-09-25 11:58 ` [PATCH v2 5/8] target/riscv: Add Smdbltrp CSRs handling Clément Léger
@ 2024-10-11 3:30 ` Alistair Francis
2024-10-11 7:40 ` Clément Léger
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-11 3:30 UTC (permalink / raw)
To: Clément Léger
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On Wed, Sep 25, 2024 at 10:02 PM Clément Léger <cleger@rivosinc.com> wrote:
>
> Add `ext_smdbltrp`in RISCVCPUConfig and implement MSTATUS.MDT behavior.
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
> target/riscv/cpu_bits.h | 1 +
> target/riscv/cpu_cfg.h | 1 +
> target/riscv/csr.c | 15 +++++++++++++++
> 3 files changed, 17 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 5557a86348..62bab1bf55 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -561,6 +561,7 @@
> #define MSTATUS_SDT 0x01000000
> #define MSTATUS_GVA 0x4000000000ULL
> #define MSTATUS_MPV 0x8000000000ULL
> +#define MSTATUS_MDT 0x40000000000ULL /* Smdbltrp extension */
>
> #define MSTATUS64_UXL 0x0000000300000000ULL
> #define MSTATUS64_SXL 0x0000000C00000000ULL
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index dd804f95d4..4c4caa2b39 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -78,6 +78,7 @@ struct RISCVCPUConfig {
> bool ext_sstc;
> bool ext_smcntrpmf;
> bool ext_ssdbltrp;
> + bool ext_smdbltrp;
> bool ext_svadu;
> bool ext_svinval;
> bool ext_svnapot;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d8280ec956..cc1940447a 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1617,6 +1617,14 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
> }
> }
>
> + if (riscv_cpu_cfg(env)->ext_smdbltrp) {
> + mask |= MSTATUS_MDT;
> + if ((val & MSTATUS_MDT) != 0) {
> + mstatus &= ~MSTATUS_MIE;
> + val &= ~MSTATUS_MIE;
> + }
> + }
This should also be set to 1 on reset
Alistair
> +
> if (xl != MXL_RV32 || env->debugger) {
> if (riscv_has_ext(env, RVH)) {
> mask |= MSTATUS_MPV | MSTATUS_GVA;
> @@ -1655,6 +1663,13 @@ static RISCVException write_mstatush(CPURISCVState *env, int csrno,
> uint64_t valh = (uint64_t)val << 32;
> uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0;
>
> + if (riscv_cpu_cfg(env)->ext_smdbltrp) {
> + mask |= MSTATUS_MDT;
> + if ((val & MSTATUS_MDT) != 0) {
> + env->mstatus &= ~MSTATUS_MIE;
> + val &= ~MSTATUS_MIE;
> + }
> + }
> env->mstatus = (env->mstatus & ~mask) | (valh & mask);
>
> return RISCV_EXCP_NONE;
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/8] target/riscv: Add Smdbltrp CSRs handling
2024-10-11 3:30 ` Alistair Francis
@ 2024-10-11 7:40 ` Clément Léger
0 siblings, 0 replies; 26+ messages in thread
From: Clément Léger @ 2024-10-11 7:40 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On 11/10/2024 05:30, Alistair Francis wrote:
> On Wed, Sep 25, 2024 at 10:02 PM Clément Léger <cleger@rivosinc.com> wrote:
>>
>> Add `ext_smdbltrp`in RISCVCPUConfig and implement MSTATUS.MDT behavior.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>> target/riscv/cpu_bits.h | 1 +
>> target/riscv/cpu_cfg.h | 1 +
>> target/riscv/csr.c | 15 +++++++++++++++
>> 3 files changed, 17 insertions(+)
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 5557a86348..62bab1bf55 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -561,6 +561,7 @@
>> #define MSTATUS_SDT 0x01000000
>> #define MSTATUS_GVA 0x4000000000ULL
>> #define MSTATUS_MPV 0x8000000000ULL
>> +#define MSTATUS_MDT 0x40000000000ULL /* Smdbltrp extension */
>>
>> #define MSTATUS64_UXL 0x0000000300000000ULL
>> #define MSTATUS64_SXL 0x0000000C00000000ULL
>> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
>> index dd804f95d4..4c4caa2b39 100644
>> --- a/target/riscv/cpu_cfg.h
>> +++ b/target/riscv/cpu_cfg.h
>> @@ -78,6 +78,7 @@ struct RISCVCPUConfig {
>> bool ext_sstc;
>> bool ext_smcntrpmf;
>> bool ext_ssdbltrp;
>> + bool ext_smdbltrp;
>> bool ext_svadu;
>> bool ext_svinval;
>> bool ext_svnapot;
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index d8280ec956..cc1940447a 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -1617,6 +1617,14 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>> }
>> }
>>
>> + if (riscv_cpu_cfg(env)->ext_smdbltrp) {
>> + mask |= MSTATUS_MDT;
>> + if ((val & MSTATUS_MDT) != 0) {
>> + mstatus &= ~MSTATUS_MIE;
>> + val &= ~MSTATUS_MIE;
>> + }
>> + }
>
> This should also be set to 1 on reset
Yes, this is actually done in patch 7/8. I'll squash this change in this
commit
>
> Alistair
>
>> +
>> if (xl != MXL_RV32 || env->debugger) {
>> if (riscv_has_ext(env, RVH)) {
>> mask |= MSTATUS_MPV | MSTATUS_GVA;
>> @@ -1655,6 +1663,13 @@ static RISCVException write_mstatush(CPURISCVState *env, int csrno,
>> uint64_t valh = (uint64_t)val << 32;
>> uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0;
>>
>> + if (riscv_cpu_cfg(env)->ext_smdbltrp) {
>> + mask |= MSTATUS_MDT;
>> + if ((val & MSTATUS_MDT) != 0) {
>> + env->mstatus &= ~MSTATUS_MIE;
>> + val &= ~MSTATUS_MIE;
>> + }
>> + }
>> env->mstatus = (env->mstatus & ~mask) | (valh & mask);
>>
>> return RISCV_EXCP_NONE;
>> --
>> 2.45.2
>>
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
2024-10-11 3:13 ` Alistair Francis
@ 2024-10-11 18:52 ` Ved Shanbhogue
2024-10-17 4:36 ` Alistair Francis
0 siblings, 1 reply; 26+ messages in thread
From: Ved Shanbhogue @ 2024-10-11 18:52 UTC (permalink / raw)
To: Alistair Francis
Cc: Clément Léger, qemu-riscv, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Atish Patra, qemu-devel
Alistair Francis wrote:
>> When the Ssdbltrp extension is enabled, SSTATUS.SDT field is cleared
>> when executing sret. When executing mret/mnret, SSTATUS.SDT is cleared
>> when returning to U, VS or VU and VSSTATUS.SDT is cleared when returning
>> to VU from HS.
>
>I don't see mret being mentioned in the spec. Where do you see that
>V/SSTATUS.SDT should be cleared?
>
Ssdbltrp specifies:
In S-mode, the SRET instruction sets sstatus.SDT to 0,
and if the new privilege mode is VU, it also sets
vsstatus.SDT to 0. However, in VS-mode, only vsstatus.SDT
is set to to 0.
The MRET instructions sets sstatus.SDT to 0, if the new
privilege mode is U, VS, or VU. Additionally, if it is
VU, then vsstatus.SDT is also set to 0.
Smdbltrp specifies:
The MRET and SRET instructions, when executed in M-mode,
set the MDT bit to 0. If the new privilege mode is U, VS,
or VU, then sstatus.SDT is also set to 0. Additionally,
if it is VU, then vsstatus.SDT is also set to 0.
The MNRET instruction sets the MDT bit to 0 if the new
privilege mode is not M. If it is U, VS, or VU, then
sstatus.SDT is also set to 0. Additionally, if it is VU,
then vsstatus.SDT is also set to 0.
regards
ved
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling
2024-10-11 3:22 ` Alistair Francis
@ 2024-10-14 7:43 ` Clément Léger
2024-10-17 4:29 ` Alistair Francis
0 siblings, 1 reply; 26+ messages in thread
From: Clément Léger @ 2024-10-14 7:43 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On 11/10/2024 05:22, Alistair Francis wrote:
> On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
>>
>> When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
>> while SSTATUS.SDT isn't cleared, generate a double trap exception to
>> M-mode.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>> target/riscv/cpu.c | 2 +-
>> target/riscv/cpu_bits.h | 1 +
>> target/riscv/cpu_helper.c | 47 ++++++++++++++++++++++++++++++++++-----
>> 3 files changed, 43 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index cf06cd741a..65347ccd5a 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
>> "load_page_fault",
>> "reserved",
>> "store_page_fault",
>> - "reserved",
>> + "double_trap",
>> "reserved",
>> "reserved",
>> "reserved",
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 3a5588d4df..5557a86348 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -699,6 +699,7 @@ typedef enum RISCVException {
>> RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
>> RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
>> RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
>> + RISCV_EXCP_DOUBLE_TRAP = 0x10,
>> RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
>> RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
>> RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 395d8235ce..69da3c3384 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>> mstatus_mask |= MSTATUS_FS;
>> }
>> bool current_virt = env->virt_enabled;
>> -
>> + if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
>> + mstatus_mask |= MSTATUS_SDT;
>> + }
>> g_assert(riscv_has_ext(env, RVH));
>>
>> if (current_virt) {
>> @@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> CPURISCVState *env = &cpu->env;
>> bool virt = env->virt_enabled;
>> bool write_gva = false;
>> + bool vsmode_exc;
>> uint64_t s;
>> int mode;
>>
>> @@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> !(env->mip & (1 << cause));
>> bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
>> !(env->mip & (1 << cause));
>> + bool smode_double_trap = false;
>> + uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
>> target_ulong tval = 0;
>> target_ulong tinst = 0;
>> target_ulong htval = 0;
>> @@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> !async &&
>> mode == PRV_M;
>>
>> + vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
>> + /*
>> + * Check double trap condition only if already in S-mode and targeting
>> + * S-mode
>> + */
>> + if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
>> + bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
>> + bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
>> + /* In VS or HS */
>> + if (riscv_has_ext(env, RVH)) {
>> + if (vsmode_exc) {
>> + /* VS -> VS */
>> + /* Stay in VS mode, use henvcfg instead of menvcfg*/
>> + dte = (env->henvcfg & HENVCFG_DTE) != 0;
>> + } else if (env->virt_enabled) {
>> + /* VS -> HS */
>> + dte = false;
>
> I don't follow why this is false
Hi Alistair,
It's indeed probably lacking some comments here. The rationale is that
if you are trapping from VS to HS, then at some point, you returned to
VS using a sret/mret and thus cleared DTE, so rather than checking the
value of mstatus_hs, just assume it is false.
Thanks,
Clément
>
> Alistair
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch
2024-10-11 3:24 ` Alistair Francis
@ 2024-10-14 7:43 ` Clément Léger
0 siblings, 0 replies; 26+ messages in thread
From: Clément Léger @ 2024-10-14 7:43 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On 11/10/2024 05:24, Alistair Francis wrote:
> On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
>>
>> Add the switch to enable the Ssdbltrp ISA extension.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>> target/riscv/cpu.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 65347ccd5a..4f52cf7ac0 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -190,6 +190,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>> ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, has_priv_1_11),
>> ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
>> ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, has_priv_1_12),
>> + ISA_EXT_DATA_ENTRY(ssdbltrp, PRIV_VERSION_1_12_0, ext_ssdbltrp),
>
> Shouldn't this be PRIV_VERSION_1_13_0?
Oups, yes indeed.
Thanks,
Clément
>
> Alistair
>
>> ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
>> ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
>> ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
>> @@ -1492,6 +1493,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>> MULTI_EXT_CFG_BOOL("smrnmi", ext_smrnmi, false),
>> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
>> MULTI_EXT_CFG_BOOL("ssaia", ext_ssaia, false),
>> + MULTI_EXT_CFG_BOOL("ssdbltrp", ext_ssdbltrp, false),
>> MULTI_EXT_CFG_BOOL("svade", ext_svade, false),
>> MULTI_EXT_CFG_BOOL("svadu", ext_svadu, true),
>> MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
>> --
>> 2.45.2
>>
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling
2024-10-14 7:43 ` Clément Léger
@ 2024-10-17 4:29 ` Alistair Francis
2024-10-17 7:45 ` Clément Léger
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-17 4:29 UTC (permalink / raw)
To: Clément Léger
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On Mon, Oct 14, 2024 at 5:43 PM Clément Léger <cleger@rivosinc.com> wrote:
>
>
>
> On 11/10/2024 05:22, Alistair Francis wrote:
> > On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
> >>
> >> When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
> >> while SSTATUS.SDT isn't cleared, generate a double trap exception to
> >> M-mode.
> >>
> >> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> >> ---
> >> target/riscv/cpu.c | 2 +-
> >> target/riscv/cpu_bits.h | 1 +
> >> target/riscv/cpu_helper.c | 47 ++++++++++++++++++++++++++++++++++-----
> >> 3 files changed, 43 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index cf06cd741a..65347ccd5a 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
> >> "load_page_fault",
> >> "reserved",
> >> "store_page_fault",
> >> - "reserved",
> >> + "double_trap",
> >> "reserved",
> >> "reserved",
> >> "reserved",
> >> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> >> index 3a5588d4df..5557a86348 100644
> >> --- a/target/riscv/cpu_bits.h
> >> +++ b/target/riscv/cpu_bits.h
> >> @@ -699,6 +699,7 @@ typedef enum RISCVException {
> >> RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
> >> RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
> >> RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
> >> + RISCV_EXCP_DOUBLE_TRAP = 0x10,
> >> RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
> >> RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
> >> RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
> >> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >> index 395d8235ce..69da3c3384 100644
> >> --- a/target/riscv/cpu_helper.c
> >> +++ b/target/riscv/cpu_helper.c
> >> @@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
> >> mstatus_mask |= MSTATUS_FS;
> >> }
> >> bool current_virt = env->virt_enabled;
> >> -
> >> + if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
> >> + mstatus_mask |= MSTATUS_SDT;
> >> + }
> >> g_assert(riscv_has_ext(env, RVH));
> >>
> >> if (current_virt) {
> >> @@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >> CPURISCVState *env = &cpu->env;
> >> bool virt = env->virt_enabled;
> >> bool write_gva = false;
> >> + bool vsmode_exc;
> >> uint64_t s;
> >> int mode;
> >>
> >> @@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >> !(env->mip & (1 << cause));
> >> bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
> >> !(env->mip & (1 << cause));
> >> + bool smode_double_trap = false;
> >> + uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
> >> target_ulong tval = 0;
> >> target_ulong tinst = 0;
> >> target_ulong htval = 0;
> >> @@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >> !async &&
> >> mode == PRV_M;
> >>
> >> + vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
> >> + /*
> >> + * Check double trap condition only if already in S-mode and targeting
> >> + * S-mode
> >> + */
> >> + if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
> >> + bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
> >> + bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
> >> + /* In VS or HS */
> >> + if (riscv_has_ext(env, RVH)) {
> >> + if (vsmode_exc) {
> >> + /* VS -> VS */
> >> + /* Stay in VS mode, use henvcfg instead of menvcfg*/
> >> + dte = (env->henvcfg & HENVCFG_DTE) != 0;
> >> + } else if (env->virt_enabled) {
> >> + /* VS -> HS */
> >> + dte = false;
> >
> > I don't follow why this is false
>
> Hi Alistair,
>
> It's indeed probably lacking some comments here. The rationale is that
> if you are trapping from VS to HS, then at some point, you returned to
> VS using a sret/mret and thus cleared DTE, so rather than checking the
Why not just clear it at sret/mret? Instead of having this assumption
Alistair
> value of mstatus_hs, just assume it is false.
>
> Thanks,
>
> Clément
>
> >
> > Alistair
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
2024-10-11 18:52 ` Ved Shanbhogue
@ 2024-10-17 4:36 ` Alistair Francis
2024-10-17 18:27 ` Ved Shanbhogue
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-17 4:36 UTC (permalink / raw)
To: Ved Shanbhogue
Cc: Clément Léger, qemu-riscv, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Atish Patra, qemu-devel
On Sat, Oct 12, 2024 at 4:52 AM Ved Shanbhogue <ved@rivosinc.com> wrote:
>
> Alistair Francis wrote:
> >> When the Ssdbltrp extension is enabled, SSTATUS.SDT field is cleared
> >> when executing sret. When executing mret/mnret, SSTATUS.SDT is cleared
> >> when returning to U, VS or VU and VSSTATUS.SDT is cleared when returning
> >> to VU from HS.
> >
> >I don't see mret being mentioned in the spec. Where do you see that
> >V/SSTATUS.SDT should be cleared?
> >
>
> Ssdbltrp specifies:
> In S-mode, the SRET instruction sets sstatus.SDT to 0,
> and if the new privilege mode is VU, it also sets
> vsstatus.SDT to 0. However, in VS-mode, only vsstatus.SDT
> is set to to 0.
I cannot find this
$ git show --shortstat
commit ef2ec9dc9afd003d0dab6d5ca36db59864c8483c (HEAD -> main, tag:
riscv-isa-release-ef2ec9d-2024-10-16, origin/main)
Author: Andrew Waterman <andrew@sifive.com>
Date: Wed Oct 16 12:09:41 2024 -0700
Remove future tense from description of now-ratified text (#1685)
2 files changed, 8 insertions(+), 15 deletions(-)
$ grep -r sstatus.SDT | grep SRET
src/hypervisor.adoc:if the new privilege mode is VU, the `SRET`
instruction sets `vsstatus.SDT`
What am I missing here?
Alistair
>
> The MRET instructions sets sstatus.SDT to 0, if the new
> privilege mode is U, VS, or VU. Additionally, if it is
> VU, then vsstatus.SDT is also set to 0.
>
> Smdbltrp specifies:
> The MRET and SRET instructions, when executed in M-mode,
> set the MDT bit to 0. If the new privilege mode is U, VS,
> or VU, then sstatus.SDT is also set to 0. Additionally,
> if it is VU, then vsstatus.SDT is also set to 0.
>
> The MNRET instruction sets the MDT bit to 0 if the new
> privilege mode is not M. If it is U, VS, or VU, then
> sstatus.SDT is also set to 0. Additionally, if it is VU,
> then vsstatus.SDT is also set to 0.
>
> regards
> ved
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling
2024-10-17 4:29 ` Alistair Francis
@ 2024-10-17 7:45 ` Clément Léger
2024-10-18 2:25 ` Alistair Francis
0 siblings, 1 reply; 26+ messages in thread
From: Clément Léger @ 2024-10-17 7:45 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On 17/10/2024 06:29, Alistair Francis wrote:
> On Mon, Oct 14, 2024 at 5:43 PM Clément Léger <cleger@rivosinc.com> wrote:
>>
>>
>>
>> On 11/10/2024 05:22, Alistair Francis wrote:
>>> On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
>>>>
>>>> When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
>>>> while SSTATUS.SDT isn't cleared, generate a double trap exception to
>>>> M-mode.
>>>>
>>>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>>>> ---
>>>> target/riscv/cpu.c | 2 +-
>>>> target/riscv/cpu_bits.h | 1 +
>>>> target/riscv/cpu_helper.c | 47 ++++++++++++++++++++++++++++++++++-----
>>>> 3 files changed, 43 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>> index cf06cd741a..65347ccd5a 100644
>>>> --- a/target/riscv/cpu.c
>>>> +++ b/target/riscv/cpu.c
>>>> @@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
>>>> "load_page_fault",
>>>> "reserved",
>>>> "store_page_fault",
>>>> - "reserved",
>>>> + "double_trap",
>>>> "reserved",
>>>> "reserved",
>>>> "reserved",
>>>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>>>> index 3a5588d4df..5557a86348 100644
>>>> --- a/target/riscv/cpu_bits.h
>>>> +++ b/target/riscv/cpu_bits.h
>>>> @@ -699,6 +699,7 @@ typedef enum RISCVException {
>>>> RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
>>>> RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
>>>> RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
>>>> + RISCV_EXCP_DOUBLE_TRAP = 0x10,
>>>> RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
>>>> RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
>>>> RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>> index 395d8235ce..69da3c3384 100644
>>>> --- a/target/riscv/cpu_helper.c
>>>> +++ b/target/riscv/cpu_helper.c
>>>> @@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>>>> mstatus_mask |= MSTATUS_FS;
>>>> }
>>>> bool current_virt = env->virt_enabled;
>>>> -
>>>> + if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
>>>> + mstatus_mask |= MSTATUS_SDT;
>>>> + }
>>>> g_assert(riscv_has_ext(env, RVH));
>>>>
>>>> if (current_virt) {
>>>> @@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>>> CPURISCVState *env = &cpu->env;
>>>> bool virt = env->virt_enabled;
>>>> bool write_gva = false;
>>>> + bool vsmode_exc;
>>>> uint64_t s;
>>>> int mode;
>>>>
>>>> @@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>>> !(env->mip & (1 << cause));
>>>> bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
>>>> !(env->mip & (1 << cause));
>>>> + bool smode_double_trap = false;
>>>> + uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
>>>> target_ulong tval = 0;
>>>> target_ulong tinst = 0;
>>>> target_ulong htval = 0;
>>>> @@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>>> !async &&
>>>> mode == PRV_M;
>>>>
>>>> + vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
>>>> + /*
>>>> + * Check double trap condition only if already in S-mode and targeting
>>>> + * S-mode
>>>> + */
>>>> + if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
>>>> + bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
>>>> + bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
>>>> + /* In VS or HS */
>>>> + if (riscv_has_ext(env, RVH)) {
>>>> + if (vsmode_exc) {
>>>> + /* VS -> VS */
>>>> + /* Stay in VS mode, use henvcfg instead of menvcfg*/
>>>> + dte = (env->henvcfg & HENVCFG_DTE) != 0;
>>>> + } else if (env->virt_enabled) {
>>>> + /* VS -> HS */
>>>> + dte = false;
>>>
>>> I don't follow why this is false
>>
>> Hi Alistair,
>>
>> It's indeed probably lacking some comments here. The rationale is that
>> if you are trapping from VS to HS, then at some point, you returned to
>> VS using a sret/mret and thus cleared DTE, so rather than checking the
s/DTE/SDT
>
> Why not just clear it at sret/mret? Instead of having this assumption
It has been cleared but since registers were swapped to enter virt mode,
hypervisor SDT value is stored in mstatus_hs rather than mstatus. So I
could have wrote it this way:
+ } else if (env->virt_enabled) {
+ /* VS -> HS */
+ sdt = (env->mstatus_hs & MSTATUS_SDT);
Since this is always 0 better assume it is 0 (but should be sdt = 0
instead of dte = 0). But if you prefer using mstatus_hs for clarity, I
can use that of course.
Clément
>
> Alistair
>
>> value of mstatus_hs, just assume it is false.
>>
>> Thanks,
>>
>> Clément
>>
>>>
>>> Alistair
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
2024-10-17 4:36 ` Alistair Francis
@ 2024-10-17 18:27 ` Ved Shanbhogue
2024-10-18 2:21 ` Alistair Francis
0 siblings, 1 reply; 26+ messages in thread
From: Ved Shanbhogue @ 2024-10-17 18:27 UTC (permalink / raw)
To: Alistair Francis
Cc: Clément Léger, qemu-riscv, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Atish Patra, qemu-devel
Alistair Francis wrote:
>$ grep -r sstatus.SDT | grep SRET
>src/hypervisor.adoc:if the new privilege mode is VU, the `SRET`
>instruction sets `vsstatus.SDT`
>
>What am I missing here?
https://github.com/riscv/riscv-isa-manual/blob/ef2ec9dc9afd003d0dab6d5ca36db59864c8483c/src/machine.adoc?plain=1#L538
regards
ved
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
2024-10-17 18:27 ` Ved Shanbhogue
@ 2024-10-18 2:21 ` Alistair Francis
2024-10-18 7:02 ` Clément Léger
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-18 2:21 UTC (permalink / raw)
To: Ved Shanbhogue
Cc: Clément Léger, qemu-riscv, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Atish Patra, qemu-devel
On Fri, Oct 18, 2024 at 4:27 AM Ved Shanbhogue <ved@rivosinc.com> wrote:
>
> Alistair Francis wrote:
> >$ grep -r sstatus.SDT | grep SRET
> >src/hypervisor.adoc:if the new privilege mode is VU, the `SRET`
> >instruction sets `vsstatus.SDT`
> >
> >What am I missing here?
>
> https://github.com/riscv/riscv-isa-manual/blob/ef2ec9dc9afd003d0dab6d5ca36db59864c8483c/src/machine.adoc?plain=1#L538
Ah, I thought you were quoting the spec directly.
Makes sense. This patch misses the MDT bit clearing though. I'm
guessing that's implemented in a different patch, but it should be
pulled in here instead
Alistair
>
>
> regards
> ved
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling
2024-10-17 7:45 ` Clément Léger
@ 2024-10-18 2:25 ` Alistair Francis
2024-10-18 7:04 ` Clément Léger
0 siblings, 1 reply; 26+ messages in thread
From: Alistair Francis @ 2024-10-18 2:25 UTC (permalink / raw)
To: Clément Léger
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On Thu, Oct 17, 2024 at 5:45 PM Clément Léger <cleger@rivosinc.com> wrote:
>
>
>
> On 17/10/2024 06:29, Alistair Francis wrote:
> > On Mon, Oct 14, 2024 at 5:43 PM Clément Léger <cleger@rivosinc.com> wrote:
> >>
> >>
> >>
> >> On 11/10/2024 05:22, Alistair Francis wrote:
> >>> On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
> >>>>
> >>>> When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
> >>>> while SSTATUS.SDT isn't cleared, generate a double trap exception to
> >>>> M-mode.
> >>>>
> >>>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> >>>> ---
> >>>> target/riscv/cpu.c | 2 +-
> >>>> target/riscv/cpu_bits.h | 1 +
> >>>> target/riscv/cpu_helper.c | 47 ++++++++++++++++++++++++++++++++++-----
> >>>> 3 files changed, 43 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >>>> index cf06cd741a..65347ccd5a 100644
> >>>> --- a/target/riscv/cpu.c
> >>>> +++ b/target/riscv/cpu.c
> >>>> @@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
> >>>> "load_page_fault",
> >>>> "reserved",
> >>>> "store_page_fault",
> >>>> - "reserved",
> >>>> + "double_trap",
> >>>> "reserved",
> >>>> "reserved",
> >>>> "reserved",
> >>>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> >>>> index 3a5588d4df..5557a86348 100644
> >>>> --- a/target/riscv/cpu_bits.h
> >>>> +++ b/target/riscv/cpu_bits.h
> >>>> @@ -699,6 +699,7 @@ typedef enum RISCVException {
> >>>> RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
> >>>> RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
> >>>> RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
> >>>> + RISCV_EXCP_DOUBLE_TRAP = 0x10,
> >>>> RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
> >>>> RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
> >>>> RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
> >>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >>>> index 395d8235ce..69da3c3384 100644
> >>>> --- a/target/riscv/cpu_helper.c
> >>>> +++ b/target/riscv/cpu_helper.c
> >>>> @@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
> >>>> mstatus_mask |= MSTATUS_FS;
> >>>> }
> >>>> bool current_virt = env->virt_enabled;
> >>>> -
> >>>> + if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
> >>>> + mstatus_mask |= MSTATUS_SDT;
> >>>> + }
> >>>> g_assert(riscv_has_ext(env, RVH));
> >>>>
> >>>> if (current_virt) {
> >>>> @@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >>>> CPURISCVState *env = &cpu->env;
> >>>> bool virt = env->virt_enabled;
> >>>> bool write_gva = false;
> >>>> + bool vsmode_exc;
> >>>> uint64_t s;
> >>>> int mode;
> >>>>
> >>>> @@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >>>> !(env->mip & (1 << cause));
> >>>> bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
> >>>> !(env->mip & (1 << cause));
> >>>> + bool smode_double_trap = false;
> >>>> + uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
> >>>> target_ulong tval = 0;
> >>>> target_ulong tinst = 0;
> >>>> target_ulong htval = 0;
> >>>> @@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >>>> !async &&
> >>>> mode == PRV_M;
> >>>>
> >>>> + vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
> >>>> + /*
> >>>> + * Check double trap condition only if already in S-mode and targeting
> >>>> + * S-mode
> >>>> + */
> >>>> + if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
> >>>> + bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
> >>>> + bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
> >>>> + /* In VS or HS */
> >>>> + if (riscv_has_ext(env, RVH)) {
> >>>> + if (vsmode_exc) {
> >>>> + /* VS -> VS */
> >>>> + /* Stay in VS mode, use henvcfg instead of menvcfg*/
> >>>> + dte = (env->henvcfg & HENVCFG_DTE) != 0;
> >>>> + } else if (env->virt_enabled) {
> >>>> + /* VS -> HS */
> >>>> + dte = false;
> >>>
> >>> I don't follow why this is false
> >>
> >> Hi Alistair,
> >>
> >> It's indeed probably lacking some comments here. The rationale is that
> >> if you are trapping from VS to HS, then at some point, you returned to
> >> VS using a sret/mret and thus cleared DTE, so rather than checking the
>
> s/DTE/SDT
>
> >
> > Why not just clear it at sret/mret? Instead of having this assumption
>
> It has been cleared but since registers were swapped to enter virt mode,
> hypervisor SDT value is stored in mstatus_hs rather than mstatus. So I
> could have wrote it this way:
>
> + } else if (env->virt_enabled) {
> + /* VS -> HS */
> + sdt = (env->mstatus_hs & MSTATUS_SDT);
>
> Since this is always 0 better assume it is 0 (but should be sdt = 0
> instead of dte = 0). But if you prefer using mstatus_hs for clarity, I
> can use that of course.
We should use the register directly. That way if it is accidently not
cleared it's easier to catch and it makes the code easier to read
Alistair
>
> Clément
>
> >
> > Alistair
> >
> >> value of mstatus_hs, just assume it is false.
> >>
> >> Thanks,
> >>
> >> Clément
> >>
> >>>
> >>> Alistair
> >>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior
2024-10-18 2:21 ` Alistair Francis
@ 2024-10-18 7:02 ` Clément Léger
0 siblings, 0 replies; 26+ messages in thread
From: Clément Léger @ 2024-10-18 7:02 UTC (permalink / raw)
To: Alistair Francis, Ved Shanbhogue
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Atish Patra, qemu-devel
On 18/10/2024 04:21, Alistair Francis wrote:
> On Fri, Oct 18, 2024 at 4:27 AM Ved Shanbhogue <ved@rivosinc.com> wrote:
>>
>> Alistair Francis wrote:
>>> $ grep -r sstatus.SDT | grep SRET
>>> src/hypervisor.adoc:if the new privilege mode is VU, the `SRET`
>>> instruction sets `vsstatus.SDT`
>>>
>>> What am I missing here?
>>
>> https://github.com/riscv/riscv-isa-manual/blob/ef2ec9dc9afd003d0dab6d5ca36db59864c8483c/src/machine.adoc?plain=1#L538
>
> Ah, I thought you were quoting the spec directly.
>
> Makes sense. This patch misses the MDT bit clearing though. I'm
> guessing that's implemented in a different patch, but it should be
> pulled in here instead
This is actually done in the patch that adds Smdbltrp (this on is for
Ssdbltrp).
Thanks,
Clément
>
> Alistair
>
>>
>>
>> regards
>> ved
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling
2024-10-18 2:25 ` Alistair Francis
@ 2024-10-18 7:04 ` Clément Léger
0 siblings, 0 replies; 26+ messages in thread
From: Clément Léger @ 2024-10-18 7:04 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Ved Shanbhogue, Atish Patra,
qemu-devel
On 18/10/2024 04:25, Alistair Francis wrote:
> On Thu, Oct 17, 2024 at 5:45 PM Clément Léger <cleger@rivosinc.com> wrote:
>>
>>
>>
>> On 17/10/2024 06:29, Alistair Francis wrote:
>>> On Mon, Oct 14, 2024 at 5:43 PM Clément Léger <cleger@rivosinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 11/10/2024 05:22, Alistair Francis wrote:
>>>>> On Wed, Sep 25, 2024 at 9:59 PM Clément Léger <cleger@rivosinc.com> wrote:
>>>>>>
>>>>>> When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
>>>>>> while SSTATUS.SDT isn't cleared, generate a double trap exception to
>>>>>> M-mode.
>>>>>>
>>>>>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>>>>>> ---
>>>>>> target/riscv/cpu.c | 2 +-
>>>>>> target/riscv/cpu_bits.h | 1 +
>>>>>> target/riscv/cpu_helper.c | 47 ++++++++++++++++++++++++++++++++++-----
>>>>>> 3 files changed, 43 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>>>> index cf06cd741a..65347ccd5a 100644
>>>>>> --- a/target/riscv/cpu.c
>>>>>> +++ b/target/riscv/cpu.c
>>>>>> @@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
>>>>>> "load_page_fault",
>>>>>> "reserved",
>>>>>> "store_page_fault",
>>>>>> - "reserved",
>>>>>> + "double_trap",
>>>>>> "reserved",
>>>>>> "reserved",
>>>>>> "reserved",
>>>>>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>>>>>> index 3a5588d4df..5557a86348 100644
>>>>>> --- a/target/riscv/cpu_bits.h
>>>>>> +++ b/target/riscv/cpu_bits.h
>>>>>> @@ -699,6 +699,7 @@ typedef enum RISCVException {
>>>>>> RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
>>>>>> RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
>>>>>> RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
>>>>>> + RISCV_EXCP_DOUBLE_TRAP = 0x10,
>>>>>> RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
>>>>>> RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
>>>>>> RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
>>>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>>>> index 395d8235ce..69da3c3384 100644
>>>>>> --- a/target/riscv/cpu_helper.c
>>>>>> +++ b/target/riscv/cpu_helper.c
>>>>>> @@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>>>>>> mstatus_mask |= MSTATUS_FS;
>>>>>> }
>>>>>> bool current_virt = env->virt_enabled;
>>>>>> -
>>>>>> + if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
>>>>>> + mstatus_mask |= MSTATUS_SDT;
>>>>>> + }
>>>>>> g_assert(riscv_has_ext(env, RVH));
>>>>>>
>>>>>> if (current_virt) {
>>>>>> @@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>>>>> CPURISCVState *env = &cpu->env;
>>>>>> bool virt = env->virt_enabled;
>>>>>> bool write_gva = false;
>>>>>> + bool vsmode_exc;
>>>>>> uint64_t s;
>>>>>> int mode;
>>>>>>
>>>>>> @@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>>>>> !(env->mip & (1 << cause));
>>>>>> bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
>>>>>> !(env->mip & (1 << cause));
>>>>>> + bool smode_double_trap = false;
>>>>>> + uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
>>>>>> target_ulong tval = 0;
>>>>>> target_ulong tinst = 0;
>>>>>> target_ulong htval = 0;
>>>>>> @@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>>>>> !async &&
>>>>>> mode == PRV_M;
>>>>>>
>>>>>> + vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
>>>>>> + /*
>>>>>> + * Check double trap condition only if already in S-mode and targeting
>>>>>> + * S-mode
>>>>>> + */
>>>>>> + if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
>>>>>> + bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
>>>>>> + bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
>>>>>> + /* In VS or HS */
>>>>>> + if (riscv_has_ext(env, RVH)) {
>>>>>> + if (vsmode_exc) {
>>>>>> + /* VS -> VS */
>>>>>> + /* Stay in VS mode, use henvcfg instead of menvcfg*/
>>>>>> + dte = (env->henvcfg & HENVCFG_DTE) != 0;
>>>>>> + } else if (env->virt_enabled) {
>>>>>> + /* VS -> HS */
>>>>>> + dte = false;
>>>>>
>>>>> I don't follow why this is false
>>>>
>>>> Hi Alistair,
>>>>
>>>> It's indeed probably lacking some comments here. The rationale is that
>>>> if you are trapping from VS to HS, then at some point, you returned to
>>>> VS using a sret/mret and thus cleared DTE, so rather than checking the
>>
>> s/DTE/SDT
>>
>>>
>>> Why not just clear it at sret/mret? Instead of having this assumption
>>
>> It has been cleared but since registers were swapped to enter virt mode,
>> hypervisor SDT value is stored in mstatus_hs rather than mstatus. So I
>> could have wrote it this way:
>>
>> + } else if (env->virt_enabled) {
>> + /* VS -> HS */
>> + sdt = (env->mstatus_hs & MSTATUS_SDT);
>>
>> Since this is always 0 better assume it is 0 (but should be sdt = 0
>> instead of dte = 0). But if you prefer using mstatus_hs for clarity, I
>> can use that of course.
>
> We should use the register directly. That way if it is accidently not
> cleared it's easier to catch and it makes the code easier to read
Yes indeed. I did that in the next version.
Thanks,
Clément
>
> Alistair
>
>>
>> Clément
>>
>>>
>>> Alistair
>>>
>>>> value of mstatus_hs, just assume it is false.
>>>>
>>>> Thanks,
>>>>
>>>> Clément
>>>>
>>>>>
>>>>> Alistair
>>>>
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-10-18 7:04 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-25 11:57 [PATCH v2 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions Clément Léger
2024-09-25 11:57 ` [PATCH v2 1/8] target/riscv: Add Ssdbltrp CSRs handling Clément Léger
2024-10-11 2:49 ` Alistair Francis
2024-09-25 11:58 ` [PATCH v2 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior Clément Léger
2024-10-11 3:13 ` Alistair Francis
2024-10-11 18:52 ` Ved Shanbhogue
2024-10-17 4:36 ` Alistair Francis
2024-10-17 18:27 ` Ved Shanbhogue
2024-10-18 2:21 ` Alistair Francis
2024-10-18 7:02 ` Clément Léger
2024-09-25 11:58 ` [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling Clément Léger
2024-10-11 3:22 ` Alistair Francis
2024-10-14 7:43 ` Clément Léger
2024-10-17 4:29 ` Alistair Francis
2024-10-17 7:45 ` Clément Léger
2024-10-18 2:25 ` Alistair Francis
2024-10-18 7:04 ` Clément Léger
2024-09-25 11:58 ` [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch Clément Léger
2024-10-11 3:24 ` Alistair Francis
2024-10-14 7:43 ` Clément Léger
2024-09-25 11:58 ` [PATCH v2 5/8] target/riscv: Add Smdbltrp CSRs handling Clément Léger
2024-10-11 3:30 ` Alistair Francis
2024-10-11 7:40 ` Clément Léger
2024-09-25 11:58 ` [PATCH v2 6/8] target/riscv: Implement Smdbltrp sret, mret and mnret behavior Clément Léger
2024-09-25 11:58 ` [PATCH v2 7/8] target/riscv: Implement Smdbltrp behavior Clément Léger
2024-09-25 11:58 ` [PATCH v2 8/8] target/riscv: Add Smdbltrp ISA extension enable switch Clément Léger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).