[PATCH-for-9.1 0/8] target/microblaze: Sprint housekeeping

[PATCH-for-9.1 0/8] target/microblaze: Sprint housekeeping

[Philippe Mathieu-Daudé]

Cleanups while trying to remove non-essential target_ulong uses.

Last use is in helper_stackprot(), but I couldn't figure what
to do there yet.

Philippe Mathieu-Daudé (8):
  target/microblaze: Use correct string format in do_unaligned_access()
  target/microblaze: Use hwaddr/vaddr in cpu_get_phys_page_attrs_debug()
  target/microblaze: Widen vaddr in mmu_translate()
  target/microblaze: Use 32-bit destination in gen_goto_tb()
  target/microblaze: Restrict 64-bit 'res_addr' to system emulation
  target/microblaze: Rename helper.c -> sys_helper.c
  target/microblaze: Move MMU helpers to sys_helper.c
  target/microblaze: Widen $ear to 64-bit

 target/microblaze/cpu.h                      | 12 ++--
 target/microblaze/mmu.h                      |  2 +-
 linux-user/elfload.c                         |  2 +-
 target/microblaze/cpu.c                      |  2 +
 target/microblaze/gdbstub.c                  |  2 +-
 target/microblaze/machine.c                  |  2 +-
 target/microblaze/mmu.c                      |  2 +-
 target/microblaze/op_helper.c                | 48 ----------------
 target/microblaze/{helper.c => sys_helper.c} | 59 +++++++++++++++++---
 target/microblaze/translate.c                | 13 +++--
 target/microblaze/meson.build                |  2 +-
 11 files changed, 75 insertions(+), 71 deletions(-)
 rename target/microblaze/{helper.c => sys_helper.c} (84%)

-- 
2.41.0

[PATCH-for-9.1 1/8] target/microblaze: Use correct string format in do_unaligned_access()

[Philippe Mathieu-Daudé]

'addr' is of type 'vaddr'; no need to cast, use the
corresponding format string.

Fixes: ab0c8d0f5b ("target/microblaze: Use cc->do_unaligned_access")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index d25c9eb4d3..0a12c4ea94 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -279,8 +279,8 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     iflags = cpu->env.iflags;
 
     qemu_log_mask(CPU_LOG_INT,
-                  "Unaligned access addr=" TARGET_FMT_lx " pc=%x iflags=%x\n",
-                  (target_ulong)addr, cpu->env.pc, iflags);
+                  "Unaligned access addr=0x%"VADDR_PRIx" pc=0x%x iflags=0x%x\n",
+                  addr, cpu->env.pc, iflags);
 
     esr = ESR_EC_UNALIGNED_DATA;
     if (likely(iflags & ESR_ESS_FLAG)) {
-- 
2.41.0

[PATCH-for-9.1 2/8] target/microblaze: Use hwaddr/vaddr in cpu_get_phys_page_attrs_debug()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index 0a12c4ea94..3f410fc7b5 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -228,7 +228,8 @@ hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
                                         MemTxAttrs *attrs)
 {
     MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
-    target_ulong vaddr, paddr = 0;
+    vaddr vaddr;
+    hwaddr paddr = 0;
     MicroBlazeMMULookup lu;
     int mmu_idx = cpu_mmu_index(cs, false);
     unsigned int hit;
-- 
2.41.0

[PATCH-for-9.1 3/8] target/microblaze: Widen vaddr in mmu_translate()

[Philippe Mathieu-Daudé]

Use 'vaddr' type for virtual addresses.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/mmu.h | 2 +-
 target/microblaze/mmu.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/microblaze/mmu.h b/target/microblaze/mmu.h
index 1068bd2d52..2aca39c923 100644
--- a/target/microblaze/mmu.h
+++ b/target/microblaze/mmu.h
@@ -86,7 +86,7 @@ typedef struct {
 } MicroBlazeMMULookup;
 
 unsigned int mmu_translate(MicroBlazeCPU *cpu, MicroBlazeMMULookup *lu,
-                           target_ulong vaddr, MMUAccessType rw, int mmu_idx);
+                           vaddr vaddr, MMUAccessType rw, int mmu_idx);
 uint32_t mmu_read(CPUMBState *env, bool ea, uint32_t rn);
 void mmu_write(CPUMBState *env, bool ea, uint32_t rn, uint32_t v);
 void mmu_init(MicroBlazeMMU *mmu);
diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index 234006634e..eb7c683020 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -75,7 +75,7 @@ static void mmu_change_pid(CPUMBState *env, unsigned int newpid)
 
 /* rw - 0 = read, 1 = write, 2 = fetch.  */
 unsigned int mmu_translate(MicroBlazeCPU *cpu, MicroBlazeMMULookup *lu,
-                           target_ulong vaddr, MMUAccessType rw, int mmu_idx)
+                           vaddr vaddr, MMUAccessType rw, int mmu_idx)
 {
     MicroBlazeMMU *mmu = &cpu->env.mmu;
     unsigned int i, hit = 0;
-- 
2.41.0

[PATCH-for-9.1 4/8] target/microblaze: Use 32-bit destination in gen_goto_tb()

[Philippe Mathieu-Daudé]

cpu_pc and jmp_dest are 32-bit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 4e52ef32db..d6a42381bb 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -121,7 +121,7 @@ static void gen_raise_hw_excp(DisasContext *dc, uint32_t esr_ec)
     gen_raise_exception_sync(dc, EXCP_HW_EXCP);
 }
 
