Re: [Qemu-devel] [PATCH] tci: Optimize saving of TCG code address

[Richard Henderson]

On 04/29/2014 10:33 AM, Stefan Weil wrote:
> I'm still investigating whether it's necessary to set tci_tb_ptr to 0
> (as you suggested). Up to now, the TCI code did never invalidate
> tci_tb_ptr, but there was no obvious indication of problems caused by
> this behaviour.

It should be exactly 6 lines of code; see below.

> The new function save_tb_ptr() is only used for the opcodes which call
> helper_{ld,st}*_mmu. Those calls are only compiled for CONFIG_SOFTMMU,
> so there is no need to save tci_tb_ptr if that macro is undefined. The
> user mode emulation (which does not set CONFIG_SOFTMMU) is faster like that.

But the user mode emulation is quicker /because/ it expects the SIGSEGV handler
to be able to fetch the address of the faulting instruction, which of course
gets mapped back to TCG generated code (or not).

TCI, at present, always fails the "TCG generated code" path of that test, so we
never apply the cpu_restore_state in handle_cpu_signal.

I did momentarily wonder if we actually would need some sort of compiler
barrier to make sure that the memory operations happen in the proper order, but
I think that -fno-strict-aliasing gives that to us for free.


r~


diff --git a/cpu-exec.c b/cpu-exec.c
index 2f54054..f398c1f 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -85,6 +85,12 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu,
uint8_t *tb_ptr)
          */
         cpu->tcg_exit_req = 0;
     }
+
+#ifdef CONFIG_TCG_INTERPRETER
+    /* No longer executing translated code.  */
+    tci_tb_ptr = 0;
+#endif
+
     return next_tb;
 }

@@ -696,6 +702,10 @@ int cpu_exec(CPUArchState *env)
 #ifdef TARGET_I386
             x86_cpu = X86_CPU(cpu);
 #endif
+#ifdef CONFIG_TCG_INTERPRETER
+            /* No longer executing translated code.  */
+            tci_tb_ptr = 0;
+#endif
             if (have_tb_lock) {
                 spin_unlock(&tcg_ctx.tb_ctx.tb_lock);
                 have_tb_lock = false;
diff --git a/tci.c b/tci.c
index 407dd3a..5f01048 100644
--- a/tci.c
+++ b/tci.c
@@ -59,9 +59,7 @@ uintptr_t tci_tb_ptr;

 static inline void save_tb_ptr(void *tb_ptr)
 {
-#ifdef CONFIG_SOFTMMU
     tci_tb_ptr = (uintptr_t)tb_ptr;
-#endif
 }

 static tcg_target_ulong tci_read_reg(TCGReg index)
diff --git a/user-exec.c b/user-exec.c
index 8ed6fec..252c236 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -89,6 +89,12 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned
long address,
     CPUClass *cc;
     int ret;

+#ifdef CONFIG_TCG_INTERPRETER
+    /* We're interested in "pc" of the translation.  In the case of
+       native generated code, that's the same as the host pc, but in
+       the case of the translator, it's different.  */
+    pc = tci_tb_ptr;
+#endif
 #if defined(DEBUG_SIGNAL)
     qemu_printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",
                 pc, address, is_write, *(unsigned long *)old_set);