diff mbox

[4/4] target-xtensa: fix tb invalidation for IBREAK and LOOP

Message ID 1333666012-16094-5-git-send-email-jcmvbkbc@gmail.com
State New
Headers show

Commit Message

Max Filippov April 5, 2012, 10:46 p.m. UTC 
Instruction breakpoint/zero overhead loop handling code is built into
TBs pointed to by IBREAKA/LEND SRs. When these or related SRs get
changed TBs at virtual addresses corresponding to their old and their
new values must be invalidated.

Virtual address range is passed to the tb_invalidate_phys_page_range,
which is incorrect in system emulation mode.

To fix it use guest TLB/MMU to translate virtual address to physical
address.

However the guest may not have virtual-to-physical mapping at the moment
of IBREAKA/LEND change, thus this fix is not 100% accurate.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 exec-all.h                |    1 +
 exec.c                    |    9 ++++++---
 target-xtensa/op_helper.c |   29 ++++++++++++++++++-----------
 3 files changed, 25 insertions(+), 14 deletions(-)

Comments

Andreas Färber April 7, 2012, 11:24 a.m. UTC | #1 
Am 06.04.2012 00:46, schrieb Max Filippov:
> Instruction breakpoint/zero overhead loop handling code is built into
> TBs pointed to by IBREAKA/LEND SRs. When these or related SRs get
> changed TBs at virtual addresses corresponding to their old and their
> new values must be invalidated.
> 
> Virtual address range is passed to the tb_invalidate_phys_page_range,
> which is incorrect in system emulation mode.
> 
> To fix it use guest TLB/MMU to translate virtual address to physical
> address.
> 
> However the guest may not have virtual-to-physical mapping at the moment
> of IBREAKA/LEND change, thus this fix is not 100% accurate.
> 
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  exec-all.h                |    1 +
>  exec.c                    |    9 ++++++---
>  target-xtensa/op_helper.c |   29 ++++++++++++++++++-----------
>  3 files changed, 25 insertions(+), 14 deletions(-)
[snip]

In addition to the breakage reported by Blue, a subject starting with
"target-xtensa: " gives the impression that it only affects Xtensa.
Please split it in two and wait for an Acked-by on the exec part before
including it in an Xtensa PULL.

Andreas
Max Filippov April 7, 2012, 4:52 p.m. UTC | #2 
>> Instruction breakpoint/zero overhead loop handling code is built into
>> TBs pointed to by IBREAKA/LEND SRs. When these or related SRs get
>> changed TBs at virtual addresses corresponding to their old and their
>> new values must be invalidated.
>>
>> Virtual address range is passed to the tb_invalidate_phys_page_range,
>> which is incorrect in system emulation mode.
>>
>> To fix it use guest TLB/MMU to translate virtual address to physical
>> address.
>>
>> However the guest may not have virtual-to-physical mapping at the moment
>> of IBREAKA/LEND change, thus this fix is not 100% accurate.
>>
>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>> ---
>>  exec-all.h                |    1 +
>>  exec.c                    |    9 ++++++---
>>  target-xtensa/op_helper.c |   29 ++++++++++++++++++-----------
>>  3 files changed, 25 insertions(+), 14 deletions(-)
> [snip]
>
> In addition to the breakage reported by Blue, a subject starting with
> "target-xtensa: " gives the impression that it only affects Xtensa.
> Please split it in two and wait for an Acked-by on the exec part before
> including it in an Xtensa PULL.

You're right, sorry about that.
diff mbox

Patch

diff --git a/exec-all.h b/exec-all.h
index 93a5b22..3da16ca 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -96,6 +96,7 @@  void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1);
 int page_unprotect(target_ulong address, unsigned long pc, void *puc);
 void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
                                    int is_cpu_write_access);
+void tb_invalidate_phys_addr(target_phys_addr_t addr);
 void tlb_flush_page(CPUArchState *env, target_ulong addr);
 void tlb_flush(CPUArchState *env, int flush_global);
 #if !defined(CONFIG_USER_ONLY)