-static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
+static void gen_goto_tb(DisasContext *dc, int n, uint32_t dest)
 {
     if (translator_use_goto_tb(&dc->base, dest)) {
         tcg_gen_goto_tb(n);
-- 
2.41.0

[PATCH-for-9.1 5/8] target/microblaze: Restrict 64-bit 'res_addr' to system emulation

[Philippe Mathieu-Daudé]

'res_addr' is only used in system emulation, where we have
TARGET_LONG_BITS = 64, so we can directly use the native
uint64_t type instead of target_ulong.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/cpu.h       | 10 +++++-----
 target/microblaze/cpu.c       |  2 ++
 target/microblaze/machine.c   |  2 +-
 target/microblaze/translate.c |  9 +++++++--
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index c0c7574dbd..c3e2aba0ec 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -260,11 +260,6 @@ struct CPUArchState {
     /* Stack protectors. Yes, it's a hw feature.  */
     uint32_t slr, shr;
 
-    /* lwx/swx reserved address */
-#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no reservation */
-    target_ulong res_addr;
-    uint32_t res_val;
-
     /* Internal flags.  */
 #define IMM_FLAG        (1 << 0)
 #define BIMM_FLAG       (1 << 1)
@@ -286,6 +281,11 @@ struct CPUArchState {
     uint32_t iflags;
 
 #if !defined(CONFIG_USER_ONLY)
+    /* lwx/swx reserved address */
+#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no reservation */
+    uint64_t res_addr;
+    uint32_t res_val;
+
     /* Unified MMU.  */
     MicroBlazeMMU mmu;
 #endif
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 96c2b71f7f..9e393cf217 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -193,7 +193,9 @@ static void mb_cpu_reset_hold(Object *obj)
     }
 
     memset(env, 0, offsetof(CPUMBState, end_reset_fields));
+#ifndef CONFIG_USER_ONLY
     env->res_addr = RES_ADDR_NONE;
+#endif
 
     /* Disable stack protector.  */
     env->shr = ~0;
diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
index 51705e4f5c..4daf8a2471 100644
--- a/target/microblaze/machine.c
+++ b/target/microblaze/machine.c
@@ -78,7 +78,7 @@ static const VMStateField vmstate_env_fields[] = {
     VMSTATE_UINT32(iflags, CPUMBState),
 
     VMSTATE_UINT32(res_val, CPUMBState),
-    VMSTATE_UINTTL(res_addr, CPUMBState),
+    VMSTATE_UINT64(res_addr, CPUMBState),
 
     VMSTATE_STRUCT(mmu, CPUMBState, 0, vmstate_mmu, MicroBlazeMMU),
 
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index d6a42381bb..493850c544 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1872,7 +1872,9 @@ void mb_tcg_init(void)
         SP(iflags),
         SP(bvalue),
         SP(btarget),
+#if !defined(CONFIG_USER_ONLY)
         SP(res_val),
+#endif
     };
 
 #undef R
@@ -1883,6 +1885,9 @@ void mb_tcg_init(void)
           tcg_global_mem_new_i32(tcg_env, i32s[i].ofs, i32s[i].name);
     }
 
-    cpu_res_addr =
-        tcg_global_mem_new(tcg_env, offsetof(CPUMBState, res_addr), "res_addr");
+#if !defined(CONFIG_USER_ONLY)
+    cpu_res_addr = tcg_global_mem_new_i64(tcg_env,
+                                          offsetof(CPUMBState, res_addr),
+                                          "res_addr");
+#endif
 }
-- 
2.41.0

[PATCH-for-9.1 6/8] target/microblaze: Rename helper.c -> sys_helper.c

[Philippe Mathieu-Daudé]

helper.c only contains system emulation helpers,
rename it as sys_helper.c.
Adapt meson and remove pointless #ifdef'ry.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/{helper.c => sys_helper.c} | 5 +----
 target/microblaze/meson.build                | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)
 rename target/microblaze/{helper.c => sys_helper.c} (99%)

diff --git a/target/microblaze/helper.c b/target/microblaze/sys_helper.c
similarity index 99%
rename from target/microblaze/helper.c
rename to target/microblaze/sys_helper.c
index 3f410fc7b5..5180500354 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/sys_helper.c
@@ -1,5 +1,5 @@
 /*
- *  MicroBlaze helper routines.
+ *  MicroBlaze system helper routines.
  *
  *  Copyright (c) 2009 Edgar E. Iglesias <edgar.iglesias@gmail.com>
  *  Copyright (c) 2009-2012 PetaLogix Qld Pty Ltd.
@@ -24,7 +24,6 @@
 #include "qemu/host-utils.h"
 #include "exec/log.h"
 
-#ifndef CONFIG_USER_ONLY
 static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
                                     MMUAccessType access_type)
 {
@@ -266,8 +265,6 @@ bool mb_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
     return false;
 }
 
-#endif /* !CONFIG_USER_ONLY */
-
 void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                 MMUAccessType access_type,
                                 int mmu_idx, uintptr_t retaddr)
diff --git a/target/microblaze/meson.build b/target/microblaze/meson.build
index 3ed4fbb67a..013ea542be 100644
--- a/target/microblaze/meson.build
+++ b/target/microblaze/meson.build
@@ -5,7 +5,6 @@ microblaze_ss.add(gen)
 microblaze_ss.add(files(
   'cpu.c',
   'gdbstub.c',
-  'helper.c',
   'op_helper.c',
   'translate.c',
 ))
@@ -14,6 +13,7 @@ microblaze_system_ss = ss.source_set()
 microblaze_system_ss.add(files(
   'mmu.c',
   'machine.c',
+  'sys_helper.c',
 ))
 
 target_arch += {'microblaze': microblaze_ss}
-- 
2.41.0

[PATCH-for-9.1 7/8] target/microblaze: Move MMU helpers to sys_helper.c

[Philippe Mathieu-Daudé]

MMU helpers are only used during system emulation,
move them to sys_helper.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/op_helper.c  | 48 ----------------------------------
 target/microblaze/sys_helper.c | 47 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 48 deletions(-)

diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index f6378030b7..45dbed4aaa 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -381,51 +381,3 @@ void helper_stackprot(CPUMBState *env, target_ulong addr)
         cpu_loop_exit_restore(cs, GETPC());
     }
 }
-
-#if !defined(CONFIG_USER_ONLY)
-/* Writes/reads to the MMU's special regs end up here.  */
-uint32_t helper_mmu_read(CPUMBState *env, uint32_t ext, uint32_t rn)
-{
-    return mmu_read(env, ext, rn);
-}
-
-void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
-{
-    mmu_write(env, ext, rn, v);
-}
-
-void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
-                               unsigned size, MMUAccessType access_type,
-                               int mmu_idx, MemTxAttrs attrs,
-                               MemTxResult response, uintptr_t retaddr)
-{
-    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
-    CPUMBState *env = &cpu->env;
-
-    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
-                  " physaddr 0x" HWADDR_FMT_plx " size %d access type %s\n",
-                  addr, physaddr, size,
-                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
-                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
-
-    if (!(env->msr & MSR_EE)) {
-        return;
-    }
-
-    if (access_type == MMU_INST_FETCH) {
-        if (!cpu->cfg.iopb_bus_exception) {
-            return;
-        }
-        env->esr = ESR_EC_INSN_BUS;
-    } else {
-        if (!cpu->cfg.dopb_bus_exception) {
-            return;
-        }
-        env->esr = ESR_EC_DATA_BUS;
-    }
-
-    env->ear = addr;
-    cs->exception_index = EXCP_HW_EXCP;
-    cpu_loop_exit_restore(cs, retaddr);
-}
-#endif
diff --git a/target/microblaze/sys_helper.c b/target/microblaze/sys_helper.c
index 5180500354..7531f95ca7 100644
--- a/target/microblaze/sys_helper.c
+++ b/target/microblaze/sys_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
+#include "exec/helper-proto.h"
 #include "qemu/host-utils.h"
 #include "exec/log.h"
 
