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

[Peter Maydell]

On 10 October 2017 at 11:41, Paolo Bonzini <address@hidden> wrote:
> 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);

This is the right thing if the signal happened directly from
translated code (as it will for guest reads/writes). This
bit of code expects that cpu_restore_state() will just do nothing
if the pc value isn't actually in a TB.

> 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);
> +    }

I think that would be better inside cpu_restore_state(), because it's
an internal detail of the TCG accelerator that it happens to put
all its code inside those bounds.

thanks
-- PMM