diff mbox series

[v2,5/5] target/riscv: Split the Hypervisor execute load helpers

Message ID a88d9bdcebe49ada0d0a69b37ac532124971c91c.1603896076.git.alistair.francis@wdc.com (mailing list archive)
State New, archived
Headers show
Series Fix the Hypervisor access functions | expand

Commit Message

Alistair Francis Oct. 28, 2020, 2:42 p.m. UTC 
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(-)

Comments

Richard Henderson Oct. 28, 2020, 3:22 p.m. UTC | #1 
On 10/28/20 7:42 AM, Alistair Francis wrote:
> +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))) {
> +        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 cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
>      }

Do not replicate the PRV tests.

My first suggestion is to compute this into TBFLAGS and test it at translate
time, so that these functions just become the one cpu_ld* call.

But failing that, at least split out the test + exception into a common helper
function.

r~
diff mbox series

Patch

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 d81d8282cc..ff4426cf4d 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -243,28 +243,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;