diff mbox

[v3,07/11] translate-all: exit cpu_restore_state early if translating

Message ID 20170307155054.5833-8-alex.bennee@linaro.org
State New
Headers show

Commit Message

Alex Bennée March 7, 2017, 3:50 p.m. UTC
The translation code uses cpu_ld*_code which can trigger a tlb_fill
which if it fails will erroneously attempts a fault resolution. This
never works during translation as the TB being generated hasn't been
added yet. The target should have checked retaddr before calling
cpu_restore_state but for those that have yet to be fixed we do it
here to avoid a recursive tb_lock() under MTTCG's new locking regime.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - check retaddr instead
  - reword commit/comments to be clearer
---
 translate-all.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Richard Henderson March 7, 2017, 7:20 p.m. UTC | #1
On 03/08/2017 02:50 AM, Alex Bennée wrote:
> The translation code uses cpu_ld*_code which can trigger a tlb_fill
> which if it fails will erroneously attempts a fault resolution. This
> never works during translation as the TB being generated hasn't been
> added yet. The target should have checked retaddr before calling
> cpu_restore_state but for those that have yet to be fixed we do it
> here to avoid a recursive tb_lock() under MTTCG's new locking regime.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
diff mbox

Patch

diff --git a/translate-all.c b/translate-all.c
index d42d003e67..34480aebba 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -333,6 +333,19 @@  bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr)
     TranslationBlock *tb;
     bool r = false;
 
+    /* A retaddr of zero is invalid so we really shouldn't have ended
+     * up here. The target code has likely forgotten to check retaddr
+     * != 0 before attempting to restore state. We return early to
+     * avoid blowing up on a recursive tb_lock(). The target must have
+     * previously survived a failed cpu_restore_state because
+     * tb_find_pc(0) would have failed anyway. It still should be
+     * fixed though.
+     */
+
+    if (!retaddr) {
+        return r;
+    }
+
     tb_lock();
     tb = tb_find_pc(retaddr);
     if (tb) {