[v3,09/30] target/mips: Merge do_translate_address into cpu_mips_translate_address

Commit Message

Philippe Mathieu-Daudé April 19, 2021, 7:18 p.m. UTC

Currently cpu_mips_translate_address() calls raise_mmu_exception(),
and do_translate_address() calls cpu_loop_exit_restore().

This API split is dangerous, we could call cpu_mips_translate_address
without returning to the main loop.

As there is only one caller, it is trivial (and safer) to merge
do_translate_address() back to cpu_mips_translate_address().

Reported-by: Richard Henderson <[e-mail]>
Suggested-by: Richard Henderson <[e-mail]>
Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
---
 target/mips/internal.h   |  2 +-
 target/mips/op_helper.c  | 20 ++------------------
 target/mips/tlb_helper.c | 11 ++++++-----
 3 files changed, 9 insertions(+), 24 deletions(-)

Comments

Richard Henderson April 19, 2021, 7:31 p.m. UTC | #1

On 4/19/21 12:18 PM, Philippe Mathieu-Daudé wrote:
> Currently cpu_mips_translate_address() calls raise_mmu_exception(),
> and do_translate_address() calls cpu_loop_exit_restore().
> 
> This API split is dangerous, we could call cpu_mips_translate_address
> without returning to the main loop.
> 
> As there is only one caller, it is trivial (and safer) to merge
> do_translate_address() back to cpu_mips_translate_address().
> 
> Reported-by: Richard Henderson<[e-mail]>
> Suggested-by: Richard Henderson<[e-mail]>
> Signed-off-by: Philippe Mathieu-Daudé<[e-mail]>
> ---
>   target/mips/internal.h   |  2 +-
>   target/mips/op_helper.c  | 20 ++------------------
>   target/mips/tlb_helper.c | 11 ++++++-----
>   3 files changed, 9 insertions(+), 24 deletions(-)

Reviewed-by: Richard Henderson <[e-mail]>

r~

Patch

diff --git a/target/mips/internal.h b/target/mips/internal.h
index 81671d567d0..806d39fa6c3 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -148,7 +148,7 @@  void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
                                     int mmu_idx, MemTxAttrs attrs,
                                     MemTxResult response, uintptr_t retaddr);
 hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address,
-                                  MMUAccessType access_type);
+                                  MMUAccessType access_type, uintptr_t retaddr);
 #endif
 
 #define cpu_signal_handler cpu_mips_signal_handler
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index f7da8c83aee..fdae5a3d687 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -287,23 +287,6 @@  target_ulong helper_rotx(target_ulong rs, uint32_t shift, uint32_t shiftx,
 
 #ifndef CONFIG_USER_ONLY
 
-static inline hwaddr do_translate_address(CPUMIPSState *env,
-                                          target_ulong address,
-                                          MMUAccessType access_type,
-                                          uintptr_t retaddr)
-{
-    hwaddr paddr;
-    CPUState *cs = env_cpu(env);
-
-    paddr = cpu_mips_translate_address(env, address, access_type);
-
-    if (paddr == -1LL) {
-        cpu_loop_exit_restore(cs, retaddr);
-    } else {
-        return paddr;
-    }
-}
-
 #define HELPER_LD_ATOMIC(name, insn, almask, do_cast)                         \
 target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx)  \
 {                                                                             \
@@ -313,7 +296,8 @@  target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx)  \
         }                                                                     \
         do_raise_exception(env, EXCP_AdEL, GETPC());                          \
     }                                                                         \
-    env->CP0_LLAddr = do_translate_address(env, arg, MMU_DATA_LOAD, GETPC()); \
+    env->CP0_LLAddr = cpu_mips_translate_address(env, arg, MMU_DATA_LOAD,     \
+                                                 GETPC());                    \
     env->lladdr = arg;                                                        \
     env->llval = do_cast cpu_##insn##_mmuidx_ra(env, arg, mem_idx, GETPC());  \
     return env->llval;                                                        \
diff --git a/target/mips/tlb_helper.c b/target/mips/tlb_helper.c
index 8d3ea497803..1ffdc1f8304 100644
--- a/target/mips/tlb_helper.c
+++ b/target/mips/tlb_helper.c
@@ -904,21 +904,22 @@  bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 
 #ifndef CONFIG_USER_ONLY
 hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address,
-                                  MMUAccessType access_type)
+                                  MMUAccessType access_type, uintptr_t retaddr)
 {
     hwaddr physical;
     int prot;
     int ret = 0;
+    CPUState *cs = env_cpu(env);
 
     /* data access */
     ret = get_physical_address(env, &physical, &prot, address, access_type,
                                cpu_mmu_index(env, false));
-    if (ret != TLBRET_MATCH) {
-        raise_mmu_exception(env, address, access_type, ret);
-        return -1LL;
-    } else {
+    if (ret == TLBRET_MATCH) {
         return physical;
     }
+
+    raise_mmu_exception(env, address, access_type, ret);
+    cpu_loop_exit_restore(cs, retaddr);
 }
 
 static void set_hflags_for_handler(CPUMIPSState *env)