From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj7eP-00063d-83 for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:58:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aj7eM-0005mE-1s for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:58:21 -0400 Received: from mail-lf0-x241.google.com ([2a00:1450:4010:c07::241]:36491) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj7eL-0005lr-R2 for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:58:17 -0400 Received: by mail-lf0-x241.google.com with SMTP id r8so4203175lfe.3 for ; Thu, 24 Mar 2016 08:58:17 -0700 (PDT) References: <1458815961-31979-1-git-send-email-sergey.fedorov@linaro.org> <1458815961-31979-5-git-send-email-sergey.fedorov@linaro.org> <87k2krrjwb.fsf@linaro.org> <56F4085D.2010704@gmail.com> <87d1qjrija.fsf@linaro.org> From: Sergey Fedorov Message-ID: <56F40E95.5040908@gmail.com> Date: Thu, 24 Mar 2016 18:58:13 +0300 MIME-Version: 1.0 In-Reply-To: <87d1qjrija.fsf@linaro.org> 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: =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: Paolo Bonzini , Richard Henderson , qemu-devel@nongnu.org, sergey.fedorov@linaro.org, Peter Crosthwaite On 24/03/16 18:40, Alex Bennée wrote: > Sergey Fedorov writes: > >> On 24/03/16 18:11, Alex Bennée wrote: >>> sergey.fedorov@linaro.org writes: >>>> From: Sergey Fedorov >>>> >>>> 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? >> I meant to keep them together with the following lines. >> >>>> + >>>> + /* 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. >> Because tcg_gen_code() gets called in between and it is passed >> '&tcg_ctx' which holds a pointer to 'tb->jmp_reset_offset'. tcg_out_op() >> is called from tcg_gen_code() and sets 'tb->jmp_reset_offset[n]' >> indirectly, as well as 'tb->jmp_insn_offset[n]'. > OK a quick addition to the comment: "these may have been reset in > tcg_gen_code" would be helpful here. The other way around: 'tb->jmp_reset_offset[n]' are reset before calling tcg_gen_code() and are set during tcg_gen_code() execution. Kind regards, Sergey