Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

On 2012-05-23 22:29, TeLeMan wrote:
> On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2012-05-23 13:02, Jan Kiszka wrote:
>>> On 2012-05-23 11:11, TeLeMan wrote:
>>>> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2012-05-23 04:09, TeLeMan wrote:
>>>>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote:
>>>>>>> Am 18.05.2012 11:49, schrieb TeLeMan:
>>>>>>>> This breakage was introduced by the commit "memory: make
>>>>>>>> phys_page_find() return an unadjusted".
>>>>>>>
>>>>>>> You seem to have found the origin of your problem. If you also mention
>>>>>>> the commit hash in your commit message then certain frontends (gitk,
>>>>>>> repo.or.cz) will display it as a handy hyperlink to that commit.
>>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: TeLeMan <geleman@gmail.com>
>>>>>>>
>>>>>>> Signed-off-by is a legal statement of origin and must not be a pseudonym.
>>>>>> Ok, please ignore this patch. I won't submit any patch just report bugs.
>>>>>
>>>>> Then please describe this bug in more details, e.g. how to reproduce.
>>>> I think its evident. cpu_get_phys_page_debug(env, pc) is not the
>>>> physical address of pc but the physical page base address of pc.
>>>
>>> ...so this bites us if the instruction spans two pages as
>>> tb_invalidate_phys_addr requests invalidation on a page granularity.
>>
>> In fact, this is irrelevant. We only need to flush the address at which
>> the instruction starts, and that is achieved by flushing all TB that
>> relate to that page as the current code does.
> 
> But the instruction start is wrong and its TB may not be found. For example,
> the pc is 0x1234 and its physical address is 0x1234. The correct
> "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and
> 0x1235. But now the "start" and "end" is 0x1000 and 0x1001.
> If 0x1000 is not translated yet, the real TB won't be invalidated.

The tb containing 0x1234 would be linked to the list of TBs that are
related to the 0x1000 page. As we declare that page invalid, all
affected TBs are dropped, not just the one containing the breakpoint.
See tb_invalidate_phys_page_range.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]