Re: [Qemu-devel] tcg/translate-all.c:169: tb_lock: Assertion `!have_tb_lock' failed when doing cpu_restore_state in usermode

[Paolo Bonzini]

On 10/10/2017 12:07, Peter Maydell wrote:
> On 10 October 2017 at 10:53, Alex Bennée <address@hidden> wrote:
>>
>> Peter Maydell <address@hidden> writes:
>>
>>> Running the test program
>>> http://people.linaro.org/~peter.maydell/thumb-over-page
>>> (source at http://people.linaro.org/~peter.maydell/thumb-over-page.c)
>>> in the usermode emulator:
>>>  ./build/x86/arm-linux-user/qemu-arm
>>> ~/linaro/qemu-misc-tests/thumb-over-page
>>
>> Does this fail when run via system mode as well?
> 
> Nope, only usermode. (Makes sense, since the handle_cpu_signal()
> codepath is only used in usermode emulation.)

I've seen the same on x86.  Using the program counter from translate.c 
here looks very fishy:

    /* Now we have a real cpu fault.  Since this is the exact location of
     * the exception, we must undo the adjustment done by cpu_restore_state
     * for handling call return addresses.  */
    cpu_restore_state(cpu, pc + GETPC_ADJ);

and cpu_restore_state would just return false because tb_find_pc fails.  Maybe
something like this?

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 492ea0826c..66a4351b96 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -119,10 +119,13 @@ static inline int handle_cpu_signal(uintptr_t pc, 
unsigned long address,
         return 1; /* the MMU fault was handled without causing real CPU fault 
*/
     }
 
-    /* Now we have a real cpu fault.  Since this is the exact location of
-     * the exception, we must undo the adjustment done by cpu_restore_state
-     * for handling call return addresses.  */
-    cpu_restore_state(cpu, pc + GETPC_ADJ);
+    if (pc >= (uintptr_t)tcg_ctx.code_gen_buffer &&
+        pc < (uintptr_t)tcg_ctx.code_gen_ptr) {
+        /* Now we have a real cpu fault.  Since this is the exact location of
+         * the exception, we must undo the adjustment done by cpu_restore_state
+         * for handling call return addresses.  */
+        cpu_restore_state(cpu, pc + GETPC_ADJ);
+    }
 
     sigprocmask(SIG_SETMASK, old_set, NULL);
     cpu_loop_exit(cpu);

(very rough idea)

Paolo