* [PATCH 0/4] QEMU RISC-V nested virtualization fixes
@ 2022-04-29 3:34 ` Anup Patel
0 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
This series does fixes and improvements to have nested virtualization
on QEMU RISC-V. The first two patches are fixes whereas the second
two patches make nested virtualization performance better on for
QEMU RISC-V.
These patches can also be found in riscv_nested_fixes_v1 branch at:
https://github.com/avpatel/qemu.git
The RISC-V nested virtualization was tested on QEMU RISC-V using
Xvisor RISC-V which has required hypervisor support to run another
hypervisor as Guest/VM.
Anup Patel (4):
target/riscv: Fix csr number based privilege checking
target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
target/riscv: Set [m|s]tval for both illegal and virtual instruction
traps
target/riscv: Update [m|h]tinst CSR in riscv_cpu_do_interrupt()
target/riscv/cpu.c | 2 +
target/riscv/cpu.h | 8 +-
target/riscv/cpu_helper.c | 170 ++++++++++++++++++++++++++++++++++++--
target/riscv/csr.c | 8 +-
target/riscv/instmap.h | 41 +++++++++
target/riscv/translate.c | 17 +++-
6 files changed, 234 insertions(+), 12 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/4] QEMU RISC-V nested virtualization fixes
@ 2022-04-29 3:34 ` Anup Patel
0 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
This series does fixes and improvements to have nested virtualization
on QEMU RISC-V. The first two patches are fixes whereas the second
two patches make nested virtualization performance better on for
QEMU RISC-V.
These patches can also be found in riscv_nested_fixes_v1 branch at:
https://github.com/avpatel/qemu.git
The RISC-V nested virtualization was tested on QEMU RISC-V using
Xvisor RISC-V which has required hypervisor support to run another
hypervisor as Guest/VM.
Anup Patel (4):
target/riscv: Fix csr number based privilege checking
target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
target/riscv: Set [m|s]tval for both illegal and virtual instruction
traps
target/riscv: Update [m|h]tinst CSR in riscv_cpu_do_interrupt()
target/riscv/cpu.c | 2 +
target/riscv/cpu.h | 8 +-
target/riscv/cpu_helper.c | 170 ++++++++++++++++++++++++++++++++++++--
target/riscv/csr.c | 8 +-
target/riscv/instmap.h | 41 +++++++++
target/riscv/translate.c | 17 +++-
6 files changed, 234 insertions(+), 12 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] target/riscv: Fix csr number based privilege checking
2022-04-29 3:34 ` Anup Patel
@ 2022-04-29 3:34 ` Anup Patel
-1 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
When hypervisor and VS CSRs are accessed from VS-mode or VU-mode,
the riscv_csrrw_check() function should generate virtual instruction
trap instead illegal instruction trap.
Fixes: 533c91e8f22c ("target/riscv: Use RISCVException enum for
CSR access")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/csr.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 3500e07f92..2bf0a97196 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3139,7 +3139,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
int read_only = get_field(csrno, 0xC00) == 3;
int csr_min_priv = csr_ops[csrno].min_priv_ver;
#if !defined(CONFIG_USER_ONLY)
- int effective_priv = env->priv;
+ int csr_priv, effective_priv = env->priv;
if (riscv_has_ext(env, RVH) &&
env->priv == PRV_S &&
@@ -3152,7 +3152,11 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
effective_priv++;
}
- if (!env->debugger && (effective_priv < get_field(csrno, 0x300))) {
+ csr_priv = get_field(csrno, 0x300);
+ if (!env->debugger && (effective_priv < csr_priv)) {
+ if (csr_priv == (PRV_S + 1) && riscv_cpu_virt_enabled(env)) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
return RISCV_EXCP_ILLEGAL_INST;
}
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 1/4] target/riscv: Fix csr number based privilege checking
@ 2022-04-29 3:34 ` Anup Patel
0 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
When hypervisor and VS CSRs are accessed from VS-mode or VU-mode,
the riscv_csrrw_check() function should generate virtual instruction
trap instead illegal instruction trap.
Fixes: 533c91e8f22c ("target/riscv: Use RISCVException enum for
CSR access")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/csr.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 3500e07f92..2bf0a97196 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3139,7 +3139,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
int read_only = get_field(csrno, 0xC00) == 3;
int csr_min_priv = csr_ops[csrno].min_priv_ver;
#if !defined(CONFIG_USER_ONLY)
- int effective_priv = env->priv;
+ int csr_priv, effective_priv = env->priv;
if (riscv_has_ext(env, RVH) &&
env->priv == PRV_S &&
@@ -3152,7 +3152,11 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
effective_priv++;
}
- if (!env->debugger && (effective_priv < get_field(csrno, 0x300))) {
+ csr_priv = get_field(csrno, 0x300);
+ if (!env->debugger && (effective_priv < csr_priv)) {
+ if (csr_priv == (PRV_S + 1) && riscv_cpu_virt_enabled(env)) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
return RISCV_EXCP_ILLEGAL_INST;
}
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
2022-04-29 3:34 ` Anup Patel
@ 2022-04-29 3:34 ` Anup Patel
-1 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
Currently, QEMU does not set hstatus.GVA bit for traps taken from
HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
on QEMU. This was working previously.
This patch updates riscv_cpu_do_interrupt() to fix the above issue.
Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu_helper.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e1aa4f2097..d83579accf 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
/* Trap into HS mode */
env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
htval = env->guest_phys_fault_addr;
- write_gva = false;
}
env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
@ 2022-04-29 3:34 ` Anup Patel
0 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
Currently, QEMU does not set hstatus.GVA bit for traps taken from
HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
on QEMU. This was working previously.
This patch updates riscv_cpu_do_interrupt() to fix the above issue.
Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu_helper.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e1aa4f2097..d83579accf 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
/* Trap into HS mode */
env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
htval = env->guest_phys_fault_addr;
- write_gva = false;
}
env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] target/riscv: Set [m|s]tval for both illegal and virtual instruction traps
2022-04-29 3:34 ` Anup Patel
@ 2022-04-29 3:34 ` Anup Patel
-1 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
Currently, the [m|s]tval CSRs are set with trapping instruction encoding
only for illegal instruction traps taken at the time of instruction
decoding.
In RISC-V world, a valid instructions might also trap as illegal or
virtual instruction based to trapping bits in various CSRs (such as
mstatus.TVM or hstatus.VTVM).
We improve setting of [m|s]tval CSRs for all types of illegal and
virtual instruction traps.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu.h | 8 +++++++-
target/riscv/cpu_helper.c | 1 +
target/riscv/translate.c | 17 +++++++++++++----
4 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index dff4606585..f0a702fee6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -406,6 +406,7 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
} else {
env->pc = data[0];
}
+ env->bins = data[1];
}
static void riscv_cpu_reset(DeviceState *dev)
@@ -445,6 +446,7 @@ static void riscv_cpu_reset(DeviceState *dev)
env->mcause = 0;
env->miclaim = MIP_SGEIP;
env->pc = env->resetvec;
+ env->bins = 0;
env->two_stage_lookup = false;
/* Initialized default priorities of local interrupts. */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index fe6c9a2c92..a55c918274 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -30,6 +30,12 @@
#define TCG_GUEST_DEFAULT_MO 0
+/*
+ * RISC-V-specific extra insn start words:
+ * 1: Original instruction opcode
+ */
+#define TARGET_INSN_START_EXTRA_WORDS 1
+
#define TYPE_RISCV_CPU "riscv-cpu"
#define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
@@ -140,7 +146,7 @@ struct CPUArchState {
target_ulong frm;
target_ulong badaddr;
- uint32_t bins;
+ target_ulong bins;
target_ulong guest_phys_fault_addr;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d83579accf..bba4fce777 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1371,6 +1371,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
tval = env->badaddr;
break;
case RISCV_EXCP_ILLEGAL_INST:
+ case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
tval = env->bins;
break;
default:
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0cd1d9ee94..55a4713af2 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -107,6 +107,8 @@ typedef struct DisasContext {
/* PointerMasking extension */
bool pm_mask_enabled;
bool pm_base_enabled;
+ /* TCG of the current insn_start */
+ TCGOp *insn_start;
} DisasContext;
static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -236,9 +238,6 @@ static void generate_exception_mtval(DisasContext *ctx, int excp)
static void gen_exception_illegal(DisasContext *ctx)
{
- tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
- offsetof(CPURISCVState, bins));
-
generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
}
@@ -1017,6 +1016,13 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
/* Include decoders for factored-out extensions */
#include "decode-XVentanaCondOps.c.inc"
+static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
+{
+ assert(ctx->insn_start != NULL);
+ tcg_set_insn_start_param(ctx->insn_start, 1, opc);
+ ctx->insn_start = NULL;
+}
+
static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
{
/*
@@ -1033,6 +1039,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
/* Check for compressed insn */
if (extract16(opcode, 0, 2) != 3) {
+ decode_save_opc(ctx, opcode);
if (!has_ext(ctx, RVC)) {
gen_exception_illegal(ctx);
} else {
@@ -1047,6 +1054,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
opcode32 = deposit32(opcode32, 16, 16,
translator_lduw(env, &ctx->base,
ctx->base.pc_next + 2));
+ decode_save_opc(ctx, opcode32);
ctx->opcode = opcode32;
ctx->pc_succ_insn = ctx->base.pc_next + 4;
@@ -1113,7 +1121,8 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
- tcg_gen_insn_start(ctx->base.pc_next);
+ tcg_gen_insn_start(ctx->base.pc_next, 0);
+ ctx->insn_start = tcg_last_op();
}
static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] target/riscv: Set [m|s]tval for both illegal and virtual instruction traps
@ 2022-04-29 3:34 ` Anup Patel
0 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
Currently, the [m|s]tval CSRs are set with trapping instruction encoding
only for illegal instruction traps taken at the time of instruction
decoding.
In RISC-V world, a valid instructions might also trap as illegal or
virtual instruction based to trapping bits in various CSRs (such as
mstatus.TVM or hstatus.VTVM).
We improve setting of [m|s]tval CSRs for all types of illegal and
virtual instruction traps.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu.h | 8 +++++++-
target/riscv/cpu_helper.c | 1 +
target/riscv/translate.c | 17 +++++++++++++----
4 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index dff4606585..f0a702fee6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -406,6 +406,7 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
} else {
env->pc = data[0];
}
+ env->bins = data[1];
}
static void riscv_cpu_reset(DeviceState *dev)
@@ -445,6 +446,7 @@ static void riscv_cpu_reset(DeviceState *dev)
env->mcause = 0;
env->miclaim = MIP_SGEIP;
env->pc = env->resetvec;
+ env->bins = 0;
env->two_stage_lookup = false;
/* Initialized default priorities of local interrupts. */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index fe6c9a2c92..a55c918274 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -30,6 +30,12 @@
#define TCG_GUEST_DEFAULT_MO 0
+/*
+ * RISC-V-specific extra insn start words:
+ * 1: Original instruction opcode
+ */
+#define TARGET_INSN_START_EXTRA_WORDS 1
+
#define TYPE_RISCV_CPU "riscv-cpu"
#define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
@@ -140,7 +146,7 @@ struct CPUArchState {
target_ulong frm;
target_ulong badaddr;
- uint32_t bins;
+ target_ulong bins;
target_ulong guest_phys_fault_addr;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d83579accf..bba4fce777 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1371,6 +1371,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
tval = env->badaddr;
break;
case RISCV_EXCP_ILLEGAL_INST:
+ case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
tval = env->bins;
break;
default:
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0cd1d9ee94..55a4713af2 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -107,6 +107,8 @@ typedef struct DisasContext {
/* PointerMasking extension */
bool pm_mask_enabled;
bool pm_base_enabled;
+ /* TCG of the current insn_start */
+ TCGOp *insn_start;
} DisasContext;
static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -236,9 +238,6 @@ static void generate_exception_mtval(DisasContext *ctx, int excp)
static void gen_exception_illegal(DisasContext *ctx)
{
- tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
- offsetof(CPURISCVState, bins));
-
generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
}
@@ -1017,6 +1016,13 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
/* Include decoders for factored-out extensions */
#include "decode-XVentanaCondOps.c.inc"
+static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
+{
+ assert(ctx->insn_start != NULL);
+ tcg_set_insn_start_param(ctx->insn_start, 1, opc);
+ ctx->insn_start = NULL;
+}
+
static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
{
/*
@@ -1033,6 +1039,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
/* Check for compressed insn */
if (extract16(opcode, 0, 2) != 3) {
+ decode_save_opc(ctx, opcode);
if (!has_ext(ctx, RVC)) {
gen_exception_illegal(ctx);
} else {
@@ -1047,6 +1054,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
opcode32 = deposit32(opcode32, 16, 16,
translator_lduw(env, &ctx->base,
ctx->base.pc_next + 2));
+ decode_save_opc(ctx, opcode32);
ctx->opcode = opcode32;
ctx->pc_succ_insn = ctx->base.pc_next + 4;
@@ -1113,7 +1121,8 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
- tcg_gen_insn_start(ctx->base.pc_next);
+ tcg_gen_insn_start(ctx->base.pc_next, 0);
+ ctx->insn_start = tcg_last_op();
}
static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] target/riscv: Update [m|h]tinst CSR in riscv_cpu_do_interrupt()
2022-04-29 3:34 ` Anup Patel
@ 2022-04-29 3:34 ` Anup Patel
-1 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
We should write transformed instruction encoding of the trapped
instruction in [m|h]tinst CSR at time of taking trap as defined
by the RISC-V privileged specification v1.12.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu_helper.c | 168 +++++++++++++++++++++++++++++++++++++-
target/riscv/instmap.h | 41 ++++++++++
2 files changed, 205 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index bba4fce777..58f3b2effb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -22,6 +22,7 @@
#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/exec-all.h"
+#include "instmap.h"
#include "tcg/tcg-op.h"
#include "trace.h"
#include "semihosting/common-semi.h"
@@ -1318,6 +1319,158 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
}
#endif /* !CONFIG_USER_ONLY */
+static target_ulong riscv_transformed_insn(CPURISCVState *env,
+ target_ulong insn)
+{
+ target_ulong xinsn = 0;
+
+ /*
+ * Only Quadrant 0 and Quadrant 2 of RVC instruction space need to
+ * be uncompressed. The Quadrant 1 of RVC instruction space need
+ * not be transformed because these instructions won't generate
+ * any load/store trap.
+ */
+
+ if ((insn & 0x3) != 0x3) {
+ /* Transform 16bit instruction into 32bit instruction */
+ switch (GET_C_OP(insn)) {
+ case OPC_RISC_C_OP_QUAD0: /* Quadrant 0 */
+ switch (GET_C_FUNC(insn)) {
+ case OPC_RISC_C_FUNC_FLD_LQ:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FLD (RV32/64) */
+ xinsn = OPC_RISC_FLD;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LD_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_LW: /* C.LW */
+ xinsn = OPC_RISC_LW;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LW_IMM(insn));
+ break;
+ case OPC_RISC_C_FUNC_FLW_LD:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FLW (RV32) */
+ xinsn = OPC_RISC_FLW;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LW_IMM(insn));
+ } else { /* C.LD (RV64/RV128) */
+ xinsn = OPC_RISC_LD;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LD_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_FSD_SQ:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FSD (RV32/64) */
+ xinsn = OPC_RISC_FSD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SD_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_SW: /* C.SW */
+ xinsn = OPC_RISC_SW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SW_IMM(insn));
+ break;
+ case OPC_RISC_C_FUNC_FSW_SD:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FSW (RV32) */
+ xinsn = OPC_RISC_FSW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SW_IMM(insn));
+ } else { /* C.SD (RV64/RV128) */
+ xinsn = OPC_RISC_SD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SD_IMM(insn));
+ }
+ break;
+ default:
+ break;
+ }
+ break;
+ case OPC_RISC_C_OP_QUAD2: /* Quadrant 2 */
+ switch (GET_C_FUNC(insn)) {
+ case OPC_RISC_C_FUNC_FLDSP_LQSP:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FLDSP (RV32/64) */
+ xinsn = OPC_RISC_FLD;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LDSP_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_LWSP: /* C.LWSP */
+ xinsn = OPC_RISC_LW;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LWSP_IMM(insn));
+ break;
+ case OPC_RISC_C_FUNC_FLWSP_LDSP:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FLWSP (RV32) */
+ xinsn = OPC_RISC_FLW;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LWSP_IMM(insn));
+ } else { /* C.LDSP (RV64/RV128) */
+ xinsn = OPC_RISC_LD;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LDSP_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_FSDSP_SQSP:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FSDSP (RV32/64) */
+ xinsn = OPC_RISC_FSD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SDSP_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_SWSP: /* C.SWSP */
+ xinsn = OPC_RISC_SW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SWSP_IMM(insn));
+ break;
+ case 7:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FSWSP (RV32) */
+ xinsn = OPC_RISC_FSW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SWSP_IMM(insn));
+ } else { /* C.SDSP (RV64/RV128) */
+ xinsn = OPC_RISC_SD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SDSP_IMM(insn));
+ }
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ /*
+ * Clear Bit1 of transformed instruction to indicate that
+ * original insruction was a 16bit instruction
+ */
+ xinsn &= ~((target_ulong)0x2);
+ } else {
+ /* No need to transform 32bit (or wider) instructions */
+ xinsn = insn;
+ }
+
+ return xinsn;
+}
+
/*
* Handle Traps
*
@@ -1340,6 +1493,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
target_ulong cause = cs->exception_index & RISCV_EXCP_INT_MASK;
uint64_t deleg = async ? env->mideleg : env->medeleg;
target_ulong tval = 0;
+ target_ulong tinst = 0;
target_ulong htval = 0;
target_ulong mtval2 = 0;
@@ -1355,18 +1509,22 @@ void riscv_cpu_do_interrupt(CPUState *cs)
if (!async) {
/* set tval to badaddr for traps with address information */
switch (cause) {
- case RISCV_EXCP_INST_GUEST_PAGE_FAULT:
case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
- case RISCV_EXCP_INST_ADDR_MIS:
- case RISCV_EXCP_INST_ACCESS_FAULT:
case RISCV_EXCP_LOAD_ADDR_MIS:
case RISCV_EXCP_STORE_AMO_ADDR_MIS:
case RISCV_EXCP_LOAD_ACCESS_FAULT:
case RISCV_EXCP_STORE_AMO_ACCESS_FAULT:
- case RISCV_EXCP_INST_PAGE_FAULT:
case RISCV_EXCP_LOAD_PAGE_FAULT:
case RISCV_EXCP_STORE_PAGE_FAULT:
+ write_gva = true;
+ tval = env->badaddr;
+ tinst = riscv_transformed_insn(env, env->bins);
+ break;
+ case RISCV_EXCP_INST_GUEST_PAGE_FAULT:
+ case RISCV_EXCP_INST_ADDR_MIS:
+ case RISCV_EXCP_INST_ACCESS_FAULT:
+ case RISCV_EXCP_INST_PAGE_FAULT:
write_gva = true;
tval = env->badaddr;
break;
@@ -1448,6 +1606,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
env->sepc = env->pc;
env->stval = tval;
env->htval = htval;
+ env->htinst = tinst;
env->pc = (env->stvec >> 2 << 2) +
((async && (env->stvec & 3) == 1) ? cause * 4 : 0);
riscv_cpu_set_mode(env, PRV_S);
@@ -1478,6 +1637,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
env->mepc = env->pc;
env->mtval = tval;
env->mtval2 = mtval2;
+ env->mtinst = tinst;
env->pc = (env->mtvec >> 2 << 2) +
((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
riscv_cpu_set_mode(env, PRV_M);
diff --git a/target/riscv/instmap.h b/target/riscv/instmap.h
index 40b6d2b64d..f4ee686c78 100644
--- a/target/riscv/instmap.h
+++ b/target/riscv/instmap.h
@@ -316,6 +316,12 @@ enum {
#define GET_RS2(inst) extract32(inst, 20, 5)
#define GET_RD(inst) extract32(inst, 7, 5)
#define GET_IMM(inst) sextract64(inst, 20, 12)
+#define SET_RS1(inst, val) deposit32(inst, 15, 5, val)
+#define SET_RS2(inst, val) deposit32(inst, 20, 5, val)
+#define SET_RD(inst, val) deposit32(inst, 7, 5, val)
+#define SET_I_IMM(inst, val) deposit32(inst, 20, 12, val)
+#define SET_S_IMM(inst, val) \
+ deposit32(deposit32(inst, 7, 5, val), 25, 7, (val) >> 5)
/* RVC decoding macros */
#define GET_C_IMM(inst) (extract32(inst, 2, 5) \
@@ -346,6 +352,8 @@ enum {
| (extract32(inst, 5, 1) << 6))
#define GET_C_LD_IMM(inst) ((extract16(inst, 10, 3) << 3) \
| (extract16(inst, 5, 2) << 6))
+#define GET_C_SW_IMM(inst) GET_C_LW_IMM(inst)
+#define GET_C_SD_IMM(inst) GET_C_LD_IMM(inst)
#define GET_C_J_IMM(inst) ((extract32(inst, 3, 3) << 1) \
| (extract32(inst, 11, 1) << 4) \
| (extract32(inst, 2, 1) << 5) \
@@ -366,4 +374,37 @@ enum {
#define GET_C_RS1S(inst) (8 + extract16(inst, 7, 3))
#define GET_C_RS2S(inst) (8 + extract16(inst, 2, 3))
+#define GET_C_FUNC(inst) extract32(inst, 13, 3)
+#define GET_C_OP(inst) extract32(inst, 0, 2)
+
+enum {
+ /* RVC Quadrants */
+ OPC_RISC_C_OP_QUAD0 = 0x0,
+ OPC_RISC_C_OP_QUAD1 = 0x1,
+ OPC_RISC_C_OP_QUAD2 = 0x2
+};
+
+enum {
+ /* RVC Quadrant 0 */
+ OPC_RISC_C_FUNC_ADDI4SPN = 0x0,
+ OPC_RISC_C_FUNC_FLD_LQ = 0x1,
+ OPC_RISC_C_FUNC_LW = 0x2,
+ OPC_RISC_C_FUNC_FLW_LD = 0x3,
+ OPC_RISC_C_FUNC_FSD_SQ = 0x5,
+ OPC_RISC_C_FUNC_SW = 0x6,
+ OPC_RISC_C_FUNC_FSW_SD = 0x7
+};
+
+enum {
+ /* RVC Quadrant 2 */
+ OPC_RISC_C_FUNC_SLLI_SLLI64 = 0x0,
+ OPC_RISC_C_FUNC_FLDSP_LQSP = 0x1,
+ OPC_RISC_C_FUNC_LWSP = 0x2,
+ OPC_RISC_C_FUNC_FLWSP_LDSP = 0x3,
+ OPC_RISC_C_FUNC_JR_MV_EBREAK_JALR_ADD = 0x4,
+ OPC_RISC_C_FUNC_FSDSP_SQSP = 0x5,
+ OPC_RISC_C_FUNC_SWSP = 0x6,
+ OPC_RISC_C_FUNC_FSWSP_SDSP = 0x7
+};
+
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] target/riscv: Update [m|h]tinst CSR in riscv_cpu_do_interrupt()
@ 2022-04-29 3:34 ` Anup Patel
0 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-04-29 3:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
We should write transformed instruction encoding of the trapped
instruction in [m|h]tinst CSR at time of taking trap as defined
by the RISC-V privileged specification v1.12.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu_helper.c | 168 +++++++++++++++++++++++++++++++++++++-
target/riscv/instmap.h | 41 ++++++++++
2 files changed, 205 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index bba4fce777..58f3b2effb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -22,6 +22,7 @@
#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/exec-all.h"
+#include "instmap.h"
#include "tcg/tcg-op.h"
#include "trace.h"
#include "semihosting/common-semi.h"
@@ -1318,6 +1319,158 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
}
#endif /* !CONFIG_USER_ONLY */
+static target_ulong riscv_transformed_insn(CPURISCVState *env,
+ target_ulong insn)
+{
+ target_ulong xinsn = 0;
+
+ /*
+ * Only Quadrant 0 and Quadrant 2 of RVC instruction space need to
+ * be uncompressed. The Quadrant 1 of RVC instruction space need
+ * not be transformed because these instructions won't generate
+ * any load/store trap.
+ */
+
+ if ((insn & 0x3) != 0x3) {
+ /* Transform 16bit instruction into 32bit instruction */
+ switch (GET_C_OP(insn)) {
+ case OPC_RISC_C_OP_QUAD0: /* Quadrant 0 */
+ switch (GET_C_FUNC(insn)) {
+ case OPC_RISC_C_FUNC_FLD_LQ:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FLD (RV32/64) */
+ xinsn = OPC_RISC_FLD;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LD_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_LW: /* C.LW */
+ xinsn = OPC_RISC_LW;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LW_IMM(insn));
+ break;
+ case OPC_RISC_C_FUNC_FLW_LD:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FLW (RV32) */
+ xinsn = OPC_RISC_FLW;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LW_IMM(insn));
+ } else { /* C.LD (RV64/RV128) */
+ xinsn = OPC_RISC_LD;
+ xinsn = SET_RD(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_I_IMM(xinsn, GET_C_LD_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_FSD_SQ:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FSD (RV32/64) */
+ xinsn = OPC_RISC_FSD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SD_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_SW: /* C.SW */
+ xinsn = OPC_RISC_SW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SW_IMM(insn));
+ break;
+ case OPC_RISC_C_FUNC_FSW_SD:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FSW (RV32) */
+ xinsn = OPC_RISC_FSW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SW_IMM(insn));
+ } else { /* C.SD (RV64/RV128) */
+ xinsn = OPC_RISC_SD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2S(insn));
+ xinsn = SET_RS1(xinsn, GET_C_RS1S(insn));
+ xinsn = SET_S_IMM(xinsn, GET_C_SD_IMM(insn));
+ }
+ break;
+ default:
+ break;
+ }
+ break;
+ case OPC_RISC_C_OP_QUAD2: /* Quadrant 2 */
+ switch (GET_C_FUNC(insn)) {
+ case OPC_RISC_C_FUNC_FLDSP_LQSP:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FLDSP (RV32/64) */
+ xinsn = OPC_RISC_FLD;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LDSP_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_LWSP: /* C.LWSP */
+ xinsn = OPC_RISC_LW;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LWSP_IMM(insn));
+ break;
+ case OPC_RISC_C_FUNC_FLWSP_LDSP:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FLWSP (RV32) */
+ xinsn = OPC_RISC_FLW;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LWSP_IMM(insn));
+ } else { /* C.LDSP (RV64/RV128) */
+ xinsn = OPC_RISC_LD;
+ xinsn = SET_RD(xinsn, GET_C_RD(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_I_IMM(xinsn, GET_C_LDSP_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_FSDSP_SQSP:
+ if (riscv_cpu_xlen(env) != 128) { /* C.FSDSP (RV32/64) */
+ xinsn = OPC_RISC_FSD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SDSP_IMM(insn));
+ }
+ break;
+ case OPC_RISC_C_FUNC_SWSP: /* C.SWSP */
+ xinsn = OPC_RISC_SW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SWSP_IMM(insn));
+ break;
+ case 7:
+ if (riscv_cpu_xlen(env) == 32) { /* C.FSWSP (RV32) */
+ xinsn = OPC_RISC_FSW;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SWSP_IMM(insn));
+ } else { /* C.SDSP (RV64/RV128) */
+ xinsn = OPC_RISC_SD;
+ xinsn = SET_RS2(xinsn, GET_C_RS2(insn));
+ xinsn = SET_RS1(xinsn, 2);
+ xinsn = SET_S_IMM(xinsn, GET_C_SDSP_IMM(insn));
+ }
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ /*
+ * Clear Bit1 of transformed instruction to indicate that
+ * original insruction was a 16bit instruction
+ */
+ xinsn &= ~((target_ulong)0x2);
+ } else {
+ /* No need to transform 32bit (or wider) instructions */
+ xinsn = insn;
+ }
+
+ return xinsn;
+}
+
/*
* Handle Traps
*
@@ -1340,6 +1493,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
target_ulong cause = cs->exception_index & RISCV_EXCP_INT_MASK;
uint64_t deleg = async ? env->mideleg : env->medeleg;
target_ulong tval = 0;
+ target_ulong tinst = 0;
target_ulong htval = 0;
target_ulong mtval2 = 0;
@@ -1355,18 +1509,22 @@ void riscv_cpu_do_interrupt(CPUState *cs)
if (!async) {
/* set tval to badaddr for traps with address information */
switch (cause) {
- case RISCV_EXCP_INST_GUEST_PAGE_FAULT:
case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
- case RISCV_EXCP_INST_ADDR_MIS:
- case RISCV_EXCP_INST_ACCESS_FAULT:
case RISCV_EXCP_LOAD_ADDR_MIS:
case RISCV_EXCP_STORE_AMO_ADDR_MIS:
case RISCV_EXCP_LOAD_ACCESS_FAULT:
case RISCV_EXCP_STORE_AMO_ACCESS_FAULT:
- case RISCV_EXCP_INST_PAGE_FAULT:
case RISCV_EXCP_LOAD_PAGE_FAULT:
case RISCV_EXCP_STORE_PAGE_FAULT:
+ write_gva = true;
+ tval = env->badaddr;
+ tinst = riscv_transformed_insn(env, env->bins);
+ break;
+ case RISCV_EXCP_INST_GUEST_PAGE_FAULT:
+ case RISCV_EXCP_INST_ADDR_MIS:
+ case RISCV_EXCP_INST_ACCESS_FAULT:
+ case RISCV_EXCP_INST_PAGE_FAULT:
write_gva = true;
tval = env->badaddr;
break;
@@ -1448,6 +1606,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
env->sepc = env->pc;
env->stval = tval;
env->htval = htval;
+ env->htinst = tinst;
env->pc = (env->stvec >> 2 << 2) +
((async && (env->stvec & 3) == 1) ? cause * 4 : 0);
riscv_cpu_set_mode(env, PRV_S);
@@ -1478,6 +1637,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
env->mepc = env->pc;
env->mtval = tval;
env->mtval2 = mtval2;
+ env->mtinst = tinst;
env->pc = (env->mtvec >> 2 << 2) +
((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
riscv_cpu_set_mode(env, PRV_M);
diff --git a/target/riscv/instmap.h b/target/riscv/instmap.h
index 40b6d2b64d..f4ee686c78 100644
--- a/target/riscv/instmap.h
+++ b/target/riscv/instmap.h
@@ -316,6 +316,12 @@ enum {
#define GET_RS2(inst) extract32(inst, 20, 5)
#define GET_RD(inst) extract32(inst, 7, 5)
#define GET_IMM(inst) sextract64(inst, 20, 12)
+#define SET_RS1(inst, val) deposit32(inst, 15, 5, val)
+#define SET_RS2(inst, val) deposit32(inst, 20, 5, val)
+#define SET_RD(inst, val) deposit32(inst, 7, 5, val)
+#define SET_I_IMM(inst, val) deposit32(inst, 20, 12, val)
+#define SET_S_IMM(inst, val) \
+ deposit32(deposit32(inst, 7, 5, val), 25, 7, (val) >> 5)
/* RVC decoding macros */
#define GET_C_IMM(inst) (extract32(inst, 2, 5) \
@@ -346,6 +352,8 @@ enum {
| (extract32(inst, 5, 1) << 6))
#define GET_C_LD_IMM(inst) ((extract16(inst, 10, 3) << 3) \
| (extract16(inst, 5, 2) << 6))
+#define GET_C_SW_IMM(inst) GET_C_LW_IMM(inst)
+#define GET_C_SD_IMM(inst) GET_C_LD_IMM(inst)
#define GET_C_J_IMM(inst) ((extract32(inst, 3, 3) << 1) \
| (extract32(inst, 11, 1) << 4) \
| (extract32(inst, 2, 1) << 5) \
@@ -366,4 +374,37 @@ enum {
#define GET_C_RS1S(inst) (8 + extract16(inst, 7, 3))
#define GET_C_RS2S(inst) (8 + extract16(inst, 2, 3))
+#define GET_C_FUNC(inst) extract32(inst, 13, 3)
+#define GET_C_OP(inst) extract32(inst, 0, 2)
+
+enum {
+ /* RVC Quadrants */
+ OPC_RISC_C_OP_QUAD0 = 0x0,
+ OPC_RISC_C_OP_QUAD1 = 0x1,
+ OPC_RISC_C_OP_QUAD2 = 0x2
+};
+
+enum {
+ /* RVC Quadrant 0 */
+ OPC_RISC_C_FUNC_ADDI4SPN = 0x0,
+ OPC_RISC_C_FUNC_FLD_LQ = 0x1,
+ OPC_RISC_C_FUNC_LW = 0x2,
+ OPC_RISC_C_FUNC_FLW_LD = 0x3,
+ OPC_RISC_C_FUNC_FSD_SQ = 0x5,
+ OPC_RISC_C_FUNC_SW = 0x6,
+ OPC_RISC_C_FUNC_FSW_SD = 0x7
+};
+
+enum {
+ /* RVC Quadrant 2 */
+ OPC_RISC_C_FUNC_SLLI_SLLI64 = 0x0,
+ OPC_RISC_C_FUNC_FLDSP_LQSP = 0x1,
+ OPC_RISC_C_FUNC_LWSP = 0x2,
+ OPC_RISC_C_FUNC_FLWSP_LDSP = 0x3,
+ OPC_RISC_C_FUNC_JR_MV_EBREAK_JALR_ADD = 0x4,
+ OPC_RISC_C_FUNC_FSDSP_SQSP = 0x5,
+ OPC_RISC_C_FUNC_SWSP = 0x6,
+ OPC_RISC_C_FUNC_FSWSP_SDSP = 0x7
+};
+
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] target/riscv: Fix csr number based privilege checking
2022-04-29 3:34 ` Anup Patel
@ 2022-04-29 10:54 ` Alistair Francis
-1 siblings, 0 replies; 22+ messages in thread
From: Alistair Francis @ 2022-04-29 10:54 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, open list:RISC-V, Sagar Karandikar, Anup Patel,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
On Fri, Apr 29, 2022 at 1:36 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> When hypervisor and VS CSRs are accessed from VS-mode or VU-mode,
> the riscv_csrrw_check() function should generate virtual instruction
> trap instead illegal instruction trap.
>
> Fixes: 533c91e8f22c ("target/riscv: Use RISCVException enum for
> CSR access")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/csr.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3500e07f92..2bf0a97196 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3139,7 +3139,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> int read_only = get_field(csrno, 0xC00) == 3;
> int csr_min_priv = csr_ops[csrno].min_priv_ver;
> #if !defined(CONFIG_USER_ONLY)
> - int effective_priv = env->priv;
> + int csr_priv, effective_priv = env->priv;
>
> if (riscv_has_ext(env, RVH) &&
> env->priv == PRV_S &&
> @@ -3152,7 +3152,11 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> effective_priv++;
> }
>
> - if (!env->debugger && (effective_priv < get_field(csrno, 0x300))) {
> + csr_priv = get_field(csrno, 0x300);
> + if (!env->debugger && (effective_priv < csr_priv)) {
> + if (csr_priv == (PRV_S + 1) && riscv_cpu_virt_enabled(env)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> return RISCV_EXCP_ILLEGAL_INST;
> }
> #endif
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] target/riscv: Fix csr number based privilege checking
@ 2022-04-29 10:54 ` Alistair Francis
0 siblings, 0 replies; 22+ messages in thread
From: Alistair Francis @ 2022-04-29 10:54 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Fri, Apr 29, 2022 at 1:36 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> When hypervisor and VS CSRs are accessed from VS-mode or VU-mode,
> the riscv_csrrw_check() function should generate virtual instruction
> trap instead illegal instruction trap.
>
> Fixes: 533c91e8f22c ("target/riscv: Use RISCVException enum for
> CSR access")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/csr.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3500e07f92..2bf0a97196 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3139,7 +3139,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> int read_only = get_field(csrno, 0xC00) == 3;
> int csr_min_priv = csr_ops[csrno].min_priv_ver;
> #if !defined(CONFIG_USER_ONLY)
> - int effective_priv = env->priv;
> + int csr_priv, effective_priv = env->priv;
>
> if (riscv_has_ext(env, RVH) &&
> env->priv == PRV_S &&
> @@ -3152,7 +3152,11 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> effective_priv++;
> }
>
> - if (!env->debugger && (effective_priv < get_field(csrno, 0x300))) {
> + csr_priv = get_field(csrno, 0x300);
> + if (!env->debugger && (effective_priv < csr_priv)) {
> + if (csr_priv == (PRV_S + 1) && riscv_cpu_virt_enabled(env)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> return RISCV_EXCP_ILLEGAL_INST;
> }
> #endif
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] target/riscv: Set [m|s]tval for both illegal and virtual instruction traps
2022-04-29 3:34 ` Anup Patel
@ 2022-04-30 3:16 ` Frank Chang
-1 siblings, 0 replies; 22+ messages in thread
From: Frank Chang @ 2022-04-30 3:16 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, open list:RISC-V, Sagar Karandikar, Anup Patel,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 5105 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:36 AM Anup Patel <apatel@ventanamicro.com> wrote:
> Currently, the [m|s]tval CSRs are set with trapping instruction encoding
> only for illegal instruction traps taken at the time of instruction
> decoding.
>
> In RISC-V world, a valid instructions might also trap as illegal or
> virtual instruction based to trapping bits in various CSRs (such as
> mstatus.TVM or hstatus.VTVM).
>
> We improve setting of [m|s]tval CSRs for all types of illegal and
> virtual instruction traps.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu.h | 8 +++++++-
> target/riscv/cpu_helper.c | 1 +
> target/riscv/translate.c | 17 +++++++++++++----
> 4 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index dff4606585..f0a702fee6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -406,6 +406,7 @@ void restore_state_to_opc(CPURISCVState *env,
> TranslationBlock *tb,
> } else {
> env->pc = data[0];
> }
> + env->bins = data[1];
> }
>
> static void riscv_cpu_reset(DeviceState *dev)
> @@ -445,6 +446,7 @@ static void riscv_cpu_reset(DeviceState *dev)
> env->mcause = 0;
> env->miclaim = MIP_SGEIP;
> env->pc = env->resetvec;
> + env->bins = 0;
> env->two_stage_lookup = false;
>
> /* Initialized default priorities of local interrupts. */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index fe6c9a2c92..a55c918274 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -30,6 +30,12 @@
>
> #define TCG_GUEST_DEFAULT_MO 0
>
> +/*
> + * RISC-V-specific extra insn start words:
> + * 1: Original instruction opcode
> + */
> +#define TARGET_INSN_START_EXTRA_WORDS 1
> +
> #define TYPE_RISCV_CPU "riscv-cpu"
>
> #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
> @@ -140,7 +146,7 @@ struct CPUArchState {
> target_ulong frm;
>
> target_ulong badaddr;
> - uint32_t bins;
> + target_ulong bins;
>
> target_ulong guest_phys_fault_addr;
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index d83579accf..bba4fce777 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1371,6 +1371,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> tval = env->badaddr;
> break;
> case RISCV_EXCP_ILLEGAL_INST:
> + case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
> tval = env->bins;
> break;
> default:
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0cd1d9ee94..55a4713af2 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -107,6 +107,8 @@ typedef struct DisasContext {
> /* PointerMasking extension */
> bool pm_mask_enabled;
> bool pm_base_enabled;
> + /* TCG of the current insn_start */
> + TCGOp *insn_start;
> } DisasContext;
>
> static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -236,9 +238,6 @@ static void generate_exception_mtval(DisasContext
> *ctx, int excp)
>
> static void gen_exception_illegal(DisasContext *ctx)
> {
> - tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
> - offsetof(CPURISCVState, bins));
> -
> generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
> }
>
> @@ -1017,6 +1016,13 @@ static uint32_t opcode_at(DisasContextBase *dcbase,
> target_ulong pc)
> /* Include decoders for factored-out extensions */
> #include "decode-XVentanaCondOps.c.inc"
>
> +static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
> +{
> + assert(ctx->insn_start != NULL);
> + tcg_set_insn_start_param(ctx->insn_start, 1, opc);
> + ctx->insn_start = NULL;
> +}
> +
> static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t
> opcode)
> {
> /*
> @@ -1033,6 +1039,7 @@ static void decode_opc(CPURISCVState *env,
> DisasContext *ctx, uint16_t opcode)
>
> /* Check for compressed insn */
> if (extract16(opcode, 0, 2) != 3) {
> + decode_save_opc(ctx, opcode);
> if (!has_ext(ctx, RVC)) {
> gen_exception_illegal(ctx);
> } else {
> @@ -1047,6 +1054,7 @@ static void decode_opc(CPURISCVState *env,
> DisasContext *ctx, uint16_t opcode)
> opcode32 = deposit32(opcode32, 16, 16,
> translator_lduw(env, &ctx->base,
> ctx->base.pc_next + 2));
> + decode_save_opc(ctx, opcode32);
> ctx->opcode = opcode32;
> ctx->pc_succ_insn = ctx->base.pc_next + 4;
>
> @@ -1113,7 +1121,8 @@ static void riscv_tr_insn_start(DisasContextBase
> *dcbase, CPUState *cpu)
> {
> DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
> - tcg_gen_insn_start(ctx->base.pc_next);
> + tcg_gen_insn_start(ctx->base.pc_next, 0);
> + ctx->insn_start = tcg_last_op();
> }
>
> static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState
> *cpu)
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 6312 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] target/riscv: Set [m|s]tval for both illegal and virtual instruction traps
@ 2022-04-30 3:16 ` Frank Chang
0 siblings, 0 replies; 22+ messages in thread
From: Frank Chang @ 2022-04-30 3:16 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
[-- Attachment #1: Type: text/plain, Size: 5105 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:36 AM Anup Patel <apatel@ventanamicro.com> wrote:
> Currently, the [m|s]tval CSRs are set with trapping instruction encoding
> only for illegal instruction traps taken at the time of instruction
> decoding.
>
> In RISC-V world, a valid instructions might also trap as illegal or
> virtual instruction based to trapping bits in various CSRs (such as
> mstatus.TVM or hstatus.VTVM).
>
> We improve setting of [m|s]tval CSRs for all types of illegal and
> virtual instruction traps.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu.h | 8 +++++++-
> target/riscv/cpu_helper.c | 1 +
> target/riscv/translate.c | 17 +++++++++++++----
> 4 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index dff4606585..f0a702fee6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -406,6 +406,7 @@ void restore_state_to_opc(CPURISCVState *env,
> TranslationBlock *tb,
> } else {
> env->pc = data[0];
> }
> + env->bins = data[1];
> }
>
> static void riscv_cpu_reset(DeviceState *dev)
> @@ -445,6 +446,7 @@ static void riscv_cpu_reset(DeviceState *dev)
> env->mcause = 0;
> env->miclaim = MIP_SGEIP;
> env->pc = env->resetvec;
> + env->bins = 0;
> env->two_stage_lookup = false;
>
> /* Initialized default priorities of local interrupts. */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index fe6c9a2c92..a55c918274 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -30,6 +30,12 @@
>
> #define TCG_GUEST_DEFAULT_MO 0
>
> +/*
> + * RISC-V-specific extra insn start words:
> + * 1: Original instruction opcode
> + */
> +#define TARGET_INSN_START_EXTRA_WORDS 1
> +
> #define TYPE_RISCV_CPU "riscv-cpu"
>
> #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
> @@ -140,7 +146,7 @@ struct CPUArchState {
> target_ulong frm;
>
> target_ulong badaddr;
> - uint32_t bins;
> + target_ulong bins;
>
> target_ulong guest_phys_fault_addr;
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index d83579accf..bba4fce777 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1371,6 +1371,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> tval = env->badaddr;
> break;
> case RISCV_EXCP_ILLEGAL_INST:
> + case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
> tval = env->bins;
> break;
> default:
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0cd1d9ee94..55a4713af2 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -107,6 +107,8 @@ typedef struct DisasContext {
> /* PointerMasking extension */
> bool pm_mask_enabled;
> bool pm_base_enabled;
> + /* TCG of the current insn_start */
> + TCGOp *insn_start;
> } DisasContext;
>
> static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -236,9 +238,6 @@ static void generate_exception_mtval(DisasContext
> *ctx, int excp)
>
> static void gen_exception_illegal(DisasContext *ctx)
> {
> - tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
> - offsetof(CPURISCVState, bins));
> -
> generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
> }
>
> @@ -1017,6 +1016,13 @@ static uint32_t opcode_at(DisasContextBase *dcbase,
> target_ulong pc)
> /* Include decoders for factored-out extensions */
> #include "decode-XVentanaCondOps.c.inc"
>
> +static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
> +{
> + assert(ctx->insn_start != NULL);
> + tcg_set_insn_start_param(ctx->insn_start, 1, opc);
> + ctx->insn_start = NULL;
> +}
> +
> static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t
> opcode)
> {
> /*
> @@ -1033,6 +1039,7 @@ static void decode_opc(CPURISCVState *env,
> DisasContext *ctx, uint16_t opcode)
>
> /* Check for compressed insn */
> if (extract16(opcode, 0, 2) != 3) {
> + decode_save_opc(ctx, opcode);
> if (!has_ext(ctx, RVC)) {
> gen_exception_illegal(ctx);
> } else {
> @@ -1047,6 +1054,7 @@ static void decode_opc(CPURISCVState *env,
> DisasContext *ctx, uint16_t opcode)
> opcode32 = deposit32(opcode32, 16, 16,
> translator_lduw(env, &ctx->base,
> ctx->base.pc_next + 2));
> + decode_save_opc(ctx, opcode32);
> ctx->opcode = opcode32;
> ctx->pc_succ_insn = ctx->base.pc_next + 4;
>
> @@ -1113,7 +1121,8 @@ static void riscv_tr_insn_start(DisasContextBase
> *dcbase, CPUState *cpu)
> {
> DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
> - tcg_gen_insn_start(ctx->base.pc_next);
> + tcg_gen_insn_start(ctx->base.pc_next, 0);
> + ctx->insn_start = tcg_last_op();
> }
>
> static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState
> *cpu)
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 6312 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] target/riscv: Fix csr number based privilege checking
2022-04-29 3:34 ` Anup Patel
@ 2022-04-30 3:19 ` Frank Chang
-1 siblings, 0 replies; 22+ messages in thread
From: Frank Chang @ 2022-04-30 3:19 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, open list:RISC-V, Sagar Karandikar, Anup Patel,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:34 AM Anup Patel <apatel@ventanamicro.com> wrote:
> When hypervisor and VS CSRs are accessed from VS-mode or VU-mode,
> the riscv_csrrw_check() function should generate virtual instruction
> trap instead illegal instruction trap.
>
> Fixes: 533c91e8f22c ("target/riscv: Use RISCVException enum for
> CSR access")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/csr.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3500e07f92..2bf0a97196 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3139,7 +3139,7 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
> int read_only = get_field(csrno, 0xC00) == 3;
> int csr_min_priv = csr_ops[csrno].min_priv_ver;
> #if !defined(CONFIG_USER_ONLY)
> - int effective_priv = env->priv;
> + int csr_priv, effective_priv = env->priv;
>
> if (riscv_has_ext(env, RVH) &&
> env->priv == PRV_S &&
> @@ -3152,7 +3152,11 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
> effective_priv++;
> }
>
> - if (!env->debugger && (effective_priv < get_field(csrno, 0x300))) {
> + csr_priv = get_field(csrno, 0x300);
> + if (!env->debugger && (effective_priv < csr_priv)) {
> + if (csr_priv == (PRV_S + 1) && riscv_cpu_virt_enabled(env)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> return RISCV_EXCP_ILLEGAL_INST;
> }
> #endif
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2384 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] target/riscv: Fix csr number based privilege checking
@ 2022-04-30 3:19 ` Frank Chang
0 siblings, 0 replies; 22+ messages in thread
From: Frank Chang @ 2022-04-30 3:19 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:34 AM Anup Patel <apatel@ventanamicro.com> wrote:
> When hypervisor and VS CSRs are accessed from VS-mode or VU-mode,
> the riscv_csrrw_check() function should generate virtual instruction
> trap instead illegal instruction trap.
>
> Fixes: 533c91e8f22c ("target/riscv: Use RISCVException enum for
> CSR access")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/csr.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3500e07f92..2bf0a97196 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3139,7 +3139,7 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
> int read_only = get_field(csrno, 0xC00) == 3;
> int csr_min_priv = csr_ops[csrno].min_priv_ver;
> #if !defined(CONFIG_USER_ONLY)
> - int effective_priv = env->priv;
> + int csr_priv, effective_priv = env->priv;
>
> if (riscv_has_ext(env, RVH) &&
> env->priv == PRV_S &&
> @@ -3152,7 +3152,11 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
> effective_priv++;
> }
>
> - if (!env->debugger && (effective_priv < get_field(csrno, 0x300))) {
> + csr_priv = get_field(csrno, 0x300);
> + if (!env->debugger && (effective_priv < csr_priv)) {
> + if (csr_priv == (PRV_S + 1) && riscv_cpu_virt_enabled(env)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> return RISCV_EXCP_ILLEGAL_INST;
> }
> #endif
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2384 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
2022-04-29 3:34 ` Anup Patel
(?)
@ 2022-05-05 9:51 ` Alistair Francis
2022-05-05 10:36 ` Anup Patel
-1 siblings, 1 reply; 22+ messages in thread
From: Alistair Francis @ 2022-05-05 9:51 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Fri, Apr 29, 2022 at 1:38 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Currently, QEMU does not set hstatus.GVA bit for traps taken from
> HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
> on QEMU. This was working previously.
>
> This patch updates riscv_cpu_do_interrupt() to fix the above issue.
>
> Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu_helper.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index e1aa4f2097..d83579accf 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> /* Trap into HS mode */
> env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> htval = env->guest_phys_fault_addr;
> - write_gva = false;
This doesn't seem right.
"Field GVA (Guest Virtual Address) is written by the implementation
whenever a trap is taken
into HS-mode. For any trap (breakpoint, address misaligned, access
fault, page fault, or guest-
page fault) that writes a guest virtual address to stval, GVA is set
to 1. For any other trap into
HS-mode, GVA is set to 0"
So if we are trapping from HS to HS, the address in stval should not
be a guest virtual address, at least in general.
We probably aren't correctly setting GVA if MPRV is set though, as
then the page faults should be guest addresses. That's probably the
issue you are seeing.
Alistair
> }
> env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva);
> }
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
2022-05-05 9:51 ` Alistair Francis
@ 2022-05-05 10:36 ` Anup Patel
2022-05-09 9:23 ` Alistair Francis
0 siblings, 1 reply; 22+ messages in thread
From: Anup Patel @ 2022-05-05 10:36 UTC (permalink / raw)
To: Alistair Francis
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Thu, May 5, 2022 at 3:21 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Apr 29, 2022 at 1:38 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Currently, QEMU does not set hstatus.GVA bit for traps taken from
> > HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
> > on QEMU. This was working previously.
> >
> > This patch updates riscv_cpu_do_interrupt() to fix the above issue.
> >
> > Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > target/riscv/cpu_helper.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index e1aa4f2097..d83579accf 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> > /* Trap into HS mode */
> > env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> > htval = env->guest_phys_fault_addr;
> > - write_gva = false;
>
> This doesn't seem right.
>
> "Field GVA (Guest Virtual Address) is written by the implementation
> whenever a trap is taken
> into HS-mode. For any trap (breakpoint, address misaligned, access
> fault, page fault, or guest-
> page fault) that writes a guest virtual address to stval, GVA is set
> to 1. For any other trap into
> HS-mode, GVA is set to 0"
>
> So if we are trapping from HS to HS, the address in stval should not
> be a guest virtual address, at least in general.
That's not correct. The HLV/HSV instructions executed by hypervisor
(HS-mode) take guest virtual address. These instructions can trap
from HS-mode to HS-mode.
>
> We probably aren't correctly setting GVA if MPRV is set though, as
> then the page faults should be guest addresses. That's probably the
> issue you are seeing.
The Xvisor nested MMU test-suit is broken on QEMU because it
uses HLV/HSV instructions in HS-mode.
Regards,
Anup
>
> Alistair
>
> > }
> > env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva);
> > }
> > --
> > 2.34.1
> >
> >
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
2022-05-05 10:36 ` Anup Patel
@ 2022-05-09 9:23 ` Alistair Francis
2022-05-09 12:00 ` Anup Patel
0 siblings, 1 reply; 22+ messages in thread
From: Alistair Francis @ 2022-05-09 9:23 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Thu, May 5, 2022 at 12:36 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Thu, May 5, 2022 at 3:21 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Fri, Apr 29, 2022 at 1:38 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > Currently, QEMU does not set hstatus.GVA bit for traps taken from
> > > HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
> > > on QEMU. This was working previously.
> > >
> > > This patch updates riscv_cpu_do_interrupt() to fix the above issue.
> > >
> > > Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > ---
> > > target/riscv/cpu_helper.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index e1aa4f2097..d83579accf 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> > > /* Trap into HS mode */
> > > env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> > > htval = env->guest_phys_fault_addr;
> > > - write_gva = false;
> >
> > This doesn't seem right.
> >
> > "Field GVA (Guest Virtual Address) is written by the implementation
> > whenever a trap is taken
> > into HS-mode. For any trap (breakpoint, address misaligned, access
> > fault, page fault, or guest-
> > page fault) that writes a guest virtual address to stval, GVA is set
> > to 1. For any other trap into
> > HS-mode, GVA is set to 0"
> >
> > So if we are trapping from HS to HS, the address in stval should not
> > be a guest virtual address, at least in general.
>
> That's not correct. The HLV/HSV instructions executed by hypervisor
> (HS-mode) take guest virtual address. These instructions can trap
> from HS-mode to HS-mode.
Ah, I forgot about those instructions, but still they are the
exception. In general we would expect a trap from HS to HS to contain
HS addresses. We should just handle the other cases specially
Alistair
>
> >
> > We probably aren't correctly setting GVA if MPRV is set though, as
> > then the page faults should be guest addresses. That's probably the
> > issue you are seeing.
>
> The Xvisor nested MMU test-suit is broken on QEMU because it
> uses HLV/HSV instructions in HS-mode.
>
> Regards,
> Anup
>
> >
> > Alistair
> >
> > > }
> > > env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva);
> > > }
> > > --
> > > 2.34.1
> > >
> > >
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] target/riscv: Set [m|s]tval for both illegal and virtual instruction traps
2022-04-29 3:34 ` Anup Patel
(?)
(?)
@ 2022-05-09 9:36 ` Alistair Francis
-1 siblings, 0 replies; 22+ messages in thread
From: Alistair Francis @ 2022-05-09 9:36 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Fri, Apr 29, 2022 at 5:36 AM Anup Patel <apatel@ventanamicro.com> wrote:>
> Currently, the [m|s]tval CSRs are set with trapping instruction encoding
> only for illegal instruction traps taken at the time of instruction
> decoding.
>
> In RISC-V world, a valid instructions might also trap as illegal or
> virtual instruction based to trapping bits in various CSRs (such as
> mstatus.TVM or hstatus.VTVM).
>
> We improve setting of [m|s]tval CSRs for all types of illegal and
> virtual instruction traps.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu.h | 8 +++++++-
> target/riscv/cpu_helper.c | 1 +
> target/riscv/translate.c | 17 +++++++++++++----
> 4 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index dff4606585..f0a702fee6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -406,6 +406,7 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
> } else {
> env->pc = data[0];
> }
> + env->bins = data[1];
> }
>
> static void riscv_cpu_reset(DeviceState *dev)
> @@ -445,6 +446,7 @@ static void riscv_cpu_reset(DeviceState *dev)
> env->mcause = 0;
> env->miclaim = MIP_SGEIP;
> env->pc = env->resetvec;
> + env->bins = 0;
> env->two_stage_lookup = false;
>
> /* Initialized default priorities of local interrupts. */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index fe6c9a2c92..a55c918274 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -30,6 +30,12 @@
>
> #define TCG_GUEST_DEFAULT_MO 0
>
> +/*
> + * RISC-V-specific extra insn start words:
> + * 1: Original instruction opcode
> + */
> +#define TARGET_INSN_START_EXTRA_WORDS 1
> +
> #define TYPE_RISCV_CPU "riscv-cpu"
>
> #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
> @@ -140,7 +146,7 @@ struct CPUArchState {
> target_ulong frm;
>
> target_ulong badaddr;
> - uint32_t bins;
> + target_ulong bins;
>
> target_ulong guest_phys_fault_addr;
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index d83579accf..bba4fce777 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1371,6 +1371,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> tval = env->badaddr;
> break;
> case RISCV_EXCP_ILLEGAL_INST:
> + case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
> tval = env->bins;
> break;
> default:
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0cd1d9ee94..55a4713af2 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -107,6 +107,8 @@ typedef struct DisasContext {
> /* PointerMasking extension */
> bool pm_mask_enabled;
> bool pm_base_enabled;
> + /* TCG of the current insn_start */
> + TCGOp *insn_start;
> } DisasContext;
>
> static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -236,9 +238,6 @@ static void generate_exception_mtval(DisasContext *ctx, int excp)
>
> static void gen_exception_illegal(DisasContext *ctx)
> {
> - tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
> - offsetof(CPURISCVState, bins));
> -
> generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
> }
>
> @@ -1017,6 +1016,13 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> /* Include decoders for factored-out extensions */
> #include "decode-XVentanaCondOps.c.inc"
>
> +static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
> +{
> + assert(ctx->insn_start != NULL);
> + tcg_set_insn_start_param(ctx->insn_start, 1, opc);
> + ctx->insn_start = NULL;
> +}
> +
> static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> {
> /*
> @@ -1033,6 +1039,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>
> /* Check for compressed insn */
> if (extract16(opcode, 0, 2) != 3) {
> + decode_save_opc(ctx, opcode);
> if (!has_ext(ctx, RVC)) {
> gen_exception_illegal(ctx);
> } else {
> @@ -1047,6 +1054,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> opcode32 = deposit32(opcode32, 16, 16,
> translator_lduw(env, &ctx->base,
> ctx->base.pc_next + 2));
> + decode_save_opc(ctx, opcode32);
> ctx->opcode = opcode32;
> ctx->pc_succ_insn = ctx->base.pc_next + 4;
>
> @@ -1113,7 +1121,8 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
> {
> DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
> - tcg_gen_insn_start(ctx->base.pc_next);
> + tcg_gen_insn_start(ctx->base.pc_next, 0);
> + ctx->insn_start = tcg_last_op();
> }
>
> static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode
2022-05-09 9:23 ` Alistair Francis
@ 2022-05-09 12:00 ` Anup Patel
0 siblings, 0 replies; 22+ messages in thread
From: Anup Patel @ 2022-05-09 12:00 UTC (permalink / raw)
To: Alistair Francis
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Mon, May 9, 2022 at 2:54 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, May 5, 2022 at 12:36 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Thu, May 5, 2022 at 3:21 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Fri, Apr 29, 2022 at 1:38 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > > >
> > > > Currently, QEMU does not set hstatus.GVA bit for traps taken from
> > > > HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
> > > > on QEMU. This was working previously.
> > > >
> > > > This patch updates riscv_cpu_do_interrupt() to fix the above issue.
> > > >
> > > > Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > > ---
> > > > target/riscv/cpu_helper.c | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > index e1aa4f2097..d83579accf 100644
> > > > --- a/target/riscv/cpu_helper.c
> > > > +++ b/target/riscv/cpu_helper.c
> > > > @@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> > > > /* Trap into HS mode */
> > > > env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> > > > htval = env->guest_phys_fault_addr;
> > > > - write_gva = false;
> > >
> > > This doesn't seem right.
> > >
> > > "Field GVA (Guest Virtual Address) is written by the implementation
> > > whenever a trap is taken
> > > into HS-mode. For any trap (breakpoint, address misaligned, access
> > > fault, page fault, or guest-
> > > page fault) that writes a guest virtual address to stval, GVA is set
> > > to 1. For any other trap into
> > > HS-mode, GVA is set to 0"
> > >
> > > So if we are trapping from HS to HS, the address in stval should not
> > > be a guest virtual address, at least in general.
> >
> > That's not correct. The HLV/HSV instructions executed by hypervisor
> > (HS-mode) take guest virtual address. These instructions can trap
> > from HS-mode to HS-mode.
>
> Ah, I forgot about those instructions, but still they are the
> exception. In general we would expect a trap from HS to HS to contain
> HS addresses. We should just handle the other cases specially
I see your point. Let me re-work this patch to ensure that the GVA bit
is only set when we have a guest virtual address.
Regards,
Anup
>
> Alistair
>
> >
> > >
> > > We probably aren't correctly setting GVA if MPRV is set though, as
> > > then the page faults should be guest addresses. That's probably the
> > > issue you are seeing.
> >
> > The Xvisor nested MMU test-suit is broken on QEMU because it
> > uses HLV/HSV instructions in HS-mode.
> >
> > Regards,
> > Anup
> >
> > >
> > > Alistair
> > >
> > > > }
> > > > env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva);
> > > > }
> > > > --
> > > > 2.34.1
> > > >
> > > >
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] target/riscv: Fix csr number based privilege checking
2022-04-30 3:19 ` Frank Chang
(?)
@ 2022-05-09 19:13 ` Atish Patra
-1 siblings, 0 replies; 22+ messages in thread
From: Atish Patra @ 2022-05-09 19:13 UTC (permalink / raw)
To: Frank Chang
Cc: Anup Patel, Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers
[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]
On Fri, Apr 29, 2022 at 8:20 PM Frank Chang <frank.chang@sifive.com> wrote:
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
>
> On Fri, Apr 29, 2022 at 11:34 AM Anup Patel <apatel@ventanamicro.com>
> wrote:
>
>> When hypervisor and VS CSRs are accessed from VS-mode or VU-mode,
>> the riscv_csrrw_check() function should generate virtual instruction
>> trap instead illegal instruction trap.
>>
>> Fixes: 533c91e8f22c ("target/riscv: Use RISCVException enum for
>> CSR access")
>>
>
This is not the correct Fixes tag. This patch just changed the error code
to enum.
The above said issue exists before this patch.
I think the correct fix should be 0a42f4c44088 (" target/riscv: Fix CSR
perm checking for HS mode"). No ?
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> ---
>> target/riscv/csr.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index 3500e07f92..2bf0a97196 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -3139,7 +3139,7 @@ static inline RISCVException
>> riscv_csrrw_check(CPURISCVState *env,
>> int read_only = get_field(csrno, 0xC00) == 3;
>> int csr_min_priv = csr_ops[csrno].min_priv_ver;
>> #if !defined(CONFIG_USER_ONLY)
>> - int effective_priv = env->priv;
>> + int csr_priv, effective_priv = env->priv;
>>
>> if (riscv_has_ext(env, RVH) &&
>> env->priv == PRV_S &&
>> @@ -3152,7 +3152,11 @@ static inline RISCVException
>> riscv_csrrw_check(CPURISCVState *env,
>> effective_priv++;
>> }
>>
>> - if (!env->debugger && (effective_priv < get_field(csrno, 0x300))) {
>> + csr_priv = get_field(csrno, 0x300);
>> + if (!env->debugger && (effective_priv < csr_priv)) {
>> + if (csr_priv == (PRV_S + 1) && riscv_cpu_virt_enabled(env)) {
>> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> + }
>> return RISCV_EXCP_ILLEGAL_INST;
>> }
>> #endif
>> --
>> 2.34.1
>>
>>
>>
--
Regards,
Atish
[-- Attachment #2: Type: text/html, Size: 3545 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2022-05-09 19:14 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-29 3:34 [PATCH 0/4] QEMU RISC-V nested virtualization fixes Anup Patel
2022-04-29 3:34 ` Anup Patel
2022-04-29 3:34 ` [PATCH 1/4] target/riscv: Fix csr number based privilege checking Anup Patel
2022-04-29 3:34 ` Anup Patel
2022-04-29 10:54 ` Alistair Francis
2022-04-29 10:54 ` Alistair Francis
2022-04-30 3:19 ` Frank Chang
2022-04-30 3:19 ` Frank Chang
2022-05-09 19:13 ` Atish Patra
2022-04-29 3:34 ` [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode Anup Patel
2022-04-29 3:34 ` Anup Patel
2022-05-05 9:51 ` Alistair Francis
2022-05-05 10:36 ` Anup Patel
2022-05-09 9:23 ` Alistair Francis
2022-05-09 12:00 ` Anup Patel
2022-04-29 3:34 ` [PATCH 3/4] target/riscv: Set [m|s]tval for both illegal and virtual instruction traps Anup Patel
2022-04-29 3:34 ` Anup Patel
2022-04-30 3:16 ` Frank Chang
2022-04-30 3:16 ` Frank Chang
2022-05-09 9:36 ` Alistair Francis
2022-04-29 3:34 ` [PATCH 4/4] target/riscv: Update [m|h]tinst CSR in riscv_cpu_do_interrupt() Anup Patel
2022-04-29 3:34 ` Anup Patel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.