diff --git a/exec.c b/exec.c
index 6731ab8..130ecaa 100644
--- a/exec.c
+++ b/exec.c
@@ -1463,13 +1463,11 @@  static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
     tb_invalidate_phys_page_range(pc, pc + 1, 0);
 }
 #else
-static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
+void tb_invalidate_phys_addr(target_phys_addr_t addr)
 {
-    target_phys_addr_t addr;
     ram_addr_t ram_addr;
     MemoryRegionSection *section;
 
-    addr = cpu_get_phys_page_debug(env, pc);
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
     if (!(memory_region_is_ram(section->mr)
           || (section->mr->rom_device && section->mr->readable))) {
@@ -1479,6 +1477,11 @@  static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
         + section_addr(section, addr);
     tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
 }
+
+static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
+{
+    tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc));
+}
 #endif
 #endif /* TARGET_HAS_ICE */
 
diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
index 806384f..03cc789 100644
--- a/target-xtensa/op_helper.c
+++ b/target-xtensa/op_helper.c
@@ -99,6 +99,18 @@  void tlb_fill(CPUXtensaState *env1, target_ulong vaddr, int is_write, int mmu_id
     env = saved_env;
 }
 
+static void tb_invalidate_virtual_addr(CPUXtensaState *env, uint32_t vaddr)
+{
+    uint32_t paddr;
+    uint32_t page_size;
+    unsigned access;
+    int ret = xtensa_get_physical_addr(env, vaddr, 2, 0,
+            &paddr, &page_size, &access);
+    if (ret == 0) {
+        tb_invalidate_phys_addr(paddr);
+    }
+}
+
 void HELPER(exception)(uint32_t excp)
 {
     env->exception_index = excp;
@@ -358,8 +370,7 @@  void HELPER(movsp)(uint32_t pc)
 void HELPER(wsr_lbeg)(uint32_t v)
 {
     if (env->sregs[LBEG] != v) {
-        tb_invalidate_phys_page_range(
-                env->sregs[LEND] - 1, env->sregs[LEND], 0);
+        tb_invalidate_virtual_addr(env, env->sregs[LEND] - 1);
         env->sregs[LBEG] = v;
     }
 }
@@ -367,11 +378,9 @@  void HELPER(wsr_lbeg)(uint32_t v)
 void HELPER(wsr_lend)(uint32_t v)
 {
     if (env->sregs[LEND] != v) {
-        tb_invalidate_phys_page_range(
-                env->sregs[LEND] - 1, env->sregs[LEND], 0);
+        tb_invalidate_virtual_addr(env, env->sregs[LEND] - 1);
         env->sregs[LEND] = v;
-        tb_invalidate_phys_page_range(
-                env->sregs[LEND] - 1, env->sregs[LEND], 0);
+        tb_invalidate_virtual_addr(env, env->sregs[LEND] - 1);
     }
 }
 
@@ -692,8 +701,7 @@  void HELPER(wsr_ibreakenable)(uint32_t v)
 
     for (i = 0; i < env->config->nibreak; ++i) {
         if (change & (1 << i)) {
-            tb_invalidate_phys_page_range(
-                    env->sregs[IBREAKA + i], env->sregs[IBREAKA + i] + 1, 0);
+            tb_invalidate_virtual_addr(env, env->sregs[IBREAKA + i]);
         }
     }
     env->sregs[IBREAKENABLE] = v & ((1 << env->config->nibreak) - 1);
@@ -702,9 +710,8 @@  void HELPER(wsr_ibreakenable)(uint32_t v)
 void HELPER(wsr_ibreaka)(uint32_t i, uint32_t v)
 {
     if (env->sregs[IBREAKENABLE] & (1 << i) && env->sregs[IBREAKA + i] != v) {
-        tb_invalidate_phys_page_range(
-                env->sregs[IBREAKA + i], env->sregs[IBREAKA + i] + 1, 0);
-        tb_invalidate_phys_page_range(v, v + 1, 0);
+        tb_invalidate_virtual_addr(env, env->sregs[IBREAKA + i]);
+        tb_invalidate_virtual_addr(env, v);
     }
     env->sregs[IBREAKA + i] = v;
 }