From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33117) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4CGx-0004VY-2v for qemu-devel@nongnu.org; Wed, 02 Dec 2015 13:37:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4CGt-0004bv-KD for qemu-devel@nongnu.org; Wed, 02 Dec 2015 13:36:58 -0500 Received: from mail-qk0-x235.google.com ([2607:f8b0:400d:c09::235]:35539) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4CGt-0004bl-E1 for qemu-devel@nongnu.org; Wed, 02 Dec 2015 13:36:55 -0500 Received: by qkao63 with SMTP id o63so19919708qka.2 for ; Wed, 02 Dec 2015 10:36:55 -0800 (PST) Sender: Richard Henderson References: <1448986767-28016-1-git-send-email-rth@twiddle.net> <20151201163254.GB9577@aurel32.net> From: Richard Henderson Message-ID: <565F3A43.9030005@twiddle.net> Date: Wed, 2 Dec 2015 10:36:51 -0800 MIME-Version: 1.0 In-Reply-To: <20151201163254.GB9577@aurel32.net> Content-Type: multipart/mixed; boundary="------------070506020108080108050404" Subject: Re: [Qemu-devel] [PATCH for-2.5] tcg: Increase the highwater reservation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------070506020108080108050404 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On 12/01/2015 08:32 AM, Aurelien Jarno wrote: > On 2015-12-01 08:19, Richard Henderson wrote: >> If there are a lot of guest memory ops in the TB, the amount of >> code generated by tcg_out_tb_finalize could be well more than 1k. >> In the short term, increase the reservation larger than any TB >> seen in practice. >> >> Reported-by: Aurelien Jarno >> Signed-off-by: Richard Henderson > > Thanks! I was testing the same patch locally after our discussion, and I > confirm it fixes the issue. Here's the proper patch for 2.6. If you'd like to run it through your same tests, please? r~ --------------070506020108080108050404 Content-Type: text/plain; charset=UTF-8; name="z" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="z" diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c index 647e9a6..b30d643 100644 --- a/tcg/ia64/tcg-target.c +++ b/tcg/ia64/tcg-target.c @@ -1572,7 +1572,7 @@ static void add_qemu_ldst_label(TCGContext *s, bool is_ld, TCGMemOp opc, be->labels = l; } -static void tcg_out_tb_finalize(TCGContext *s) +static int tcg_out_tb_finalize(TCGContext *s) { static const void * const helpers[8] = { helper_ret_stb_mmu, @@ -1620,7 +1620,16 @@ static void tcg_out_tb_finalize(TCGContext *s) } reloc_pcrel21b_slot2(l->label_ptr, dest); + + /* Test for (pending) buffer overflow. The assumption is that any + one operation beginning below the high water mark cannot overrun + the buffer completely. Thus we can test for overflow after + generating code without having to check during generation. */ + if (unlikely((void *)s->code_ptr > s->code_gen_highwater)) { + return -1; + } } + return 0; } static inline void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args) diff --git a/tcg/tcg-be-ldst.h b/tcg/tcg-be-ldst.h index 40a2369..08222a3 100644 --- a/tcg/tcg-be-ldst.h +++ b/tcg/tcg-be-ldst.h @@ -56,7 +56,7 @@ static inline void tcg_out_tb_init(TCGContext *s) static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l); static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l); -static void tcg_out_tb_finalize(TCGContext *s) +static int tcg_out_tb_finalize(TCGContext *s) { TCGLabelQemuLdst *lb; @@ -67,7 +67,16 @@ static void tcg_out_tb_finalize(TCGContext *s) } else { tcg_out_qemu_st_slow_path(s, lb); } + + /* Test for (pending) buffer overflow. The assumption is that any + one operation beginning below the high water mark cannot overrun + the buffer completely. Thus we can test for overflow after + generating code without having to check during generation. */ + if (unlikely((void *)s->code_ptr > s->code_gen_highwater)) { + return -1; + } } + return 0; } /* diff --git a/tcg/tcg-be-null.h b/tcg/tcg-be-null.h index 74c57d5..051f609 100644 --- a/tcg/tcg-be-null.h +++ b/tcg/tcg-be-null.h @@ -38,6 +38,7 @@ static inline void tcg_out_tb_init(TCGContext *s) * Generate TB finalization at the end of block */ -static inline void tcg_out_tb_finalize(TCGContext *s) +static inline int tcg_out_tb_finalize(TCGContext *s) { + return 0; } diff --git a/tcg/tcg.c b/tcg/tcg.c index a163541..841ed53 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -110,7 +110,7 @@ static void tcg_out_call(TCGContext *s, tcg_insn_unit *target); static int tcg_target_const_match(tcg_target_long val, TCGType type, const TCGArgConstraint *arg_ct); static void tcg_out_tb_init(TCGContext *s); -static void tcg_out_tb_finalize(TCGContext *s); +static int tcg_out_tb_finalize(TCGContext *s); @@ -388,11 +388,7 @@ void tcg_prologue_init(TCGContext *s) /* Compute a high-water mark, at which we voluntarily flush the buffer and start over. The size here is arbitrary, significantly larger than we expect the code generation for any one opcode to require. */ - /* ??? We currently have no good estimate for, or checks in, - tcg_out_tb_finalize. If there are quite a lot of guest memory ops, - the number of out-of-line fragments could be quite high. In the - short-term, increase the highwater buffer. */ - s->code_gen_highwater = s->code_gen_buffer + (total_size - 64*1024); + s->code_gen_highwater = s->code_gen_buffer + (total_size - 1024); tcg_register_jit(s->code_gen_buffer, total_size); @@ -2455,7 +2451,9 @@ int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf) s->gen_insn_end_off[num_insns] = tcg_current_code_size(s); /* Generate TB finalization at the end of block */ - tcg_out_tb_finalize(s); + if (tcg_out_tb_finalize(s) < 0) { + return -1; + } /* flush instruction cache */ flush_icache_range((uintptr_t)s->code_buf, (uintptr_t)s->code_ptr); --------------070506020108080108050404--