diff mbox

[3/5] exec: fix code tlb entry misused as iotlb in get_page_addr_code()

Message ID 4F660E6B.9080904@redhat.com
State New
Headers show

Commit Message

Avi Kivity March 18, 2012, 4:33 p.m. UTC
On 03/18/2012 01:07 PM, Max Filippov wrote:
> >>> get_page_addr_code() reads a code tlb entry, but interprets it as an
> >>> iotlb entry.  This works by accident since the low bits of a RAM code
> >>> tlb entry are clear, and match a RAM iotlb entry.  This accident is
> >>> about to unhappen, so fix the code to use an iotlb entry (using the
> >>> code entry with TLB_MMIO may fail if the page is a watchpoint).
> >>>
> >>> Signed-off-by: Avi Kivity<avi@redhat.com>
> >>> ---
> >>>   exec.c |    2 +-
> >>>   1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/exec.c b/exec.c
> >>> index a35eb4f..f26d1b0 100644
> >>> --- a/exec.c
> >>> +++ b/exec.c
> >>> @@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState
> >>> *env1, target_ulong addr)
> >>>                    (addr&  TARGET_PAGE_MASK))) {
> >>>           ldub_code(addr);
> >>>       }
> >>> -    pd = env1->tlb_table[mmu_idx][page_index].addr_code&
> >>> ~TARGET_PAGE_MASK;
> >>> +    pd = env1->iotlb[mmu_idx][page_index]&  ~TARGET_PAGE_MASK;
> >>>       if (pd != io_mem_ram.ram_addr&&  pd != io_mem_rom.ram_addr
> >>>           &&  !io_mem_region[pd]->rom_device) {
> >>>   #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) ||
> >>> defined(TARGET_SPARC)
> >>
> >> With this patch xtensa debug option unit test causes qemu abort with
> >> the message
> >>
> >>  qemu: fatal: Trying to execute code outside RAM or ROM at 0xd000088c
> >>
> >> This happens immediately after a watchpoint setup for a data
> >> breakpoint at the same memory page where the currently
> >> executed code is located. Any idea on how to fix it?
> >
> > Can you provide details on how to reproduce this?
>
> It may be reproduced by running test_break.tst unit test:
>
> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting  -kernel
> ./test_break.tst
>
> Compiled test binary is here:
> http://jcmvbkbc.spb.ru/~dumb/ws/osll/qemu-xtensa/20120318/test_break.tst
>

Thanks.  Please try out the attached patch.

Comments

Max Filippov March 18, 2012, 4:45 p.m. UTC | #1
>> >>> get_page_addr_code() reads a code tlb entry, but interprets it as an
>> >>> iotlb entry.  This works by accident since the low bits of a RAM code
>> >>> tlb entry are clear, and match a RAM iotlb entry.  This accident is
>> >>> about to unhappen, so fix the code to use an iotlb entry (using the
>> >>> code entry with TLB_MMIO may fail if the page is a watchpoint).
>> >>>
>> >>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> >>> ---
>> >>>   exec.c |    2 +-
>> >>>   1 files changed, 1 insertions(+), 1 deletions(-)
>> >>>
>> >>> diff --git a/exec.c b/exec.c
>> >>> index a35eb4f..f26d1b0 100644
>> >>> --- a/exec.c
>> >>> +++ b/exec.c
>> >>> @@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState
>> >>> *env1, target_ulong addr)
>> >>>                    (addr&  TARGET_PAGE_MASK))) {
>> >>>           ldub_code(addr);
>> >>>       }
>> >>> -    pd = env1->tlb_table[mmu_idx][page_index].addr_code&
>> >>> ~TARGET_PAGE_MASK;
>> >>> +    pd = env1->iotlb[mmu_idx][page_index]&  ~TARGET_PAGE_MASK;
>> >>>       if (pd != io_mem_ram.ram_addr&&  pd != io_mem_rom.ram_addr
>> >>>           &&  !io_mem_region[pd]->rom_device) {
>> >>>   #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) ||
>> >>> defined(TARGET_SPARC)
>> >>
>> >> With this patch xtensa debug option unit test causes qemu abort with
>> >> the message
>> >>
>> >>  qemu: fatal: Trying to execute code outside RAM or ROM at 0xd000088c
>> >>
>> >> This happens immediately after a watchpoint setup for a data
>> >> breakpoint at the same memory page where the currently
>> >> executed code is located. Any idea on how to fix it?
>> >
>> > Can you provide details on how to reproduce this?
>>
>> It may be reproduced by running test_break.tst unit test:
>>
>> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting  -kernel
>> ./test_break.tst
>>
>> Compiled test binary is here:
>> http://jcmvbkbc.spb.ru/~dumb/ws/osll/qemu-xtensa/20120318/test_break.tst
>>
>
> Thanks.  Please try out the attached patch.

Thanks, with this patch it works.

-- Max
diff mbox

Patch

From a7b99c89dfe9784a94af86df344d4ef893c15ee4 Mon Sep 17 00:00:00 2001
From: Avi Kivity <avi@redhat.com>
Date: Sun, 18 Mar 2012 18:31:13 +0200
Subject: [PATCH] memory: check for watchpoints when getting code ram_addr

The code to get the ram_addr from a (tlb entry, vaddr) pair
checks that the resulting memory is not MMIO, but neglects to
check whether the region is hidden by a watchpoint page.

Add the missing check.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/exec.c b/exec.c
index d8b089e..405e277 100644
--- a/exec.c
+++ b/exec.c
@@ -4605,7 +4605,8 @@  tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
     pd = env1->iotlb[mmu_idx][page_index] & ~TARGET_PAGE_MASK;
     mr = iotlb_to_region(pd);
     if (mr != &io_mem_ram && mr != &io_mem_rom
-        && mr != &io_mem_notdirty && !mr->rom_device) {
+        && mr != &io_mem_notdirty && !mr->rom_device
+        && mr != &io_mem_watch) {
 #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
         cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
 #else
-- 
1.7.9