@@ -292,3 +293,49 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     cs->exception_index = EXCP_HW_EXCP;
     cpu_loop_exit(cs);
 }
+
+/* Writes/reads to the MMU's special regs end up here.  */
+uint32_t helper_mmu_read(CPUMBState *env, uint32_t ext, uint32_t rn)
+{
+    return mmu_read(env, ext, rn);
+}
+
+void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
+{
+    mmu_write(env, ext, rn, v);
+}
+
+void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
+                               unsigned size, MMUAccessType access_type,
+                               int mmu_idx, MemTxAttrs attrs,
+                               MemTxResult response, uintptr_t retaddr)
+{
+    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+    CPUMBState *env = &cpu->env;
+
+    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
+                  " physaddr 0x" HWADDR_FMT_plx " size %d access type %s\n",
+                  addr, physaddr, size,
+                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
+                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
+
+    if (!(env->msr & MSR_EE)) {
+        return;
+    }
+
+    if (access_type == MMU_INST_FETCH) {
+        if (!cpu->cfg.iopb_bus_exception) {
+            return;
+        }
+        env->esr = ESR_EC_INSN_BUS;
+    } else {
+        if (!cpu->cfg.dopb_bus_exception) {
+            return;
+        }
+        env->esr = ESR_EC_DATA_BUS;
+    }
+
+    env->ear = addr;
+    cs->exception_index = EXCP_HW_EXCP;
+    cpu_loop_exit_restore(cs, retaddr);
+}
-- 
2.41.0

[RFC PATCH-for-9.1 8/8] target/microblaze: Widen $ear to 64-bit

[Philippe Mathieu-Daudé]

The Exception Address Register is 64-bit wide.
User emulation only access the 32 lower bits.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/microblaze/cpu.h       | 2 +-
 linux-user/elfload.c          | 2 +-
 target/microblaze/gdbstub.c   | 2 +-
 target/microblaze/translate.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index c3e2aba0ec..a9f93b37b7 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -251,7 +251,7 @@ struct CPUArchState {
     uint32_t pc;
     uint32_t msr;    /* All bits of MSR except MSR[C] and MSR[CC] */
     uint32_t msr_c;  /* MSR[C], in low bit; other bits must be 0 */
-    target_ulong ear;
+    uint64_t ear;
     uint32_t esr;
     uint32_t fsr;
     uint32_t btr;
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 60cf55b36c..4612aef95a 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1498,7 +1498,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
     (*regs)[pos++] = tswapreg(env->pc);
     (*regs)[pos++] = tswapreg(mb_cpu_read_msr(env));
     (*regs)[pos++] = 0;
-    (*regs)[pos++] = tswapreg(env->ear);
+    (*regs)[pos++] = tswapreg((uint32_t)env->ear);
     (*regs)[pos++] = 0;
     (*regs)[pos++] = tswapreg(env->esr);
 }
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index 09d74e164d..147d20c3e4 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -63,7 +63,7 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
         val = mb_cpu_read_msr(env);
         break;
     case GDB_EAR:
-        val = env->ear;
+        val = (uint32_t)env->ear;
         break;
     case GDB_ESR:
         val = env->esr;
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 493850c544..19b180501f 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1835,7 +1835,7 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     }
 
     qemu_fprintf(f, "\nesr=0x%04x fsr=0x%02x btr=0x%08x edr=0x%x\n"
-                 "ear=0x" TARGET_FMT_lx " slr=0x%x shr=0x%x\n",
+                 "ear=0x%" PRIx64 " slr=0x%x shr=0x%x\n",
                  env->esr, env->fsr, env->btr, env->edr,
                  env->ear, env->slr, env->shr);
 
-- 
2.41.0

Re: [PATCH-for-9.1 0/8] target/microblaze: Sprint housekeeping

[Philippe Mathieu-Daudé]

s/sprint/spring/ in Subject ;)

On 19/3/24 07:28, Philippe Mathieu-Daudé wrote:
> Cleanups while trying to remove non-essential target_ulong uses.
> 
> Last use is in helper_stackprot(), but I couldn't figure what
> to do there yet.
> 
> Philippe Mathieu-Daudé (8):
>    target/microblaze: Use correct string format in do_unaligned_access()
>    target/microblaze: Use hwaddr/vaddr in cpu_get_phys_page_attrs_debug()
>    target/microblaze: Widen vaddr in mmu_translate()
>    target/microblaze: Use 32-bit destination in gen_goto_tb()
>    target/microblaze: Restrict 64-bit 'res_addr' to system emulation
>    target/microblaze: Rename helper.c -> sys_helper.c
>    target/microblaze: Move MMU helpers to sys_helper.c
>    target/microblaze: Widen $ear to 64-bit

