|
From: | Richard Henderson |
Subject: | Re: [Qemu-devel] [PATCH] target-*: Advance pc after recognizing a breakpoint |
Date: | Mon, 19 Oct 2015 07:04:45 -1000 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 10/19/2015 01:04 AM, Sergey Fedorov wrote:
On 19.10.2015 01:46, Richard Henderson wrote: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 <address@hidden> 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.It's clear except that instructions crossing a page boundary can be different in size. AFAIK, x86 instructions can be up to 15-byte long. What if only the very last byte of instruction crosses a page boundary?
Then only the last byte crosses? What's your point? r~
[Prev in Thread] | Current Thread | [Next in Thread] |