diff mbox

Fix unassigned memory access handling

Message ID CAAu8pHvW-6drG+XQstebrOrEkVZkcsacO5YOMwm6Z8M56Xx-iQ@mail.gmail.com
State New
Headers show

Commit Message

Blue Swirl July 3, 2011, 9:42 a.m. UTC 
cea5f9a28faa528b6b1b117c9ab2d8828f473fef exposed bugs in unassigned memory
access handling. Fix them by always passing CPUState to the handlers.

Reported-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 exec-all.h                    |    2 +-
 exec.c                        |   12 ++++++------
 target-alpha/cpu.h            |    5 +++--
 target-alpha/op_helper.c      |   10 ++++++++--
 target-microblaze/cpu.h       |    4 ++--
 target-microblaze/op_helper.c |   14 ++++----------
 target-mips/cpu.h             |    4 ++--
 target-mips/op_helper.c       |   10 ++++++++--
 target-sparc/cpu.h            |    4 ++--
 target-sparc/op_helper.c      |   31 ++++++++++++++++++++++++-------
 10 files changed, 60 insertions(+), 36 deletions(-)

     int fault_type;
@@ -4272,8 +4277,8 @@ void do_unassigned_access(target_phys_addr_t
addr, int is_write, int is_exec,
 static void do_unassigned_access(target_ulong addr, int is_write, int is_exec,
                           int is_asi, int size)
 #else
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int is_asi, int size)
+static void do_unassigned_access(target_phys_addr_t addr, int is_write,
+                                 int is_exec, int is_asi, int size)
 #endif
 {
     CPUState *saved_env;
@@ -4297,7 +4302,6 @@ void do_unassigned_access(target_phys_addr_t
addr, int is_write, int is_exec,
 }
 #endif

-
 #ifdef TARGET_SPARC64
 void helper_tick_set_count(void *opaque, uint64_t count)
 {
@@ -4322,3 +4326,16 @@ void helper_tick_set_limit(void *opaque, uint64_t limit)
 #endif
 }
 #endif
+
+#if !defined(CONFIG_USER_ONLY)
+void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
+                           int is_write, int is_exec, int is_asi, int size)
+{
+    CPUState *saved_env;
+
+    saved_env = env;
+    env = env1;
+    do_unassigned_access(addr, is_write, is_exec, is_asi, size);
+    env = saved_env;
+}
+#endif

Comments

Richard Henderson July 3, 2011, 4:08 p.m. UTC | #1 
On 07/03/2011 02:42 AM, Blue Swirl wrote:
>  }
> 
> -void QEMU_NORETURN do_unassigned_access(target_phys_addr_t addr, int is_write,
> -                                        int is_exec, int unused, int size)
> +void QEMU_NORETURN cpu_unassigned_access(CPUState *env1,
> +                                         target_phys_addr_t addr, int is_write,
> +                                         int is_exec, int unused, int size)
>  {
> +    CPUState *saved_env;
> +
> +    saved_env = env;
> +    env = env1;
>      env->trap_arg0 = addr;
>      env->trap_arg1 = is_write;
>      dynamic_excp(EXCP_MCHK, 0);
> +    env = saved_env;
>  }

For Alpha and MIPS, these functions always throw an exception exiting
the cpu loop.  There's no point in saving the old value of ENV.

It's Sparc and Microblaze that only sometimes throw the exception.


r~
diff mbox

Patch

From c6a67abaf0a0851ef5723115f6670d061a61adac Mon Sep 17 00:00:00 2001
Message-Id: <c6a67abaf0a0851ef5723115f6670d061a61adac.1309685959.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sun, 3 Jul 2011 08:53:46 +0000
Subject: [PATCH] Fix unassigned memory access handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

cea5f9a28faa528b6b1b117c9ab2d8828f473fef exposed bugs in unassigned memory
access handling. Fix them by always passing CPUState to the handlers.