Re: [PATCH-for-9.1 1/8] target/microblaze: Use correct string format in do_unaligned_access()

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> 'addr' is of type 'vaddr'; no need to cast, use the
> corresponding format string.
> 
> Fixes: ab0c8d0f5b ("target/microblaze: Use cc->do_unaligned_access")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index d25c9eb4d3..0a12c4ea94 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -279,8 +279,8 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      iflags = cpu->env.iflags;
>  
>      qemu_log_mask(CPU_LOG_INT,
> -                  "Unaligned access addr=" TARGET_FMT_lx " pc=%x iflags=%x\n",
> -                  (target_ulong)addr, cpu->env.pc, iflags);
> +                  "Unaligned access addr=0x%"VADDR_PRIx" pc=0x%x iflags=0x%x\n",
> +                  addr, cpu->env.pc, iflags);
>  
>      esr = ESR_EC_UNALIGNED_DATA;
>      if (likely(iflags & ESR_ESS_FLAG)) {
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-9.1 2/8] target/microblaze: Use hwaddr/vaddr in cpu_get_phys_page_attrs_debug()

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index 0a12c4ea94..3f410fc7b5 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -228,7 +228,8 @@ hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
>                                          MemTxAttrs *attrs)
>  {
>      MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> -    target_ulong vaddr, paddr = 0;
> +    vaddr vaddr;
> +    hwaddr paddr = 0;
>      MicroBlazeMMULookup lu;
>      int mmu_idx = cpu_mmu_index(cs, false);
>      unsigned int hit;
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-9.1 3/8] target/microblaze: Widen vaddr in mmu_translate()

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> Use 'vaddr' type for virtual addresses.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/mmu.h | 2 +-
>  target/microblaze/mmu.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/mmu.h b/target/microblaze/mmu.h
> index 1068bd2d52..2aca39c923 100644
> --- a/target/microblaze/mmu.h
> +++ b/target/microblaze/mmu.h
> @@ -86,7 +86,7 @@ typedef struct {
>  } MicroBlazeMMULookup;
>  
>  unsigned int mmu_translate(MicroBlazeCPU *cpu, MicroBlazeMMULookup *lu,
> -                           target_ulong vaddr, MMUAccessType rw, int mmu_idx);
> +                           vaddr vaddr, MMUAccessType rw, int mmu_idx);
>  uint32_t mmu_read(CPUMBState *env, bool ea, uint32_t rn);
>  void mmu_write(CPUMBState *env, bool ea, uint32_t rn, uint32_t v);
>  void mmu_init(MicroBlazeMMU *mmu);
> diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
> index 234006634e..eb7c683020 100644
> --- a/target/microblaze/mmu.c
> +++ b/target/microblaze/mmu.c
> @@ -75,7 +75,7 @@ static void mmu_change_pid(CPUMBState *env, unsigned int newpid)
>  
>  /* rw - 0 = read, 1 = write, 2 = fetch.  */
>  unsigned int mmu_translate(MicroBlazeCPU *cpu, MicroBlazeMMULookup *lu,
> -                           target_ulong vaddr, MMUAccessType rw, int mmu_idx)
> +                           vaddr vaddr, MMUAccessType rw, int mmu_idx)
>  {
>      MicroBlazeMMU *mmu = &cpu->env.mmu;
>      unsigned int i, hit = 0;
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-9.1 4/8] target/microblaze: Use 32-bit destination in gen_goto_tb()

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> cpu_pc and jmp_dest are 32-bit.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 4e52ef32db..d6a42381bb 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -121,7 +121,7 @@ static void gen_raise_hw_excp(DisasContext *dc, uint32_t esr_ec)
>      gen_raise_exception_sync(dc, EXCP_HW_EXCP);
>  }
>  
> -static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
> +static void gen_goto_tb(DisasContext *dc, int n, uint32_t dest)
>  {
>      if (translator_use_goto_tb(&dc->base, dest)) {
>          tcg_gen_goto_tb(n);
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-9.1 5/8] target/microblaze: Restrict 64-bit 'res_addr' to system emulation

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> 'res_addr' is only used in system emulation, where we have
> TARGET_LONG_BITS = 64, so we can directly use the native
> uint64_t type instead of target_ulong.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/cpu.h       | 10 +++++-----
>  target/microblaze/cpu.c       |  2 ++
>  target/microblaze/machine.c   |  2 +-
>  target/microblaze/translate.c |  9 +++++++--
>  4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index c0c7574dbd..c3e2aba0ec 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -260,11 +260,6 @@ struct CPUArchState {
>      /* Stack protectors. Yes, it's a hw feature.  */
>      uint32_t slr, shr;
>  
> -    /* lwx/swx reserved address */
> -#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no reservation */
> -    target_ulong res_addr;
> -    uint32_t res_val;
> -
>      /* Internal flags.  */
>  #define IMM_FLAG        (1 << 0)
>  #define BIMM_FLAG       (1 << 1)
> @@ -286,6 +281,11 @@ struct CPUArchState {
>      uint32_t iflags;
>  
>  #if !defined(CONFIG_USER_ONLY)
> +    /* lwx/swx reserved address */
> +#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no reservation */
> +    uint64_t res_addr;
> +    uint32_t res_val;
> +
>      /* Unified MMU.  */
>      MicroBlazeMMU mmu;
>  #endif
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 96c2b71f7f..9e393cf217 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -193,7 +193,9 @@ static void mb_cpu_reset_hold(Object *obj)
>      }
>  
>      memset(env, 0, offsetof(CPUMBState, end_reset_fields));
> +#ifndef CONFIG_USER_ONLY
>      env->res_addr = RES_ADDR_NONE;
> +#endif
>  
>      /* Disable stack protector.  */
>      env->shr = ~0;
> diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
> index 51705e4f5c..4daf8a2471 100644
> --- a/target/microblaze/machine.c
> +++ b/target/microblaze/machine.c
> @@ -78,7 +78,7 @@ static const VMStateField vmstate_env_fields[] = {
>      VMSTATE_UINT32(iflags, CPUMBState),
>  
>      VMSTATE_UINT32(res_val, CPUMBState),
> -    VMSTATE_UINTTL(res_addr, CPUMBState),
> +    VMSTATE_UINT64(res_addr, CPUMBState),
>  
>      VMSTATE_STRUCT(mmu, CPUMBState, 0, vmstate_mmu, MicroBlazeMMU),
>  
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index d6a42381bb..493850c544 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1872,7 +1872,9 @@ void mb_tcg_init(void)
>          SP(iflags),
>          SP(bvalue),
>          SP(btarget),
> +#if !defined(CONFIG_USER_ONLY)
>          SP(res_val),
> +#endif
>      };
>  
>  #undef R
> @@ -1883,6 +1885,9 @@ void mb_tcg_init(void)
>            tcg_global_mem_new_i32(tcg_env, i32s[i].ofs, i32s[i].name);
>      }
>  
> -    cpu_res_addr =
> -        tcg_global_mem_new(tcg_env, offsetof(CPUMBState, res_addr), "res_addr");
> +#if !defined(CONFIG_USER_ONLY)
> +    cpu_res_addr = tcg_global_mem_new_i64(tcg_env,
> +                                          offsetof(CPUMBState, res_addr),
> +                                          "res_addr");
> +#endif
>  }
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-9.1 6/8] target/microblaze: Rename helper.c -> sys_helper.c

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> helper.c only contains system emulation helpers,
> rename it as sys_helper.c.
> Adapt meson and remove pointless #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/{helper.c => sys_helper.c} | 5 +----
>  target/microblaze/meson.build                | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
>  rename target/microblaze/{helper.c => sys_helper.c} (99%)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/sys_helper.c
> similarity index 99%
> rename from target/microblaze/helper.c
> rename to target/microblaze/sys_helper.c
> index 3f410fc7b5..5180500354 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/sys_helper.c
> @@ -1,5 +1,5 @@
>  /*
> - *  MicroBlaze helper routines.
> + *  MicroBlaze system helper routines.
>   *
>   *  Copyright (c) 2009 Edgar E. Iglesias <edgar.iglesias@gmail.com>
>   *  Copyright (c) 2009-2012 PetaLogix Qld Pty Ltd.
> @@ -24,7 +24,6 @@
>  #include "qemu/host-utils.h"
>  #include "exec/log.h"
>  
> -#ifndef CONFIG_USER_ONLY
>  static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
>                                      MMUAccessType access_type)
>  {
> @@ -266,8 +265,6 @@ bool mb_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>      return false;
>  }
>  
> -#endif /* !CONFIG_USER_ONLY */
> -
>  void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>                                  MMUAccessType access_type,
>                                  int mmu_idx, uintptr_t retaddr)
> diff --git a/target/microblaze/meson.build b/target/microblaze/meson.build
> index 3ed4fbb67a..013ea542be 100644
> --- a/target/microblaze/meson.build
> +++ b/target/microblaze/meson.build
> @@ -5,7 +5,6 @@ microblaze_ss.add(gen)
>  microblaze_ss.add(files(
>    'cpu.c',
>    'gdbstub.c',
> -  'helper.c',
>    'op_helper.c',
>    'translate.c',
>  ))
> @@ -14,6 +13,7 @@ microblaze_system_ss = ss.source_set()
>  microblaze_system_ss.add(files(
>    'mmu.c',
>    'machine.c',
> +  'sys_helper.c',
>  ))
>  
>  target_arch += {'microblaze': microblaze_ss}
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-9.1 7/8] target/microblaze: Move MMU helpers to sys_helper.c

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> MMU helpers are only used during system emulation,
> move them to sys_helper.c.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/op_helper.c  | 48 ----------------------------------
>  target/microblaze/sys_helper.c | 47 +++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 48 deletions(-)
> 
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index f6378030b7..45dbed4aaa 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -381,51 +381,3 @@ void helper_stackprot(CPUMBState *env, target_ulong addr)
>          cpu_loop_exit_restore(cs, GETPC());
>      }
>  }
> -
> -#if !defined(CONFIG_USER_ONLY)
> -/* Writes/reads to the MMU's special regs end up here.  */
> -uint32_t helper_mmu_read(CPUMBState *env, uint32_t ext, uint32_t rn)
> -{
> -    return mmu_read(env, ext, rn);
> -}
> -
> -void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
> -{
> -    mmu_write(env, ext, rn, v);
> -}
> -
> -void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> -                               unsigned size, MMUAccessType access_type,
> -                               int mmu_idx, MemTxAttrs attrs,
> -                               MemTxResult response, uintptr_t retaddr)
> -{
> -    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> -    CPUMBState *env = &cpu->env;
> -
> -    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> -                  " physaddr 0x" HWADDR_FMT_plx " size %d access type %s\n",
> -                  addr, physaddr, size,
> -                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> -                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
> -
> -    if (!(env->msr & MSR_EE)) {
> -        return;
> -    }
> -
> -    if (access_type == MMU_INST_FETCH) {
> -        if (!cpu->cfg.iopb_bus_exception) {
> -            return;
> -        }
> -        env->esr = ESR_EC_INSN_BUS;
> -    } else {
> -        if (!cpu->cfg.dopb_bus_exception) {
> -            return;
> -        }
> -        env->esr = ESR_EC_DATA_BUS;
> -    }
> -
> -    env->ear = addr;
> -    cs->exception_index = EXCP_HW_EXCP;
> -    cpu_loop_exit_restore(cs, retaddr);
> -}
> -#endif
> diff --git a/target/microblaze/sys_helper.c b/target/microblaze/sys_helper.c
> index 5180500354..7531f95ca7 100644
> --- a/target/microblaze/sys_helper.c
> +++ b/target/microblaze/sys_helper.c
> @@ -21,6 +21,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "exec/exec-all.h"
> +#include "exec/helper-proto.h"
>  #include "qemu/host-utils.h"
>  #include "exec/log.h"
>  
> @@ -292,3 +293,49 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      cs->exception_index = EXCP_HW_EXCP;
>      cpu_loop_exit(cs);
>  }
> +
> +/* Writes/reads to the MMU's special regs end up here.  */
> +uint32_t helper_mmu_read(CPUMBState *env, uint32_t ext, uint32_t rn)
> +{
> +    return mmu_read(env, ext, rn);
> +}
> +
> +void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
> +{
> +    mmu_write(env, ext, rn, v);
> +}
> +
> +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> +                               unsigned size, MMUAccessType access_type,
> +                               int mmu_idx, MemTxAttrs attrs,
> +                               MemTxResult response, uintptr_t retaddr)
> +{
> +    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> +    CPUMBState *env = &cpu->env;
> +
> +    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> +                  " physaddr 0x" HWADDR_FMT_plx " size %d access type %s\n",
> +                  addr, physaddr, size,
> +                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> +                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
> +
> +    if (!(env->msr & MSR_EE)) {
> +        return;
> +    }
> +
> +    if (access_type == MMU_INST_FETCH) {
> +        if (!cpu->cfg.iopb_bus_exception) {
> +            return;
> +        }
> +        env->esr = ESR_EC_INSN_BUS;
> +    } else {
> +        if (!cpu->cfg.dopb_bus_exception) {
> +            return;
> +        }
> +        env->esr = ESR_EC_DATA_BUS;
> +    }
> +
> +    env->ear = addr;
> +    cs->exception_index = EXCP_HW_EXCP;
> +    cpu_loop_exit_restore(cs, retaddr);
> +}
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [RFC PATCH-for-9.1 8/8] target/microblaze: Widen $ear to 64-bit

