Re: [Qemu-devel] [PATCH v4 09/10] tcg: Clean up direct block chaining safety checks

[Sergey Fedorov]

On 21/04/16 16:18, Alex Bennée wrote:
> Sergey Fedorov <address@hidden> writes:
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index bbfcbfb54385..065cc9159477 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -508,11 +508,8 @@ int cpu_exec(CPUState *cpu)
>>                      next_tb = 0;
>>                      tcg_ctx.tb_ctx.tb_invalidated_flag = 0;
>>                  }
>> -                /* see if we can patch the calling TB. When the TB
>> -                   spans two pages, we cannot safely do a direct
>> -                   jump. */
>> -                if (next_tb != 0 && tb->page_addr[1] == -1
>> -                    && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
>> +                /* See if we can patch the calling TB. */
>> +                if (next_tb != 0 &&
>> !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> A pointer to the definitive comment helps ;-)
>
>                 /* See if we can patch the calling TB, see tcg_gen_goto_tb */

I'm not so sure that the comment for tcg_gen_goto_tb() would be of much
use here. Actually, what we check here is if we know the calling TB
(what is called 'next_tb' here so far) and if logging settings don't
forbid us to chain TBs. The note in the comment for tcg_gen_goto_tb() is
all about when goto_tb TCG ops can be emitted by the target translation
code, not so relevant here, I suppose.

Kind regards,
Sergey

>
>>                      tb_add_jump((TranslationBlock *)(next_tb & 
>> ~TB_EXIT_MASK),
>>                                  next_tb & TB_EXIT_MASK, tb);
>>                  }
(snip)
>> diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
>> index c446d3dc7293..ace39619ef89 100644
>> --- a/tcg/tcg-op.h
>> +++ b/tcg/tcg-op.h
>> @@ -753,6 +753,16 @@ static inline void tcg_gen_exit_tb(uintptr_t val)
>>      tcg_gen_op1i(INDEX_op_exit_tb, val);
>>  }
>>
>> +/**
>> + * tcg_gen_goto_tb() - output goto_tb TCG operation
>> + * @idx: Direct jump slot index (0 or 1)
>> + *
>> + * See tcg/README for more info about this TCG operation.
>> + *
>> + * NOTE: Direct jumps with goto_tb are only safe within the pages this TB
>> + * resides in because we don't take care of direct jumps when address 
>> mapping
>> + * changes, e.g. in tlb_flush().
>> + */
>>  void tcg_gen_goto_tb(unsigned idx);
>>
>>  #if TARGET_LONG_BITS == 32