Re: [Qemu-devel] [PATCH 4/8] tcg: Init TB's direct jumps before making it visible

[Alex Bennée]

address@hidden writes:

> From: Sergey Fedorov <address@hidden>
>
> Initialize TB's direct jump list data fields and reset the jumps before
> tb_link_page() puts it into the physical hash table and the physical
> page list. So TB is completely initialized before it becomes visible.
>
> Signed-off-by: Sergey Fedorov <address@hidden>
> Signed-off-by: Sergey Fedorov <address@hidden>
> ---
>  translate-all.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/translate-all.c b/translate-all.c
> index ca01dd325b8d..f68716e1819f 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -1131,19 +1131,6 @@ static void tb_link_page(TranslationBlock *tb, 
> tb_page_addr_t phys_pc,
>          tb->page_addr[1] = -1;
>      }
>
> -    assert(((uintptr_t)tb & 3) == 0);
> -    tb->jmp_list_first = (uintptr_t)tb | 2;
> -    tb->jmp_list_next[0] = (uintptr_t)NULL;
> -    tb->jmp_list_next[1] = (uintptr_t)NULL;
> -
> -    /* init original jump addresses */
> -    if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) {
> -        tb_reset_jump(tb, 0);
> -    }
> -    if (tb->jmp_reset_offset[1] != TB_JMP_RESET_OFFSET_INVALID) {
> -        tb_reset_jump(tb, 1);
> -    }
> -
>  #ifdef DEBUG_TB_CHECK
>      tb_page_check();
>  #endif
> @@ -1251,6 +1238,20 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>          ROUND_UP((uintptr_t)gen_code_buf + gen_code_size + search_size,
>                   CODE_GEN_ALIGN);
>
> +    /* init jump list */
> +    assert(((uintptr_t)tb & 3) == 0);
> +    tb->jmp_list_first = (uintptr_t)tb | 2;
> +    tb->jmp_list_next[0] = (uintptr_t)NULL;
> +    tb->jmp_list_next[1] = (uintptr_t)NULL;

maybe these should be further up the function with the other jmp setting code?

> +
> +    /* init original jump addresses */
> +    if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) {
> +        tb_reset_jump(tb, 0);
> +    }
> +    if (tb->jmp_reset_offset[1] != TB_JMP_RESET_OFFSET_INVALID) {
> +        tb_reset_jump(tb, 1);
> +    }

Why would tb->jmp_reset_offset[0] == TB_JMP_RESET_OFFSET_INVALID not be
the case as it is set a few lines further up.

> +
>      /* check next page if needed */
>      virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
>      phys_page2 = -1;


--
Alex Bennée