[Anton Johansson via]

On 19/03/24, Philippe Mathieu-Daudé wrote:
> The Exception Address Register is 64-bit wide.
> User emulation only access the 32 lower bits.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/cpu.h       | 2 +-
>  linux-user/elfload.c          | 2 +-
>  target/microblaze/gdbstub.c   | 2 +-
>  target/microblaze/translate.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index c3e2aba0ec..a9f93b37b7 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -251,7 +251,7 @@ struct CPUArchState {
>      uint32_t pc;
>      uint32_t msr;    /* All bits of MSR except MSR[C] and MSR[CC] */
>      uint32_t msr_c;  /* MSR[C], in low bit; other bits must be 0 */
> -    target_ulong ear;
> +    uint64_t ear;
>      uint32_t esr;
>      uint32_t fsr;
>      uint32_t btr;
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 60cf55b36c..4612aef95a 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1498,7 +1498,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
>      (*regs)[pos++] = tswapreg(env->pc);
>      (*regs)[pos++] = tswapreg(mb_cpu_read_msr(env));
>      (*regs)[pos++] = 0;
> -    (*regs)[pos++] = tswapreg(env->ear);
> +    (*regs)[pos++] = tswapreg((uint32_t)env->ear);
As far as I can tell env->ear is never written to from TCG so we 
shouldn't have any problems w. endian mismatch between guest/host,
right?

Anyway,

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH-for-9.1 5/8] target/microblaze: Restrict 64-bit 'res_addr' to system emulation

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:52AM +0100, Philippe Mathieu-Daudé wrote:
> 'res_addr' is only used in system emulation, where we have
> TARGET_LONG_BITS = 64, so we can directly use the native
> uint64_t type instead of target_ulong.


Hi Philippe,

This breaks linux-user, lwx and swx are valid user-mode insns.

Best regards,
Edgar


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/cpu.h       | 10 +++++-----
>  target/microblaze/cpu.c       |  2 ++
>  target/microblaze/machine.c   |  2 +-
>  target/microblaze/translate.c |  9 +++++++--
>  4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index c0c7574dbd..c3e2aba0ec 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -260,11 +260,6 @@ struct CPUArchState {
>      /* Stack protectors. Yes, it's a hw feature.  */
>      uint32_t slr, shr;
>  
> -    /* lwx/swx reserved address */
> -#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no reservation */
> -    target_ulong res_addr;
> -    uint32_t res_val;
> -
>      /* Internal flags.  */
>  #define IMM_FLAG        (1 << 0)
>  #define BIMM_FLAG       (1 << 1)
> @@ -286,6 +281,11 @@ struct CPUArchState {
>      uint32_t iflags;
>  
>  #if !defined(CONFIG_USER_ONLY)
> +    /* lwx/swx reserved address */
> +#define RES_ADDR_NONE 0xffffffff /* Use 0xffffffff to indicate no reservation */
> +    uint64_t res_addr;
> +    uint32_t res_val;
> +
>      /* Unified MMU.  */
>      MicroBlazeMMU mmu;
>  #endif
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 96c2b71f7f..9e393cf217 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -193,7 +193,9 @@ static void mb_cpu_reset_hold(Object *obj)
>      }
>  
>      memset(env, 0, offsetof(CPUMBState, end_reset_fields));
> +#ifndef CONFIG_USER_ONLY
>      env->res_addr = RES_ADDR_NONE;
> +#endif
>  
>      /* Disable stack protector.  */
>      env->shr = ~0;
> diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
> index 51705e4f5c..4daf8a2471 100644
> --- a/target/microblaze/machine.c
> +++ b/target/microblaze/machine.c
> @@ -78,7 +78,7 @@ static const VMStateField vmstate_env_fields[] = {
>      VMSTATE_UINT32(iflags, CPUMBState),
>  
>      VMSTATE_UINT32(res_val, CPUMBState),
> -    VMSTATE_UINTTL(res_addr, CPUMBState),
> +    VMSTATE_UINT64(res_addr, CPUMBState),
>  
>      VMSTATE_STRUCT(mmu, CPUMBState, 0, vmstate_mmu, MicroBlazeMMU),
>  
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index d6a42381bb..493850c544 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1872,7 +1872,9 @@ void mb_tcg_init(void)
>          SP(iflags),
>          SP(bvalue),
>          SP(btarget),
> +#if !defined(CONFIG_USER_ONLY)
>          SP(res_val),
> +#endif
>      };
>  
>  #undef R
> @@ -1883,6 +1885,9 @@ void mb_tcg_init(void)
>            tcg_global_mem_new_i32(tcg_env, i32s[i].ofs, i32s[i].name);
>      }
>  
> -    cpu_res_addr =
> -        tcg_global_mem_new(tcg_env, offsetof(CPUMBState, res_addr), "res_addr");
> +#if !defined(CONFIG_USER_ONLY)
> +    cpu_res_addr = tcg_global_mem_new_i64(tcg_env,
> +                                          offsetof(CPUMBState, res_addr),
> +                                          "res_addr");
> +#endif
>  }
> -- 
> 2.41.0
> 

Re: [PATCH-for-9.1 1/8] target/microblaze: Use correct string format in do_unaligned_access()

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:48AM +0100, Philippe Mathieu-Daudé wrote:
> 'addr' is of type 'vaddr'; no need to cast, use the
> corresponding format string.
> 
> Fixes: ab0c8d0f5b ("target/microblaze: Use cc->do_unaligned_access")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  target/microblaze/helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index d25c9eb4d3..0a12c4ea94 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -279,8 +279,8 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      iflags = cpu->env.iflags;
>  
>      qemu_log_mask(CPU_LOG_INT,
> -                  "Unaligned access addr=" TARGET_FMT_lx " pc=%x iflags=%x\n",
> -                  (target_ulong)addr, cpu->env.pc, iflags);
> +                  "Unaligned access addr=0x%"VADDR_PRIx" pc=0x%x iflags=0x%x\n",
> +                  addr, cpu->env.pc, iflags);
>  
>      esr = ESR_EC_UNALIGNED_DATA;
>      if (likely(iflags & ESR_ESS_FLAG)) {
> -- 
> 2.41.0
> 

Re: [PATCH-for-9.1 2/8] target/microblaze: Use hwaddr/vaddr in cpu_get_phys_page_attrs_debug()

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:49AM +0100, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  target/microblaze/helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index 0a12c4ea94..3f410fc7b5 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -228,7 +228,8 @@ hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
>                                          MemTxAttrs *attrs)
>  {
>      MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> -    target_ulong vaddr, paddr = 0;
> +    vaddr vaddr;
> +    hwaddr paddr = 0;
>      MicroBlazeMMULookup lu;
>      int mmu_idx = cpu_mmu_index(cs, false);
>      unsigned int hit;
> -- 
> 2.41.0
> 

Re: [PATCH-for-9.1 5/8] target/microblaze: Restrict 64-bit 'res_addr' to system emulation

[Philippe Mathieu-Daudé]

On 19/3/24 17:27, Edgar E. Iglesias wrote:
> On Tue, Mar 19, 2024 at 07:28:52AM +0100, Philippe Mathieu-Daudé wrote:
>> 'res_addr' is only used in system emulation, where we have
>> TARGET_LONG_BITS = 64, so we can directly use the native
>> uint64_t type instead of target_ulong.
> 
> 
> Hi Philippe,
> 
> This breaks linux-user, lwx and swx are valid user-mode insns.

Now I see I missed cpu_res_addr. Thanks for the quick review!

> 
> Best regards,
> Edgar
> 
> 
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/microblaze/cpu.h       | 10 +++++-----
>>   target/microblaze/cpu.c       |  2 ++
>>   target/microblaze/machine.c   |  2 +-
>>   target/microblaze/translate.c |  9 +++++++--
>>   4 files changed, 15 insertions(+), 8 deletions(-)

Re: [PATCH-for-9.1 3/8] target/microblaze: Widen vaddr in mmu_translate()

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:50AM +0100, Philippe Mathieu-Daudé wrote:
> Use 'vaddr' type for virtual addresses.


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/mmu.h | 2 +-
>  target/microblaze/mmu.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/mmu.h b/target/microblaze/mmu.h
> index 1068bd2d52..2aca39c923 100644
> --- a/target/microblaze/mmu.h
> +++ b/target/microblaze/mmu.h
> @@ -86,7 +86,7 @@ typedef struct {
>  } MicroBlazeMMULookup;
>  
>  unsigned int mmu_translate(MicroBlazeCPU *cpu, MicroBlazeMMULookup *lu,
> -                           target_ulong vaddr, MMUAccessType rw, int mmu_idx);
> +                           vaddr vaddr, MMUAccessType rw, int mmu_idx);
>  uint32_t mmu_read(CPUMBState *env, bool ea, uint32_t rn);
>  void mmu_write(CPUMBState *env, bool ea, uint32_t rn, uint32_t v);
>  void mmu_init(MicroBlazeMMU *mmu);
> diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
> index 234006634e..eb7c683020 100644
> --- a/target/microblaze/mmu.c
> +++ b/target/microblaze/mmu.c
> @@ -75,7 +75,7 @@ static void mmu_change_pid(CPUMBState *env, unsigned int newpid)
>  
>  /* rw - 0 = read, 1 = write, 2 = fetch.  */
>  unsigned int mmu_translate(MicroBlazeCPU *cpu, MicroBlazeMMULookup *lu,
> -                           target_ulong vaddr, MMUAccessType rw, int mmu_idx)
> +                           vaddr vaddr, MMUAccessType rw, int mmu_idx)
>  {
>      MicroBlazeMMU *mmu = &cpu->env.mmu;
>      unsigned int i, hit = 0;
> -- 
> 2.41.0
> 

Re: [PATCH-for-9.1 4/8] target/microblaze: Use 32-bit destination in gen_goto_tb()

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:51AM +0100, Philippe Mathieu-Daudé wrote:
> cpu_pc and jmp_dest are 32-bit.
>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>

 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 4e52ef32db..d6a42381bb 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -121,7 +121,7 @@ static void gen_raise_hw_excp(DisasContext *dc, uint32_t esr_ec)
>      gen_raise_exception_sync(dc, EXCP_HW_EXCP);
>  }
>  
> -static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
> +static void gen_goto_tb(DisasContext *dc, int n, uint32_t dest)
>  {
>      if (translator_use_goto_tb(&dc->base, dest)) {
>          tcg_gen_goto_tb(n);
> -- 
> 2.41.0
> 

Re: [PATCH-for-9.1 6/8] target/microblaze: Rename helper.c -> sys_helper.c

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:53AM +0100, Philippe Mathieu-Daudé wrote:
> helper.c only contains system emulation helpers,
> rename it as sys_helper.c.
> Adapt meson and remove pointless #ifdef'ry.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/{helper.c => sys_helper.c} | 5 +----
>  target/microblaze/meson.build                | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
>  rename target/microblaze/{helper.c => sys_helper.c} (99%)
> 
> diff --git a/target/microblaze/helper.c b/target/microblaze/sys_helper.c
> similarity index 99%
> rename from target/microblaze/helper.c
> rename to target/microblaze/sys_helper.c
> index 3f410fc7b5..5180500354 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/sys_helper.c
> @@ -1,5 +1,5 @@
>  /*
> - *  MicroBlaze helper routines.
> + *  MicroBlaze system helper routines.
>   *
>   *  Copyright (c) 2009 Edgar E. Iglesias <edgar.iglesias@gmail.com>
>   *  Copyright (c) 2009-2012 PetaLogix Qld Pty Ltd.
> @@ -24,7 +24,6 @@
>  #include "qemu/host-utils.h"
>  #include "exec/log.h"
>  
> -#ifndef CONFIG_USER_ONLY
>  static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
>                                      MMUAccessType access_type)
>  {
> @@ -266,8 +265,6 @@ bool mb_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>      return false;
>  }
>  
> -#endif /* !CONFIG_USER_ONLY */
> -
>  void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>                                  MMUAccessType access_type,
>                                  int mmu_idx, uintptr_t retaddr)
> diff --git a/target/microblaze/meson.build b/target/microblaze/meson.build
> index 3ed4fbb67a..013ea542be 100644
> --- a/target/microblaze/meson.build
> +++ b/target/microblaze/meson.build
> @@ -5,7 +5,6 @@ microblaze_ss.add(gen)
>  microblaze_ss.add(files(
>    'cpu.c',
>    'gdbstub.c',
> -  'helper.c',
>    'op_helper.c',
>    'translate.c',
>  ))
> @@ -14,6 +13,7 @@ microblaze_system_ss = ss.source_set()
>  microblaze_system_ss.add(files(
>    'mmu.c',
>    'machine.c',
> +  'sys_helper.c',
>  ))
>  
>  target_arch += {'microblaze': microblaze_ss}
> -- 
> 2.41.0
> 

Re: [PATCH-for-9.1 7/8] target/microblaze: Move MMU helpers to sys_helper.c

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:54AM +0100, Philippe Mathieu-Daudé wrote:
> MMU helpers are only used during system emulation,
> move them to sys_helper.c.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/op_helper.c  | 48 ----------------------------------
>  target/microblaze/sys_helper.c | 47 +++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 48 deletions(-)
> 
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index f6378030b7..45dbed4aaa 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -381,51 +381,3 @@ void helper_stackprot(CPUMBState *env, target_ulong addr)
>          cpu_loop_exit_restore(cs, GETPC());
>      }
>  }
> -
> -#if !defined(CONFIG_USER_ONLY)
> -/* Writes/reads to the MMU's special regs end up here.  */
> -uint32_t helper_mmu_read(CPUMBState *env, uint32_t ext, uint32_t rn)
> -{
> -    return mmu_read(env, ext, rn);
> -}
> -
> -void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
> -{
> -    mmu_write(env, ext, rn, v);
> -}
> -
> -void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> -                               unsigned size, MMUAccessType access_type,
> -                               int mmu_idx, MemTxAttrs attrs,
> -                               MemTxResult response, uintptr_t retaddr)
> -{
> -    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> -    CPUMBState *env = &cpu->env;
> -
> -    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> -                  " physaddr 0x" HWADDR_FMT_plx " size %d access type %s\n",
> -                  addr, physaddr, size,
> -                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> -                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
> -
> -    if (!(env->msr & MSR_EE)) {
> -        return;
> -    }
> -
> -    if (access_type == MMU_INST_FETCH) {
> -        if (!cpu->cfg.iopb_bus_exception) {
> -            return;
> -        }
> -        env->esr = ESR_EC_INSN_BUS;
> -    } else {
> -        if (!cpu->cfg.dopb_bus_exception) {
> -            return;
> -        }
> -        env->esr = ESR_EC_DATA_BUS;
> -    }
> -
> -    env->ear = addr;
> -    cs->exception_index = EXCP_HW_EXCP;
> -    cpu_loop_exit_restore(cs, retaddr);
> -}
> -#endif
> diff --git a/target/microblaze/sys_helper.c b/target/microblaze/sys_helper.c
> index 5180500354..7531f95ca7 100644
> --- a/target/microblaze/sys_helper.c
> +++ b/target/microblaze/sys_helper.c
> @@ -21,6 +21,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "exec/exec-all.h"
> +#include "exec/helper-proto.h"
>  #include "qemu/host-utils.h"
>  #include "exec/log.h"
>  
> @@ -292,3 +293,49 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      cs->exception_index = EXCP_HW_EXCP;
>      cpu_loop_exit(cs);
>  }
> +
> +/* Writes/reads to the MMU's special regs end up here.  */
> +uint32_t helper_mmu_read(CPUMBState *env, uint32_t ext, uint32_t rn)
> +{
> +    return mmu_read(env, ext, rn);
> +}
> +
> +void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
> +{
> +    mmu_write(env, ext, rn, v);
> +}
> +
> +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> +                               unsigned size, MMUAccessType access_type,
> +                               int mmu_idx, MemTxAttrs attrs,
> +                               MemTxResult response, uintptr_t retaddr)
> +{
> +    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> +    CPUMBState *env = &cpu->env;
> +
> +    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> +                  " physaddr 0x" HWADDR_FMT_plx " size %d access type %s\n",
> +                  addr, physaddr, size,
> +                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> +                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
> +
> +    if (!(env->msr & MSR_EE)) {
> +        return;
> +    }
> +
> +    if (access_type == MMU_INST_FETCH) {
> +        if (!cpu->cfg.iopb_bus_exception) {
> +            return;
> +        }
> +        env->esr = ESR_EC_INSN_BUS;
> +    } else {
> +        if (!cpu->cfg.dopb_bus_exception) {
> +            return;
> +        }
> +        env->esr = ESR_EC_DATA_BUS;
> +    }
> +
> +    env->ear = addr;
> +    cs->exception_index = EXCP_HW_EXCP;
> +    cpu_loop_exit_restore(cs, retaddr);
> +}
> -- 
> 2.41.0
> 

