From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57677) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Znwix-0007Sa-E9 for qemu-devel@nongnu.org; Sun, 18 Oct 2015 18:46:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Znwiu-0000K3-7s for qemu-devel@nongnu.org; Sun, 18 Oct 2015 18:46:43 -0400 Received: from mail-pa0-x232.google.com ([2607:f8b0:400e:c03::232]:33733) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Znwiu-0000Jx-1w for qemu-devel@nongnu.org; Sun, 18 Oct 2015 18:46:40 -0400 Received: by pabrc13 with SMTP id rc13so170863425pab.0 for ; Sun, 18 Oct 2015 15:46:39 -0700 (PDT) Sender: Richard Henderson References: <1444774253-10492-1-git-send-email-rth@twiddle.net> <561EC2FA.40901@twiddle.net> <56204F63.6010501@twiddle.net> <562104CB.1080801@gmail.com> From: Richard Henderson Message-ID: <5624214B.3020401@twiddle.net> Date: Sun, 18 Oct 2015 12:46:35 -1000 MIME-Version: 1.0 In-Reply-To: <562104CB.1080801@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target-*: Advance pc after recognizing a breakpoint List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Fedorov , Peter Maydell Cc: QEMU Developers On 10/16/2015 04:08 AM, Sergey Fedorov wrote: > On 16.10.2015 04:14, Richard Henderson wrote: >> On 10/16/2015 03:36 AM, Peter Maydell wrote: >>> On 14 October 2015 at 22:02, Richard Henderson wrote: >>>> On 10/15/2015 06:34 AM, Peter Maydell wrote: >>>>> >>>>> This is still the same cryptic comment we have in the >>>>> targets which do do this. Can we have something >>>>> that is a bit more explanatory about what is going on and >>>>> why we need to do this, please? >>>> >>>> >>>> Suggestions? >>> >>> ...well, I don't entirely understand the problem it's >>> fixing, which is why I'm asking for a better comment :-) >> >> Heh. Fair enough. How about >> >> /* The address covered by the breakpoint must be included in >> [tb->pc, tb->pc + tb->size) in order to for it to be >> properly cleared -- thus we increment the PC here so that >> the logic setting tb->size below does the right thing. */ >> >> There are two edge cases that cause the problem with clearing that >> could be described, but I think that the comment becomes too bulky, as >> well as confuses the situation for someone cutting-and-pasting the >> logic to a new port. > > Maybe we could rather fix that condition in > tb_invalidate_phys_page_range()? It seems weird that it can't handle a > zero-sized TB. We also need to be able to handle a TB which crosses a page. E.g. the breakpoint is at the page boundary, and we fall through into it from the top. This will be true on e.g. x86. This is not simply true for breakpoint insertion/removal, but also page invalidation. The same fix, adding a byte to the size, handles this as well. r~