From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32795) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxGHR-0004c2-Pi for qemu-devel@nongnu.org; Tue, 26 May 2015 10:56:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YxGHN-0004t5-57 for qemu-devel@nongnu.org; Tue, 26 May 2015 10:56:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37646) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxGHM-0004sx-Ty for qemu-devel@nongnu.org; Tue, 26 May 2015 10:56:29 -0400 Message-ID: <5564898E.9000801@redhat.com> Date: Tue, 26 May 2015 16:56:14 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <20141022113831.9548.36452.stgit@PASHA-ISP> <54B37FCA.4020805@siemens.com> <000801d02e41$75c1a000$6144e000$@Dovgaluk@ispras.ru> <54B38612.6020903@siemens.com> <54B38C03.9000608@redhat.com> <5561E380.5010003@web.de> In-Reply-To: <5561E380.5010003@web.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] i386: fix breakpoints handling in icount mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka , Pavel Dovgaluk , qemu-devel@nongnu.org Cc: batuzovk@ispras.ru, zealot351@gmail.com, maria.klimushenkova@ispras.ru, mark.burton@greensocs.com, fred.konrad@greensocs.com On 24/05/2015 16:43, Jan Kiszka wrote: > diff --git a/target-i386/translate.c b/target-i386/translate.c > index 305ce50..57b607d 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -8006,6 +8006,7 @@ static inline void gen_intermediate_code_internal(X86CPU *cpu, > if (bp->pc == pc_ptr && > !((bp->flags & BP_CPU) && (tb->flags & HF_RF_MASK))) { > gen_debug(dc, pc_ptr - dc->cs_base); > + pc_ptr = disas_insn(env, dc, pc_ptr); > goto done_generating; > } > } > > pc_ptr is used at the end of the function to calculate the tb size. I > suspect that the difference prevents that the breakpoint event is > associated with the stored location. Can someone explain this more > properly? Then I would happily pass patch credits. So when a breakpoint is removed at address X, you have to also remove translation blocks that end exactly at X? That is: diff --git a/translate-all.c b/translate-all.c index 536008f..6e50b8f 100644 --- a/translate-all.c +++ b/translate-all.c @@ -1113,7 +1116,7 @@ tb_start = tb->page_addr[1]; tb_end = tb_start + ((tb->pc + tb->size) & ~TARGET_PAGE_MASK); } - if (!(tb_end <= start || tb_start >= end)) { + if (tb_start < end && tb_end >= start) { #ifdef TARGET_HAS_PRECISE_SMC if (current_tb_not_found) { current_tb_not_found = 0; Does this fix the bug? Is there any other case where this is desirable? Should tb_invalidate_phys_page_range grow another argument to choose between "tb_end > start" and "tb_end >= start"? Paolo