Re: [RFC PATCH-for-9.1 8/8] target/microblaze: Widen $ear to 64-bit

[Edgar E. Iglesias]

On Tue, Mar 19, 2024 at 07:28:55AM +0100, Philippe Mathieu-Daudé wrote:
> The Exception Address Register is 64-bit wide.
> User emulation only access the 32 lower bits.


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/microblaze/cpu.h       | 2 +-
>  linux-user/elfload.c          | 2 +-
>  target/microblaze/gdbstub.c   | 2 +-
>  target/microblaze/translate.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index c3e2aba0ec..a9f93b37b7 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -251,7 +251,7 @@ struct CPUArchState {
>      uint32_t pc;
>      uint32_t msr;    /* All bits of MSR except MSR[C] and MSR[CC] */
>      uint32_t msr_c;  /* MSR[C], in low bit; other bits must be 0 */
> -    target_ulong ear;
> +    uint64_t ear;
>      uint32_t esr;
>      uint32_t fsr;
>      uint32_t btr;
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 60cf55b36c..4612aef95a 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1498,7 +1498,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
>      (*regs)[pos++] = tswapreg(env->pc);
>      (*regs)[pos++] = tswapreg(mb_cpu_read_msr(env));
>      (*regs)[pos++] = 0;
> -    (*regs)[pos++] = tswapreg(env->ear);
> +    (*regs)[pos++] = tswapreg((uint32_t)env->ear);
>      (*regs)[pos++] = 0;
>      (*regs)[pos++] = tswapreg(env->esr);
>  }
> diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
> index 09d74e164d..147d20c3e4 100644
> --- a/target/microblaze/gdbstub.c
> +++ b/target/microblaze/gdbstub.c
> @@ -63,7 +63,7 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
>          val = mb_cpu_read_msr(env);
>          break;
>      case GDB_EAR:
> -        val = env->ear;
> +        val = (uint32_t)env->ear;
>          break;
>      case GDB_ESR:
>          val = env->esr;
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 493850c544..19b180501f 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1835,7 +1835,7 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
>      }
>  
>      qemu_fprintf(f, "\nesr=0x%04x fsr=0x%02x btr=0x%08x edr=0x%x\n"
> -                 "ear=0x" TARGET_FMT_lx " slr=0x%x shr=0x%x\n",
> +                 "ear=0x%" PRIx64 " slr=0x%x shr=0x%x\n",
>                   env->esr, env->fsr, env->btr, env->edr,
>                   env->ear, env->slr, env->shr);
>  
> -- 
> 2.41.0
>