* [PATCH v1 1/5] target/riscv: Add a virtualised MMU Mode
2020-10-23 15:25 [PATCH v1 0/5] Fix the Hypervisor access functions Alistair Francis
@ 2020-10-23 15:26 ` Alistair Francis
2020-10-23 19:13 ` Richard Henderson
2020-10-23 15:26 ` [PATCH v1 2/5] target/riscv: Set the virtualised MMU mode when doing hyp accesses Alistair Francis
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Alistair Francis @ 2020-10-23 15:26 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: alistair.francis, richard.henderson, bmeng.cn, palmer, alistair23
Add a new MMU mode that includes the current virt mode.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/cpu-param.h | 2 +-
target/riscv/cpu.h | 4 +++-
target/riscv/cpu_helper.c | 6 +++++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/target/riscv/cpu-param.h b/target/riscv/cpu-param.h
index 664fc1d371..502ed6489e 100644
--- a/target/riscv/cpu-param.h
+++ b/target/riscv/cpu-param.h
@@ -18,6 +18,6 @@
# define TARGET_VIRT_ADDR_SPACE_BITS 32 /* sv32 */
#endif
#define TARGET_PAGE_BITS 12 /* 4 KiB Pages */
-#define NB_MMU_MODES 4
+#define NB_MMU_MODES 8
#endif
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de4705bb57..8ac01f3a64 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -365,7 +365,9 @@ void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env,
target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
-#define TB_FLAGS_MMU_MASK 3
+#define TB_FLAGS_MMU_MASK 7
+#define TB_FLAGS_PRIV_MMU_MASK 3
+#define TB_FLAGS_PRIV_HYP_ACCESS_MASK (1 << 2)
#define TB_FLAGS_MSTATUS_FS MSTATUS_FS
typedef CPURISCVState CPUArchState;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 4652082df1..46b62a0f37 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -30,6 +30,10 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
#ifdef CONFIG_USER_ONLY
return 0;
#else
+ if (riscv_cpu_virt_enabled(env)) {
+ return env->priv | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+ }
+
return env->priv;
#endif
}
@@ -336,7 +340,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
* (riscv_cpu_do_interrupt) is correct */
MemTxResult res;
MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
- int mode = mmu_idx;
+ int mode = mmu_idx & 0x3;
bool use_background = false;
/*
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/5] target/riscv: Add a virtualised MMU Mode
2020-10-23 15:26 ` [PATCH v1 1/5] target/riscv: Add a virtualised MMU Mode Alistair Francis
@ 2020-10-23 19:13 ` Richard Henderson
2020-10-23 23:29 ` Alistair Francis
0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2020-10-23 19:13 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer
On 10/23/20 8:26 AM, Alistair Francis wrote:
> +++ b/target/riscv/cpu-param.h
> @@ -18,6 +18,6 @@
> # define TARGET_VIRT_ADDR_SPACE_BITS 32 /* sv32 */
> #endif
> #define TARGET_PAGE_BITS 12 /* 4 KiB Pages */
> -#define NB_MMU_MODES 4
> +#define NB_MMU_MODES 8
Is there really a PRV_M + virt enabled state?
> +#define TB_FLAGS_PRIV_MMU_MASK 3
...
> - int mode = mmu_idx;
> + int mode = mmu_idx & 0x3;
Use that MASK here?
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/5] target/riscv: Add a virtualised MMU Mode
2020-10-23 19:13 ` Richard Henderson
@ 2020-10-23 23:29 ` Alistair Francis
0 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-10-23 23:29 UTC (permalink / raw)
To: Richard Henderson
Cc: open list:RISC-V, Palmer Dabbelt, Bin Meng, Alistair Francis,
qemu-devel@nongnu.org Developers
On Fri, Oct 23, 2020 at 12:13 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 10/23/20 8:26 AM, Alistair Francis wrote:
> > +++ b/target/riscv/cpu-param.h
> > @@ -18,6 +18,6 @@
> > # define TARGET_VIRT_ADDR_SPACE_BITS 32 /* sv32 */
> > #endif
> > #define TARGET_PAGE_BITS 12 /* 4 KiB Pages */
> > -#define NB_MMU_MODES 4
> > +#define NB_MMU_MODES 8
>
> Is there really a PRV_M + virt enabled state?
No, there isn't.
>
> > +#define TB_FLAGS_PRIV_MMU_MASK 3
> ...
> > - int mode = mmu_idx;
> > + int mode = mmu_idx & 0x3;
>
> Use that MASK here?
Good idea.
Alistair
>
>
> r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 2/5] target/riscv: Set the virtualised MMU mode when doing hyp accesses
2020-10-23 15:25 [PATCH v1 0/5] Fix the Hypervisor access functions Alistair Francis
2020-10-23 15:26 ` [PATCH v1 1/5] target/riscv: Add a virtualised MMU Mode Alistair Francis
@ 2020-10-23 15:26 ` Alistair Francis
2020-10-23 15:26 ` [PATCH v1 3/5] target/riscv: Remove the HS_TWO_STAGE flag Alistair Francis
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-10-23 15:26 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: alistair.francis, richard.henderson, bmeng.cn, palmer, alistair23
When performing the hypervisor load/store operations set the MMU mode to
indicate that we are virtualised.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/op_helper.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 4ce73575a7..bc6df6c8c9 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -238,30 +238,31 @@ target_ulong helper_hyp_load(CPURISCVState *env, target_ulong address,
(env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
get_field(env->hstatus, HSTATUS_HU))) {
target_ulong pte;
+ int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
riscv_cpu_set_two_stage_lookup(env, true);
switch (memop) {
case MO_SB:
- pte = cpu_ldsb_data_ra(env, address, GETPC());
+ pte = cpu_ldsb_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
case MO_UB:
- pte = cpu_ldub_data_ra(env, address, GETPC());
+ pte = cpu_ldub_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
case MO_TESW:
- pte = cpu_ldsw_data_ra(env, address, GETPC());
+ pte = cpu_ldsw_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
case MO_TEUW:
- pte = cpu_lduw_data_ra(env, address, GETPC());
+ pte = cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
case MO_TESL:
- pte = cpu_ldl_data_ra(env, address, GETPC());
+ pte = cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
case MO_TEUL:
- pte = cpu_ldl_data_ra(env, address, GETPC());
+ pte = cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
case MO_TEQ:
- pte = cpu_ldq_data_ra(env, address, GETPC());
+ pte = cpu_ldq_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
default:
g_assert_not_reached();
@@ -287,23 +288,25 @@ void helper_hyp_store(CPURISCVState *env, target_ulong address,
(env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
(env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
get_field(env->hstatus, HSTATUS_HU))) {
+ int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+
riscv_cpu_set_two_stage_lookup(env, true);
switch (memop) {
case MO_SB:
case MO_UB:
- cpu_stb_data_ra(env, address, val, GETPC());
+ cpu_stb_mmuidx_ra(env, address, val, mmu_idx, GETPC());
break;
case MO_TESW:
case MO_TEUW:
- cpu_stw_data_ra(env, address, val, GETPC());
+ cpu_stw_mmuidx_ra(env, address, val, mmu_idx, GETPC());
break;
case MO_TESL:
case MO_TEUL:
- cpu_stl_data_ra(env, address, val, GETPC());
+ cpu_stl_mmuidx_ra(env, address, val, mmu_idx, GETPC());
break;
case MO_TEQ:
- cpu_stq_data_ra(env, address, val, GETPC());
+ cpu_stq_mmuidx_ra(env, address, val, mmu_idx, GETPC());
break;
default:
g_assert_not_reached();
@@ -329,15 +332,16 @@ target_ulong helper_hyp_x_load(CPURISCVState *env, target_ulong address,
(env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
get_field(env->hstatus, HSTATUS_HU))) {
target_ulong pte;
+ int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
riscv_cpu_set_two_stage_lookup(env, true);
switch (memop) {
case MO_TEUW:
- pte = cpu_lduw_data_ra(env, address, GETPC());
+ pte = cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
case MO_TEUL:
- pte = cpu_ldl_data_ra(env, address, GETPC());
+ pte = cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
break;
default:
g_assert_not_reached();
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v1 3/5] target/riscv: Remove the HS_TWO_STAGE flag
2020-10-23 15:25 [PATCH v1 0/5] Fix the Hypervisor access functions Alistair Francis
2020-10-23 15:26 ` [PATCH v1 1/5] target/riscv: Add a virtualised MMU Mode Alistair Francis
2020-10-23 15:26 ` [PATCH v1 2/5] target/riscv: Set the virtualised MMU mode when doing hyp accesses Alistair Francis
@ 2020-10-23 15:26 ` Alistair Francis
2020-10-23 15:26 ` [PATCH v1 4/5] target/riscv: Remove the hyp load and store functions Alistair Francis
2020-10-23 15:26 ` [PATCH v1 5/5] target/riscv: Split the Hypervisor execute load helpers Alistair Francis
4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-10-23 15:26 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: alistair.francis, richard.henderson, bmeng.cn, palmer, alistair23
The HS_TWO_STAGE flag is no longer required as the MMU index contains
the information if we are performing a two stage access.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/cpu.h | 3 +-
target/riscv/cpu_bits.h | 1 -
target/riscv/cpu_helper.c | 61 ++++++++++++++++-----------------------
target/riscv/op_helper.c | 12 --------
4 files changed, 26 insertions(+), 51 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 8ac01f3a64..694f51ebd4 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -325,8 +325,7 @@ bool riscv_cpu_virt_enabled(CPURISCVState *env);
void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
bool riscv_cpu_force_hs_excep_enabled(CPURISCVState *env);
void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable);
-bool riscv_cpu_two_stage_lookup(CPURISCVState *env);
-void riscv_cpu_set_two_stage_lookup(CPURISCVState *env, bool enable);
+bool riscv_cpu_two_stage_lookup(int mmu_idx);
int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index bd36062877..20b6ec4ec5 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -480,7 +480,6 @@
* page table fault.
*/
#define FORCE_HS_EXCEP 2
-#define HS_TWO_STAGE 4
/* RV32 satp CSR field masks */
#define SATP32_MODE 0x80000000
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 46b62a0f37..a1f94ea518 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -224,22 +224,9 @@ void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable)
env->virt = set_field(env->virt, FORCE_HS_EXCEP, enable);
}
-bool riscv_cpu_two_stage_lookup(CPURISCVState *env)
+bool riscv_cpu_two_stage_lookup(int mmu_idx)
{
- if (!riscv_has_ext(env, RVH)) {
- return false;
- }
-
- return get_field(env->virt, HS_TWO_STAGE);
-}
-
-void riscv_cpu_set_two_stage_lookup(CPURISCVState *env, bool enable)
-{
- if (!riscv_has_ext(env, RVH)) {
- return;
- }
-
- env->virt = set_field(env->virt, HS_TWO_STAGE, enable);
+ return mmu_idx & TB_FLAGS_PRIV_HYP_ACCESS_MASK;
}
int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
@@ -350,7 +337,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
* was called. Background registers will be used if the guest has
* forced a two stage translation to be on (in HS or M mode).
*/
- if (riscv_cpu_two_stage_lookup(env) && access_type != MMU_INST_FETCH) {
+ if ((!riscv_cpu_virt_enabled(env) && riscv_cpu_two_stage_lookup(mmu_idx))
+ && access_type != MMU_INST_FETCH) {
use_background = true;
}
@@ -589,7 +577,7 @@ restart:
static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
MMUAccessType access_type, bool pmp_violation,
- bool first_stage)
+ bool first_stage, bool two_stage)
{
CPUState *cs = env_cpu(env);
int page_fault_exceptions;
@@ -612,8 +600,7 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
}
break;
case MMU_DATA_LOAD:
- if ((riscv_cpu_virt_enabled(env) || riscv_cpu_two_stage_lookup(env)) &&
- !first_stage) {
+ if (two_stage && !first_stage) {
cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
} else {
cs->exception_index = page_fault_exceptions ?
@@ -621,8 +608,7 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
}
break;
case MMU_DATA_STORE:
- if ((riscv_cpu_virt_enabled(env) || riscv_cpu_two_stage_lookup(env)) &&
- !first_stage) {
+ if (two_stage && !first_stage) {
cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
} else {
cs->exception_index = page_fault_exceptions ?
@@ -713,6 +699,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
int prot, prot2;
bool pmp_violation = false;
bool first_stage_error = true;
+ bool two_stage_lookup = false;
int ret = TRANSLATE_FAIL;
int mode = mmu_idx;
target_ulong tlb_size = 0;
@@ -732,11 +719,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
access_type != MMU_INST_FETCH &&
get_field(env->mstatus, MSTATUS_MPRV) &&
MSTATUS_MPV_ISSET(env)) {
- riscv_cpu_set_two_stage_lookup(env, true);
+ two_stage_lookup = true;
}
if (riscv_cpu_virt_enabled(env) ||
- (riscv_cpu_two_stage_lookup(env) && access_type != MMU_INST_FETCH)) {
+ ((riscv_cpu_two_stage_lookup(mmu_idx) || two_stage_lookup) &&
+ access_type != MMU_INST_FETCH)) {
/* Two stage lookup */
ret = get_physical_address(env, &pa, &prot, address,
&env->guest_phys_fault_addr, access_type,
@@ -799,14 +787,6 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
__func__, address, ret, pa, prot);
}
- /* We did the two stage lookup based on MPRV, unset the lookup */
- if (riscv_has_ext(env, RVH) && env->priv == PRV_M &&
- access_type != MMU_INST_FETCH &&
- get_field(env->mstatus, MSTATUS_MPRV) &&
- MSTATUS_MPV_ISSET(env)) {
- riscv_cpu_set_two_stage_lookup(env, false);
- }
-
if (riscv_feature(env, RISCV_FEATURE_PMP) &&
(ret == TRANSLATE_SUCCESS) &&
!pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
@@ -828,7 +808,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
} else if (probe) {
return false;
} else {
- raise_mmu_exception(env, address, access_type, pmp_violation, first_stage_error);
+ raise_mmu_exception(env, address, access_type, pmp_violation,
+ first_stage_error,
+ riscv_cpu_virt_enabled(env) ||
+ riscv_cpu_two_stage_lookup(mmu_idx));
riscv_raise_exception(env, cs->exception_index, retaddr);
}
@@ -932,9 +915,16 @@ void riscv_cpu_do_interrupt(CPUState *cs)
/* handle the trap in S-mode */
if (riscv_has_ext(env, RVH)) {
target_ulong hdeleg = async ? env->hideleg : env->hedeleg;
+ bool two_stage_lookup = false;
+
+ if (env->priv == PRV_M ||
+ (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
+ (env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
+ get_field(env->hstatus, HSTATUS_HU))) {
+ two_stage_lookup = true;
+ }
- if ((riscv_cpu_virt_enabled(env) ||
- riscv_cpu_two_stage_lookup(env)) && write_tval) {
+ if ((riscv_cpu_virt_enabled(env) || two_stage_lookup) && write_tval) {
/*
* If we are writing a guest virtual address to stval, set
* this to 1. If we are trapping to VS we will set this to 0
@@ -972,11 +962,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
riscv_cpu_set_force_hs_excep(env, 0);
} else {
/* Trap into HS mode */
- if (!riscv_cpu_two_stage_lookup(env)) {
+ if (!two_stage_lookup) {
env->hstatus = set_field(env->hstatus, HSTATUS_SPV,
riscv_cpu_virt_enabled(env));
}
- riscv_cpu_set_two_stage_lookup(env, false);
htval = env->guest_phys_fault_addr;
}
}
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index bc6df6c8c9..556a23f031 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -240,8 +240,6 @@ target_ulong helper_hyp_load(CPURISCVState *env, target_ulong address,
target_ulong pte;
int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
- riscv_cpu_set_two_stage_lookup(env, true);
-
switch (memop) {
case MO_SB:
pte = cpu_ldsb_mmuidx_ra(env, address, mmu_idx, GETPC());
@@ -268,8 +266,6 @@ target_ulong helper_hyp_load(CPURISCVState *env, target_ulong address,
g_assert_not_reached();
}
- riscv_cpu_set_two_stage_lookup(env, false);
-
return pte;
}
@@ -290,8 +286,6 @@ void helper_hyp_store(CPURISCVState *env, target_ulong address,
get_field(env->hstatus, HSTATUS_HU))) {
int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
- riscv_cpu_set_two_stage_lookup(env, true);
-
switch (memop) {
case MO_SB:
case MO_UB:
@@ -312,8 +306,6 @@ void helper_hyp_store(CPURISCVState *env, target_ulong address,
g_assert_not_reached();
}
- riscv_cpu_set_two_stage_lookup(env, false);
-
return;
}
@@ -334,8 +326,6 @@ target_ulong helper_hyp_x_load(CPURISCVState *env, target_ulong address,
target_ulong pte;
int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
- riscv_cpu_set_two_stage_lookup(env, true);
-
switch (memop) {
case MO_TEUW:
pte = cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
@@ -347,8 +337,6 @@ target_ulong helper_hyp_x_load(CPURISCVState *env, target_ulong address,
g_assert_not_reached();
}
- riscv_cpu_set_two_stage_lookup(env, false);
-
return pte;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v1 4/5] target/riscv: Remove the hyp load and store functions
2020-10-23 15:25 [PATCH v1 0/5] Fix the Hypervisor access functions Alistair Francis
` (2 preceding siblings ...)
2020-10-23 15:26 ` [PATCH v1 3/5] target/riscv: Remove the HS_TWO_STAGE flag Alistair Francis
@ 2020-10-23 15:26 ` Alistair Francis
2020-10-23 15:26 ` [PATCH v1 5/5] target/riscv: Split the Hypervisor execute load helpers Alistair Francis
4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-10-23 15:26 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: alistair.francis, richard.henderson, bmeng.cn, palmer, alistair23
Remove the special Virtulisation load and store functions and just use
the standard tcg tcg_gen_qemu_ld_tl() and tcg_gen_qemu_st_tl() functions
instead.
As part of this change we ensure we still run an access check to make
sure we can perform the operations.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/helper.h | 3 +-
target/riscv/op_helper.c | 72 +--------------
target/riscv/insn_trans/trans_rvh.c.inc | 111 +++++++-----------------
3 files changed, 35 insertions(+), 151 deletions(-)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 4b690147fb..7dbdd117d2 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -81,8 +81,7 @@ DEF_HELPER_1(tlb_flush, void, env)
#ifndef CONFIG_USER_ONLY
DEF_HELPER_1(hyp_tlb_flush, void, env)
DEF_HELPER_1(hyp_gvma_tlb_flush, void, env)
-DEF_HELPER_4(hyp_load, tl, env, tl, tl, tl)
-DEF_HELPER_5(hyp_store, void, env, tl, tl, tl, tl)
+DEF_HELPER_1(hyp_access_check, void, env)
DEF_HELPER_4(hyp_x_load, tl, env, tl, tl, tl)
#endif
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 556a23f031..95281db3ec 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -230,82 +230,12 @@ void helper_hyp_gvma_tlb_flush(CPURISCVState *env)
helper_hyp_tlb_flush(env);
}
-target_ulong helper_hyp_load(CPURISCVState *env, target_ulong address,
- target_ulong attrs, target_ulong memop)
+void helper_hyp_access_check(CPURISCVState *env)
{
if (env->priv == PRV_M ||
(env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
(env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
get_field(env->hstatus, HSTATUS_HU))) {
- target_ulong pte;
- int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
-
- switch (memop) {
- case MO_SB:
- pte = cpu_ldsb_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- case MO_UB:
- pte = cpu_ldub_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- case MO_TESW:
- pte = cpu_ldsw_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- case MO_TEUW:
- pte = cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- case MO_TESL:
- pte = cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- case MO_TEUL:
- pte = cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- case MO_TEQ:
- pte = cpu_ldq_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- default:
- g_assert_not_reached();
- }
-
- return pte;
- }
-
- if (riscv_cpu_virt_enabled(env)) {
- riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
- } else {
- riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
- }
- return 0;
-}
-
-void helper_hyp_store(CPURISCVState *env, target_ulong address,
- target_ulong val, target_ulong attrs, target_ulong memop)
-{
- if (env->priv == PRV_M ||
- (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
- (env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
- get_field(env->hstatus, HSTATUS_HU))) {
- int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
-
- switch (memop) {
- case MO_SB:
- case MO_UB:
- cpu_stb_mmuidx_ra(env, address, val, mmu_idx, GETPC());
- break;
- case MO_TESW:
- case MO_TEUW:
- cpu_stw_mmuidx_ra(env, address, val, mmu_idx, GETPC());
- break;
- case MO_TESL:
- case MO_TEUL:
- cpu_stl_mmuidx_ra(env, address, val, mmu_idx, GETPC());
- break;
- case MO_TEQ:
- cpu_stq_mmuidx_ra(env, address, val, mmu_idx, GETPC());
- break;
- default:
- g_assert_not_reached();
- }
-
return;
}
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc
index 881c9ef4d2..79968701e9 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -22,20 +22,16 @@ static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_SB);
- gen_helper_hyp_load(t1, cpu_env, t0, mem_idx, memop);
+ tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_SB);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -48,20 +44,16 @@ static bool trans_hlv_h(DisasContext *ctx, arg_hlv_h *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TESW);
- gen_helper_hyp_load(t1, cpu_env, t0, mem_idx, memop);
+ tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TESW);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -74,20 +66,16 @@ static bool trans_hlv_w(DisasContext *ctx, arg_hlv_w *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TESL);
- gen_helper_hyp_load(t1, cpu_env, t0, mem_idx, memop);
+ tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TESL);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -100,20 +88,16 @@ static bool trans_hlv_bu(DisasContext *ctx, arg_hlv_bu *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_UB);
- gen_helper_hyp_load(t1, cpu_env, t0, mem_idx, memop);
+ tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_UB);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -126,20 +110,15 @@ static bool trans_hlv_hu(DisasContext *ctx, arg_hlv_hu *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
- gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TEUW);
+ gen_helper_hyp_access_check(cpu_env);
- gen_helper_hyp_load(t1, cpu_env, t0, mem_idx, memop);
+ gen_get_gpr(t0, a->rs1);
+ tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TEUW);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -152,20 +131,16 @@ static bool trans_hsv_b(DisasContext *ctx, arg_hsv_b *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv dat = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
gen_get_gpr(dat, a->rs2);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_SB);
- gen_helper_hyp_store(cpu_env, t0, dat, mem_idx, memop);
+ tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_SB);
tcg_temp_free(t0);
tcg_temp_free(dat);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -178,20 +153,16 @@ static bool trans_hsv_h(DisasContext *ctx, arg_hsv_h *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv dat = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
gen_get_gpr(dat, a->rs2);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TESW);
- gen_helper_hyp_store(cpu_env, t0, dat, mem_idx, memop);
+ tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TESW);
tcg_temp_free(t0);
tcg_temp_free(dat);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -204,20 +175,16 @@ static bool trans_hsv_w(DisasContext *ctx, arg_hsv_w *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv dat = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
gen_get_gpr(dat, a->rs2);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TESL);
- gen_helper_hyp_store(cpu_env, t0, dat, mem_idx, memop);
+ tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TESL);
tcg_temp_free(t0);
tcg_temp_free(dat);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -231,20 +198,16 @@ static bool trans_hlv_wu(DisasContext *ctx, arg_hlv_wu *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TEUL);
- gen_helper_hyp_load(t1, cpu_env, t0, mem_idx, memop);
+ tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TEUL);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -257,20 +220,16 @@ static bool trans_hlv_d(DisasContext *ctx, arg_hlv_d *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TEQ);
- gen_helper_hyp_load(t1, cpu_env, t0, mem_idx, memop);
+ tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TEQ);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -283,20 +242,16 @@ static bool trans_hsv_d(DisasContext *ctx, arg_hsv_d *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv dat = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
+
+ gen_helper_hyp_access_check(cpu_env);
gen_get_gpr(t0, a->rs1);
gen_get_gpr(dat, a->rs2);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TEQ);
- gen_helper_hyp_store(cpu_env, t0, dat, mem_idx, memop);
+ tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TEQ);
tcg_temp_free(t0);
tcg_temp_free(dat);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v1 5/5] target/riscv: Split the Hypervisor execute load helpers
2020-10-23 15:25 [PATCH v1 0/5] Fix the Hypervisor access functions Alistair Francis
` (3 preceding siblings ...)
2020-10-23 15:26 ` [PATCH v1 4/5] target/riscv: Remove the hyp load and store functions Alistair Francis
@ 2020-10-23 15:26 ` Alistair Francis
4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-10-23 15:26 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: alistair.francis, richard.henderson, bmeng.cn, palmer, alistair23
Split the hypervisor execute load functions into two seperate functions.
This avoids us having to pass the memop to the C helper functions.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/helper.h | 3 +-
target/riscv/op_helper.c | 38 ++++++++++++++-----------
target/riscv/insn_trans/trans_rvh.c.inc | 16 ++---------
3 files changed, 26 insertions(+), 31 deletions(-)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 7dbdd117d2..ae50730273 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -82,7 +82,8 @@ DEF_HELPER_1(tlb_flush, void, env)
DEF_HELPER_1(hyp_tlb_flush, void, env)
DEF_HELPER_1(hyp_gvma_tlb_flush, void, env)
DEF_HELPER_1(hyp_access_check, void, env)
-DEF_HELPER_4(hyp_x_load, tl, env, tl, tl, tl)
+DEF_HELPER_2(hyp_hlvx_hu, tl, env, tl)
+DEF_HELPER_2(hyp_hlvx_wu, tl, env, tl)
#endif
/* Vector functions */
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 95281db3ec..065aa57d97 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -246,28 +246,34 @@ void helper_hyp_access_check(CPURISCVState *env)
}
}
-target_ulong helper_hyp_x_load(CPURISCVState *env, target_ulong address,
- target_ulong attrs, target_ulong memop)
+target_ulong helper_hyp_hlvx_hu(CPURISCVState *env, target_ulong address)
{
if (env->priv == PRV_M ||
(env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
(env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
get_field(env->hstatus, HSTATUS_HU))) {
- target_ulong pte;
- int mmu_idx = cpu_mmu_index(env, false) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
-
- switch (memop) {
- case MO_TEUW:
- pte = cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- case MO_TEUL:
- pte = cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
- break;
- default:
- g_assert_not_reached();
- }
+ int mmu_idx = cpu_mmu_index(env, true) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+
+ return cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
+ }
+
+ if (riscv_cpu_virt_enabled(env)) {
+ riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
+ } else {
+ riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
+ }
+ return 0;
+}
+
+target_ulong helper_hyp_hlvx_wu(CPURISCVState *env, target_ulong address)
+{
+ if (env->priv == PRV_M ||
+ (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
+ (env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
+ get_field(env->hstatus, HSTATUS_HU))) {
+ int mmu_idx = cpu_mmu_index(env, true) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
- return pte;
+ return cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
}
if (riscv_cpu_virt_enabled(env)) {
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc
index 79968701e9..f3ffd742d3 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -265,20 +265,14 @@ static bool trans_hlvx_hu(DisasContext *ctx, arg_hlvx_hu *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TEUW);
- gen_helper_hyp_x_load(t1, cpu_env, t0, mem_idx, memop);
+ gen_helper_hyp_hlvx_hu(t1, cpu_env, t0);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
@@ -291,20 +285,14 @@ static bool trans_hlvx_wu(DisasContext *ctx, arg_hlvx_wu *a)
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
- TCGv mem_idx = tcg_temp_new();
- TCGv memop = tcg_temp_new();
gen_get_gpr(t0, a->rs1);
- tcg_gen_movi_tl(mem_idx, ctx->mem_idx);
- tcg_gen_movi_tl(memop, MO_TEUL);
- gen_helper_hyp_x_load(t1, cpu_env, t0, mem_idx, memop);
+ gen_helper_hyp_hlvx_wu(t1, cpu_env, t0);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
- tcg_temp_free(mem_idx);
- tcg_temp_free(memop);
return true;
#else
return false;
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread