From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60763) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj6v1-0004fE-V9 for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:11:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aj6uy-0000pH-O7 for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:11:27 -0400 Received: from mail-wm0-x234.google.com ([2a00:1450:400c:c09::234]:33471) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj6uy-0000p0-Dn for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:11:24 -0400 Received: by mail-wm0-x234.google.com with SMTP id l68so279245950wml.0 for ; Thu, 24 Mar 2016 08:11:24 -0700 (PDT) References: <1458815961-31979-1-git-send-email-sergey.fedorov@linaro.org> <1458815961-31979-5-git-send-email-sergey.fedorov@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1458815961-31979-5-git-send-email-sergey.fedorov@linaro.org> Date: Thu, 24 Mar 2016 15:11:00 +0000 Message-ID: <87k2krrjwb.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 4/8] tcg: Init TB's direct jumps before making it visible List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: sergey.fedorov@linaro.org Cc: Sergey Fedorov , Richard Henderson , Peter Crosthwaite , qemu-devel@nongnu.org, Paolo Bonzini sergey.fedorov@linaro.org writes: > From: Sergey Fedorov > > 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 > Signed-off-by: Sergey Fedorov > --- > 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