Reported-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 exec-all.h                    |    2 +-
 exec.c                        |   12 ++++++------
 target-alpha/cpu.h            |    5 +++--
 target-alpha/op_helper.c      |   10 ++++++++--
 target-microblaze/cpu.h       |    4 ++--
 target-microblaze/op_helper.c |   14 ++++----------
 target-mips/cpu.h             |    4 ++--
 target-mips/op_helper.c       |   10 ++++++++--
 target-sparc/cpu.h            |    4 ++--
 target-sparc/op_helper.c      |   31 ++++++++++++++++++++++++-------
 10 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 21a69d6..c24518e 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -327,7 +327,7 @@  static inline tb_page_addr_t get_page_addr_code(CPUState *env1, target_ulong add
     pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK;
     if (pd > IO_MEM_ROM && !(pd & IO_MEM_ROMD)) {
 #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
-        do_unassigned_access(addr, 0, 1, 0, 4);
+        cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
 #else
         cpu_abort(env1, "Trying to execute code outside RAM or ROM at 0x" TARGET_FMT_lx "\n", addr);
 #endif
diff --git a/exec.c b/exec.c
index 4c45299..4d9d92b 100644
--- a/exec.c
+++ b/exec.c
@@ -3236,7 +3236,7 @@  static uint32_t unassigned_mem_readb(void *opaque, target_phys_addr_t addr)
     printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
 #endif
 #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    do_unassigned_access(addr, 0, 0, 0, 1);
+    cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 1);
 #endif
     return 0;
 }
@@ -3247,7 +3247,7 @@  static uint32_t unassigned_mem_readw(void *opaque, target_phys_addr_t addr)
     printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
 #endif
 #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    do_unassigned_access(addr, 0, 0, 0, 2);
+    cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 2);
 #endif
     return 0;
 }
@@ -3258,7 +3258,7 @@  static uint32_t unassigned_mem_readl(void *opaque, target_phys_addr_t addr)
     printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
 #endif
 #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    do_unassigned_access(addr, 0, 0, 0, 4);
+    cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 4);
 #endif
     return 0;
 }
@@ -3269,7 +3269,7 @@  static void unassigned_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_
     printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
 #endif
 #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    do_unassigned_access(addr, 1, 0, 0, 1);
+    cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 1);
 #endif
 }
 
@@ -3279,7 +3279,7 @@  static void unassigned_mem_writew(void *opaque, target_phys_addr_t addr, uint32_
     printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
 #endif
 #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    do_unassigned_access(addr, 1, 0, 0, 2);
+    cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 2);
 #endif
 }
 
@@ -3289,7 +3289,7 @@  static void unassigned_mem_writel(void *opaque, target_phys_addr_t addr, uint32_
     printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
 #endif
 #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    do_unassigned_access(addr, 1, 0, 0, 4);
+    cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 4);
 #endif
 }
 
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 411bd55..ccdcd1b 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -436,8 +436,9 @@  uint64_t cpu_alpha_load_fpcr (CPUState *env);
 void cpu_alpha_store_fpcr (CPUState *env, uint64_t val);
 #ifndef CONFIG_USER_ONLY
 void swap_shadow_regs(CPUState *env);
-extern QEMU_NORETURN void do_unassigned_access(target_phys_addr_t addr,
-                                               int, int, int, int);
+QEMU_NORETURN void cpu_unassigned_access(CPUState *env1,
+                                         target_phys_addr_t addr, int is_write,
+                                         int is_exec, int unused, int size);
 #endif
 
 /* Bits in TB->FLAGS that control how translation is processed.  */
diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index 51d1bd7..90f3c75 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -1301,12 +1301,18 @@  static void QEMU_NORETURN do_unaligned_access(target_ulong addr, int is_write,
     helper_excp(EXCP_UNALIGN, 0);
 }
 
-void QEMU_NORETURN do_unassigned_access(target_phys_addr_t addr, int is_write,
-                                        int is_exec, int unused, int size)
+void QEMU_NORETURN cpu_unassigned_access(CPUState *env1,
+                                         target_phys_addr_t addr, int is_write,
+                                         int is_exec, int unused, int size)
 {
+    CPUState *saved_env;
+
+    saved_env = env;
+    env = env1;
     env->trap_arg0 = addr;
     env->trap_arg1 = is_write;
     dynamic_excp(EXCP_MCHK, 0);
+    env = saved_env;
 }
 
 #define MMUSUFFIX _mmu
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index 51a13e3..76f4fc4 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -347,8 +347,8 @@  static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 }
 
 #if !defined(CONFIG_USER_ONLY)
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int is_asi, int size);
+void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
+                           int is_write, int is_exec, int is_asi, int size);
 #endif
 
 static inline bool cpu_has_work(CPUState *env)
diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
index 1a0a476..664ffe5 100644
--- a/target-microblaze/op_helper.c
+++ b/target-microblaze/op_helper.c
@@ -488,20 +488,14 @@  void helper_mmu_write(uint32_t rn, uint32_t v)
     mmu_write(env, rn, v);
 }
 
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int is_asi, int size)
+void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
+                           int is_write, int is_exec, int is_asi, int size)
 {
     CPUState *saved_env;
 
-    if (!cpu_single_env) {
-        /* XXX: ???   */
-        return;
-    }
-
-    /* XXX: hack to restore env in all cases, even if not called from
-       generated code */
     saved_env = env;
-    env = cpu_single_env;
+    env = env1;
+
     qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n",
              addr, is_write, is_exec);
     if (!(env->sregs[SR_MSR] & MSR_EE)) {
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index b0ac4da..33be296 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -493,8 +493,8 @@  void r4k_helper_tlbwr (void);
 void r4k_helper_tlbp (void);
 void r4k_helper_tlbr (void);
 
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int unused, int size);
+void cpu_unassigned_access(CPUState *env, target_phys_addr_t addr,
+                           int is_write, int is_exec, int unused, int size);
 #endif
 
 void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf);
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 6b966b1..f47f052 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1980,13 +1980,19 @@  void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
     env = saved_env;
 }
 
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int unused, int size)
+void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
+                           int is_write, int is_exec, int unused, int size)
 {
+    CPUState *saved_env;
+
+    saved_env = env;
+    env = env1;
+
     if (is_exec)
         helper_raise_exception(EXCP_IBE);
     else
         helper_raise_exception(EXCP_DBE);
+    env = saved_env;
 }
 #endif /* !CONFIG_USER_ONLY */
 
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 4edae78..f6cb300 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -510,8 +510,8 @@  static inline int tlb_compare_context(const SparcTLBEntry *tlb,
 
 /* cpu-exec.c */
 #if !defined(CONFIG_USER_ONLY)
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int is_asi, int size);
+void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
+                           int is_write, int is_exec, int is_asi, int size);
 target_phys_addr_t cpu_get_phys_page_nofault(CPUState *env, target_ulong addr,
                                            int mmu_idx);
 
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index fd0cfbd..fc1e5da 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -79,9 +79,14 @@ 
 #define CACHE_CTRL_FD (1 << 22)  /* Flush Data cache (Write only) */
 #define CACHE_CTRL_DS (1 << 23)  /* Data cache snoop enable */
 
-#if defined(CONFIG_USER_ONLY) && defined(TARGET_SPARC64)
+#if !defined(CONFIG_USER_ONLY)
+static void do_unassigned_access(target_phys_addr_t addr, int is_write,
+                                 int is_exec, int is_asi, int size);
+#else
+#ifdef TARGET_SPARC64
 static void do_unassigned_access(target_ulong addr, int is_write, int is_exec,
-                          int is_asi, int size);
+                                 int is_asi, int size);
+#endif
 #endif
 
 #if defined(TARGET_SPARC64) && !defined(CONFIG_USER_ONLY)
@@ -4206,8 +4211,8 @@  void tlb_fill(target_ulong addr, int is_write, int mmu_idx, void *retaddr)
 
 #ifndef TARGET_SPARC64
 #if !defined(CONFIG_USER_ONLY)
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int is_asi, int size)
+static void do_unassigned_access(target_phys_addr_t addr, int is_write,
+                                 int is_exec, int is_asi, int size)
 {
     CPUState *saved_env;
     int fault_type;
@@ -4272,8 +4277,8 @@  void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
 static void do_unassigned_access(target_ulong addr, int is_write, int is_exec,
                           int is_asi, int size)
 #else
-void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
-                          int is_asi, int size)
+static void do_unassigned_access(target_phys_addr_t addr, int is_write,
+                                 int is_exec, int is_asi, int size)
 #endif
 {
     CPUState *saved_env;
@@ -4297,7 +4302,6 @@  void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
 }
 #endif
 
-
 #ifdef TARGET_SPARC64
 void helper_tick_set_count(void *opaque, uint64_t count)
 {
@@ -4322,3 +4326,16 @@  void helper_tick_set_limit(void *opaque, uint64_t limit)
 #endif
 }
 #endif
+
+#if !defined(CONFIG_USER_ONLY)
+void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
+                           int is_write, int is_exec, int is_asi, int size)
+{
+    CPUState *saved_env;
+
+    saved_env = env;
+    env = env1;
+    do_unassigned_access(addr, is_write, is_exec, is_asi, size);
+    env = saved_env;
+}
+#endif
-- 
1.7.2.5