[PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Richard Henderson]

Use out-of-line helpers to implement extended address memory ops.
With this, we can reduce TARGET_LONG_BITS to the more natural 32
for this 32-bit cpu.


r~


Richard Henderson (9):
  target/microblaze: Split out mb_unaligned_access_internal
  target/microblaze: Split out mb_transaction_failed_internal
  target/microblaze: Implement extended address load/store out of line
  target/microblaze: Use uint64_t for CPUMBState.ear
  target/microblaze: Use TCGv_i64 for compute_ldst_addr_ea
  target/microblaze: Fix printf format in mmu_translate
  target/microblaze: Use TARGET_LONG_BITS == 32 for system mode
  target/microblaze: Drop DisasContext.r0
  target/microblaze: Simplify compute_ldst_addr_type{a,b}

 target/microblaze/cpu.h                  |   5 +-
 target/microblaze/helper.h               |  16 ++-
 target/microblaze/helper.c               |  25 ++--
 target/microblaze/mmu.c                  |   3 +-
 target/microblaze/op_helper.c            | 157 +++++++++++++++++++----
 target/microblaze/translate.c            | 118 +++++++++--------
 configs/targets/microblaze-softmmu.mak   |   4 +-
 configs/targets/microblazeel-softmmu.mak |   4 +-
 8 files changed, 228 insertions(+), 104 deletions(-)

-- 
2.43.0

[PATCH 1/9] target/microblaze: Split out mb_unaligned_access_internal

[Richard Henderson]

Use an explicit 64-bit type for the address to store in EAR.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/cpu.h    |  3 +++
 target/microblaze/helper.c | 25 ++++++++++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index f6879eee35..45f7f49809 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -372,6 +372,9 @@ bool mb_cpu_exec_interrupt(CPUState *cs, int int_req);
 hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
                                         MemTxAttrs *attrs);
 #endif /* !CONFIG_USER_ONLY */
+G_NORETURN void mb_unaligned_access_internal(CPUState *cs, uint64_t addr,
+                                             MMUAccessType access_type,
+                                             uintptr_t retaddr);
 G_NORETURN void mb_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
                                            MMUAccessType access_type,
                                            int mmu_idx, uintptr_t retaddr);
diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index 5d3259ce31..8b096e3e58 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -268,20 +268,20 @@ bool mb_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 
 #endif /* !CONFIG_USER_ONLY */
 
-void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
-                                MMUAccessType access_type,
-                                int mmu_idx, uintptr_t retaddr)
+G_NORETURN
+void mb_unaligned_access_internal(CPUState *cs, uint64_t addr,
+                                  MMUAccessType access_type, uintptr_t retaddr)
 {
-    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+    CPUMBState *env = cpu_env(cs);
     uint32_t esr, iflags;
 
     /* Recover the pc and iflags from the corresponding insn_start.  */
     cpu_restore_state(cs, retaddr);
-    iflags = cpu->env.iflags;
+    iflags = 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%" PRIx64 " pc=%x iflags=%x\n",
+                  addr, env->pc, iflags);
 
     esr = ESR_EC_UNALIGNED_DATA;
     if (likely(iflags & ESR_ESS_FLAG)) {
@@ -290,8 +290,15 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
         qemu_log_mask(LOG_UNIMP, "Unaligned access without ESR_ESS_FLAG\n");
     }
 
-    cpu->env.ear = addr;
-    cpu->env.esr = esr;
+    env->ear = addr;
+    env->esr = esr;
     cs->exception_index = EXCP_HW_EXCP;
     cpu_loop_exit(cs);
 }
+
+void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+                                MMUAccessType access_type,
+                                int mmu_idx, uintptr_t retaddr)
+{
+    mb_unaligned_access_internal(cs, addr, access_type, retaddr);
+}
-- 
2.43.0

[PATCH 2/9] target/microblaze: Split out mb_transaction_failed_internal

[Richard Henderson]

Use an explicit 64-bit type for the address to store in EAR.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/op_helper.c | 70 +++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 28 deletions(-)

diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index f6378030b7..6019c5b2eb 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -394,38 +394,52 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
     mmu_write(env, ext, rn, v);
 }
 
+static void mb_transaction_failed_internal(CPUState *cs, hwaddr physaddr,
+                                           uint64_t addr, unsigned size,
+                                           MMUAccessType access_type,
+                                           uintptr_t retaddr)
+{
+    CPUMBState *env = cpu_env(cs);
+    MicroBlazeCPU *cpu = env_archcpu(env);
+    const char *access_name = "INVALID";
+    bool take = env->msr & MSR_EE;
+    uint32_t esr = ESR_EC_DATA_BUS;
+
+    switch (access_type) {
+    case MMU_INST_FETCH:
+        access_name = "INST_FETCH";
+        esr = ESR_EC_INSN_BUS;
+        take &= cpu->cfg.iopb_bus_exception;
+        break;
+    case MMU_DATA_LOAD:
+        access_name = "DATA_LOAD";
+        take &= cpu->cfg.dopb_bus_exception;
+        break;
+    case MMU_DATA_STORE:
+        access_name = "DATA_STORE";
+        take &= cpu->cfg.dopb_bus_exception;
+        break;
+    }
+
+    qemu_log_mask(CPU_LOG_INT, "Transaction failed: addr 0x%" PRIx64
+                  "physaddr 0x" HWADDR_FMT_plx " size %d access-type %s (%s)\n",
+                  addr, physaddr, size, access_name,
+                  take ? "TAKEN" : "DROPPED");
+
+    if (take) {
+        env->esr = esr;
+        env->ear = addr;
+        cs->exception_index = EXCP_HW_EXCP;
+        cpu_loop_exit_restore(cs, retaddr);
+    }
+}
+
 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);
+    mb_transaction_failed_internal(cs, physaddr, addr, size,
+                                   access_type, retaddr);
 }
 #endif
-- 
2.43.0

Re: [PATCH 2/9] target/microblaze: Split out mb_transaction_failed_internal

[Philippe Mathieu-Daudé]

On 12/2/25 23:01, Richard Henderson wrote:
> Use an explicit 64-bit type for the address to store in EAR.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/microblaze/op_helper.c | 70 +++++++++++++++++++++--------------
>   1 file changed, 42 insertions(+), 28 deletions(-)


> +static void mb_transaction_failed_internal(CPUState *cs, hwaddr physaddr,
> +                                           uint64_t addr, unsigned size,
> +                                           MMUAccessType access_type,
> +                                           uintptr_t retaddr)
> +{
> +    CPUMBState *env = cpu_env(cs);
> +    MicroBlazeCPU *cpu = env_archcpu(env);
> +    const char *access_name = "INVALID";
> +    bool take = env->msr & MSR_EE;
> +    uint32_t esr = ESR_EC_DATA_BUS;
> +
> +    switch (access_type) {
> +    case MMU_INST_FETCH:
> +        access_name = "INST_FETCH";
> +        esr = ESR_EC_INSN_BUS;
> +        take &= cpu->cfg.iopb_bus_exception;
> +        break;
> +    case MMU_DATA_LOAD:
> +        access_name = "DATA_LOAD";
> +        take &= cpu->cfg.dopb_bus_exception;
> +        break;
> +    case MMU_DATA_STORE:
> +        access_name = "DATA_STORE";
> +        take &= cpu->cfg.dopb_bus_exception;
> +        break;
> +    }
> +
> +    qemu_log_mask(CPU_LOG_INT, "Transaction failed: addr 0x%" PRIx64
> +                  "physaddr 0x" HWADDR_FMT_plx " size %d access-type %s (%s)\n",
> +                  addr, physaddr, size, access_name,
> +                  take ? "TAKEN" : "DROPPED");

Helpful.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +
> +    if (take) {
> +        env->esr = esr;
> +        env->ear = addr;
> +        cs->exception_index = EXCP_HW_EXCP;
> +        cpu_loop_exit_restore(cs, retaddr);
> +    }
> +}

[PATCH 3/9] target/microblaze: Implement extended address load/store out of line

[Richard Henderson]

Use helpers and address_space_ld/st instead of inline
loads and stores.  This allows us to perform operations
on physical addresses wider than virtual addresses.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/helper.h    | 16 ++++--
 target/microblaze/op_helper.c | 99 +++++++++++++++++++++++++++++++++++
 target/microblaze/translate.c | 42 ++++++++++-----
 3 files changed, 140 insertions(+), 17 deletions(-)

diff --git a/target/microblaze/helper.h b/target/microblaze/helper.h
index f740835fcb..f3ef5a3150 100644
--- a/target/microblaze/helper.h
+++ b/target/microblaze/helper.h
@@ -20,12 +20,18 @@ DEF_HELPER_FLAGS_3(fcmp_ne, TCG_CALL_NO_WG, i32, env, i32, i32)
 DEF_HELPER_FLAGS_3(fcmp_ge, TCG_CALL_NO_WG, i32, env, i32, i32)
 
 DEF_HELPER_FLAGS_2(pcmpbf, TCG_CALL_NO_RWG_SE, i32, i32, i32)
-#if !defined(CONFIG_USER_ONLY)
-DEF_HELPER_FLAGS_3(mmu_read, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_4(mmu_write, TCG_CALL_NO_RWG, void, env, i32, i32, i32)
-#endif
-
 DEF_HELPER_FLAGS_2(stackprot, TCG_CALL_NO_WG, void, env, tl)
 
 DEF_HELPER_FLAGS_2(get, TCG_CALL_NO_RWG, i32, i32, i32)
 DEF_HELPER_FLAGS_3(put, TCG_CALL_NO_RWG, void, i32, i32, i32)
+
+#if !defined(CONFIG_USER_ONLY)
+DEF_HELPER_FLAGS_3(mmu_read, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_4(mmu_write, TCG_CALL_NO_RWG, void, env, i32, i32, i32)
+DEF_HELPER_FLAGS_2(lbuea, TCG_CALL_NO_WG, i32, env, i64)
+DEF_HELPER_FLAGS_2(lhuea, TCG_CALL_NO_WG, i32, env, i64)
+DEF_HELPER_FLAGS_2(lwea, TCG_CALL_NO_WG, i32, env, i64)
+DEF_HELPER_FLAGS_3(sbea, TCG_CALL_NO_WG, void, env, i32, i64)
+DEF_HELPER_FLAGS_3(shea, TCG_CALL_NO_WG, void, env, i32, i64)
+DEF_HELPER_FLAGS_3(swea, TCG_CALL_NO_WG, void, env, i32, i64)
+#endif
diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 6019c5b2eb..722810cb9c 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -25,6 +25,7 @@
 #include "qemu/host-utils.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
+#include "exec/memory.h"
 #include "fpu/softfloat.h"
 
 void helper_put(uint32_t id, uint32_t ctrl, uint32_t data)
@@ -442,4 +443,102 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
     mb_transaction_failed_internal(cs, physaddr, addr, size,
                                    access_type, retaddr);
 }
+
+uint32_t HELPER(lbuea)(CPUMBState *env, uint64_t ea)
+{
+    CPUState *cs = env_cpu(env);
+    uintptr_t ra = GETPC();
+    MemTxResult txres;
+    uint8_t ret;
+
+    ret = address_space_ldub(cs->as, ea, MEMTXATTRS_UNSPECIFIED, &txres);
+    if (unlikely(txres != MEMTX_OK)) {
+        mb_transaction_failed_internal(cs, ea, ea, 1, MMU_DATA_LOAD, ra);
+    }
+    return ret;
+}
+
+uint32_t HELPER(lhuea)(CPUMBState *env, uint64_t ea)
+{
+    CPUState *cs = env_cpu(env);
+    uintptr_t ra = GETPC();
+    MemTxResult txres;
+    uint16_t ret;
+
+    if (unlikely(ea & 1)
+        && (env->msr & MSR_EE)
+        && env_archcpu(env)->cfg.unaligned_exceptions) {
+        mb_unaligned_access_internal(cs, ea, MMU_DATA_LOAD, ra);
+    }
+    ret = address_space_lduw(cs->as, ea, MEMTXATTRS_UNSPECIFIED, &txres);
+    if (unlikely(txres != MEMTX_OK)) {
+        mb_transaction_failed_internal(cs, ea, ea, 2, MMU_DATA_LOAD, ra);
+    }
+    return ret;
+}
+
+uint32_t HELPER(lwea)(CPUMBState *env, uint64_t ea)
+{
+    CPUState *cs = env_cpu(env);
+    uintptr_t ra = GETPC();
+    MemTxResult txres;
+    uint32_t ret;
+
+    if (unlikely(ea & 3)
+        && (env->msr & MSR_EE)
+        && env_archcpu(env)->cfg.unaligned_exceptions) {
+        mb_unaligned_access_internal(cs, ea, MMU_DATA_LOAD, ra);
+    }
+    ret = address_space_ldl(cs->as, ea, MEMTXATTRS_UNSPECIFIED, &txres);
+    if (unlikely(txres != MEMTX_OK)) {
+        mb_transaction_failed_internal(cs, ea, ea, 4, MMU_DATA_LOAD, ra);
+    }
+    return ret;
+}
+
+void HELPER(sbea)(CPUMBState *env, uint32_t data, uint64_t ea)
+{
+    CPUState *cs = env_cpu(env);
+    uintptr_t ra = GETPC();
+    MemTxResult txres;
+
+    address_space_stb(cs->as, ea, data, MEMTXATTRS_UNSPECIFIED, &txres);
+    if (unlikely(txres != MEMTX_OK)) {
+        mb_transaction_failed_internal(cs, ea, ea, 1, MMU_DATA_STORE, ra);
+    }
+}
+
+void HELPER(shea)(CPUMBState *env, uint32_t data, uint64_t ea)
+{
+    CPUState *cs = env_cpu(env);
+    uintptr_t ra = GETPC();
+    MemTxResult txres;
+
+    if (unlikely(ea & 1)
+        && (env->msr & MSR_EE)
+        && env_archcpu(env)->cfg.unaligned_exceptions) {
+        mb_unaligned_access_internal(cs, ea, MMU_DATA_STORE, ra);
+    }
+    address_space_stw(cs->as, ea, data, MEMTXATTRS_UNSPECIFIED, &txres);
+    if (unlikely(txres != MEMTX_OK)) {
+        mb_transaction_failed_internal(cs, ea, ea, 2, MMU_DATA_STORE, ra);
+    }
+}
+
+void HELPER(swea)(CPUMBState *env, uint32_t data, uint64_t ea)
+{
+    CPUState *cs = env_cpu(env);
+    uintptr_t ra = GETPC();
+    MemTxResult txres;
+
+    if (unlikely(ea & 3)
+        && (env->msr & MSR_EE)
+        && env_archcpu(env)->cfg.unaligned_exceptions) {
+        mb_unaligned_access_internal(cs, ea, MMU_DATA_STORE, ra);
+    }
+    address_space_stl(cs->as, ea, data, MEMTXATTRS_UNSPECIFIED, &txres);
+    if (unlikely(txres != MEMTX_OK)) {
+        mb_transaction_failed_internal(cs, ea, ea, 4, MMU_DATA_STORE, ra);
+    }
+}
 #endif
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 24005f05b2..d5c5e650e0 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -764,10 +764,11 @@ static bool trans_lbuea(DisasContext *dc, arg_typea *arg)
         return true;
     }
 #ifdef CONFIG_USER_ONLY
-    return true;
+    g_assert_not_reached();
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_UB, MMU_NOMMU_IDX, false);
+    gen_helper_lbuea(reg_for_write(dc, arg->rd), tcg_env, addr);
+    return true;
 #endif
 }
 
@@ -795,10 +796,14 @@ static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
         return true;
     }
 #ifdef CONFIG_USER_ONLY
-    return true;
+    g_assert_not_reached();
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUW, MMU_NOMMU_IDX, false);
+    if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
+        record_unaligned_ess(dc, arg->rd, MO_16, false);
+    }
+    gen_helper_lhuea(reg_for_write(dc, arg->rd), tcg_env, addr);
+    return true;
 #endif
 }
 
@@ -826,10 +831,14 @@ static bool trans_lwea(DisasContext *dc, arg_typea *arg)
         return true;
     }
 #ifdef CONFIG_USER_ONLY
-    return true;
+    g_assert_not_reached();
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUL, MMU_NOMMU_IDX, false);
+    if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
+        record_unaligned_ess(dc, arg->rd, MO_32, false);
+    }
+    gen_helper_lwea(reg_for_write(dc, arg->rd), tcg_env, addr);
+    return true;
 #endif
 }
 
@@ -914,10 +923,11 @@ static bool trans_sbea(DisasContext *dc, arg_typea *arg)
         return true;
     }
 #ifdef CONFIG_USER_ONLY
-    return true;
+    g_assert_not_reached();
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_UB, MMU_NOMMU_IDX, false);
+    gen_helper_sbea(tcg_env, reg_for_read(dc, arg->rd), addr);
+    return true;
 #endif
 }
 
@@ -945,10 +955,14 @@ static bool trans_shea(DisasContext *dc, arg_typea *arg)
         return true;
     }
 #ifdef CONFIG_USER_ONLY
-    return true;
+    g_assert_not_reached();
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUW, MMU_NOMMU_IDX, false);
+    if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
+        record_unaligned_ess(dc, arg->rd, MO_16, true);
+    }
+    gen_helper_shea(tcg_env, reg_for_read(dc, arg->rd), addr);
+    return true;
 #endif
 }
 
@@ -976,10 +990,14 @@ static bool trans_swea(DisasContext *dc, arg_typea *arg)
         return true;
     }
 #ifdef CONFIG_USER_ONLY
-    return true;
+    g_assert_not_reached();
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUL, MMU_NOMMU_IDX, false);
+    if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
+        record_unaligned_ess(dc, arg->rd, MO_32, true);
+    }
+    gen_helper_swea(tcg_env, reg_for_read(dc, arg->rd), addr);
+    return true;
 #endif
 }
 
-- 
2.43.0

[PATCH 4/9] target/microblaze: Use uint64_t for CPUMBState.ear

[Richard Henderson]

Use an explicit 64-bit type for EAR.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/cpu.h       | 2 +-
 target/microblaze/translate.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 45f7f49809..01571d4f86 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -248,7 +248,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/target/microblaze/translate.c b/target/microblaze/translate.c
index d5c5e650e0..549013d25e 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1842,7 +1842,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.43.0

Re: [PATCH 4/9] target/microblaze: Use uint64_t for CPUMBState.ear

[Philippe Mathieu-Daudé]

On 12/2/25 23:01, Richard Henderson wrote:
> Use an explicit 64-bit type for EAR.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/microblaze/cpu.h       | 2 +-
>   target/microblaze/translate.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index 45f7f49809..01571d4f86 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -248,7 +248,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/target/microblaze/translate.c b/target/microblaze/translate.c
> index d5c5e650e0..549013d25e 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1842,7 +1842,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);
>   

So IIUC no need to worry about the upper 32-bits as Anton
suggested in my RFC:
https://lore.kernel.org/qemu-devel/rbczkcp7whvovj55htcvongsc45xyhia5sgckqunszldag3iey@4vsbsjak4wr2/

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 4/9] target/microblaze: Use uint64_t for CPUMBState.ear

[Richard Henderson]

On 2/13/25 04:42, Philippe Mathieu-Daudé wrote:
> On 12/2/25 23:01, Richard Henderson wrote:
>> Use an explicit 64-bit type for EAR.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/microblaze/cpu.h       | 2 +-
>>   target/microblaze/translate.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
>> index 45f7f49809..01571d4f86 100644
>> --- a/target/microblaze/cpu.h
>> +++ b/target/microblaze/cpu.h
>> @@ -248,7 +248,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/target/microblaze/translate.c b/target/microblaze/translate.c
>> index d5c5e650e0..549013d25e 100644
>> --- a/target/microblaze/translate.c
>> +++ b/target/microblaze/translate.c
>> @@ -1842,7 +1842,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);
> 
> So IIUC no need to worry about the upper 32-bits as Anton
> suggested in my RFC:
> https://lore.kernel.org/qemu-devel/ 
> rbczkcp7whvovj55htcvongsc45xyhia5sgckqunszldag3iey@4vsbsjak4wr2/

The upper 32 bits can only be written by the 64-bit "extended address" instructions, which 
are supervisor only.  So certainly the upper 32-bits are not relevant to linux-user.

We are not currently, but *should* be writing to ear from linux-user so that ear gets 
populated in the signal context.  We're missing a record_sigsegv hook.


r~

Re: [PATCH 4/9] target/microblaze: Use uint64_t for CPUMBState.ear

[Philippe Mathieu-Daudé]

On 12/2/25 23:01, Richard Henderson wrote:
> Use an explicit 64-bit type for EAR.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/microblaze/cpu.h       | 2 +-
>   target/microblaze/translate.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index 45f7f49809..01571d4f86 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -248,7 +248,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;

Interestingly you already migrate it as 64-bit, see commit 77f63e1dadb
("target/microblaze: Fill in VMStateDescription for cpu").

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 5/9] target/microblaze: Use TCGv_i64 for compute_ldst_addr_ea

[Richard Henderson]

Use an explicit 64-bit type for extended addresses.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/translate.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 549013d25e..a1d81b0166 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -666,23 +666,23 @@ static TCGv compute_ldst_addr_typeb(DisasContext *dc, int ra, int imm)
 }
 
 #ifndef CONFIG_USER_ONLY
-static TCGv compute_ldst_addr_ea(DisasContext *dc, int ra, int rb)
+static TCGv_i64 compute_ldst_addr_ea(DisasContext *dc, int ra, int rb)
 {
     int addr_size = dc->cfg->addr_size;
-    TCGv ret = tcg_temp_new();
+    TCGv_i64 ret = tcg_temp_new_i64();
 
     if (addr_size == 32 || ra == 0) {
         if (rb) {
-            tcg_gen_extu_i32_tl(ret, cpu_R[rb]);
+            tcg_gen_extu_i32_i64(ret, cpu_R[rb]);
         } else {
-            tcg_gen_movi_tl(ret, 0);
+            return tcg_constant_i64(0);
         }
     } else {
         if (rb) {
             tcg_gen_concat_i32_i64(ret, cpu_R[rb], cpu_R[ra]);
         } else {
-            tcg_gen_extu_i32_tl(ret, cpu_R[ra]);
-            tcg_gen_shli_tl(ret, ret, 32);
+            tcg_gen_extu_i32_i64(ret, cpu_R[ra]);
+            tcg_gen_shli_i64(ret, ret, 32);
         }
         if (addr_size < 64) {
             /* Mask off out of range bits.  */
@@ -766,7 +766,7 @@ static bool trans_lbuea(DisasContext *dc, arg_typea *arg)
 #ifdef CONFIG_USER_ONLY
     g_assert_not_reached();
 #else
-    TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
+    TCGv_i64 addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
     gen_helper_lbuea(reg_for_write(dc, arg->rd), tcg_env, addr);
     return true;
 #endif
@@ -798,7 +798,7 @@ static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
 #ifdef CONFIG_USER_ONLY
     g_assert_not_reached();
 #else
-    TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
+    TCGv_i64 addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
     if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
         record_unaligned_ess(dc, arg->rd, MO_16, false);
     }
@@ -833,7 +833,7 @@ static bool trans_lwea(DisasContext *dc, arg_typea *arg)
 #ifdef CONFIG_USER_ONLY
     g_assert_not_reached();
 #else
-    TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
+    TCGv_i64 addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
     if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
         record_unaligned_ess(dc, arg->rd, MO_32, false);
     }
@@ -925,7 +925,7 @@ static bool trans_sbea(DisasContext *dc, arg_typea *arg)
 #ifdef CONFIG_USER_ONLY
     g_assert_not_reached();
 #else
-    TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
+    TCGv_i64 addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
     gen_helper_sbea(tcg_env, reg_for_read(dc, arg->rd), addr);
     return true;
 #endif
@@ -957,7 +957,7 @@ static bool trans_shea(DisasContext *dc, arg_typea *arg)
 #ifdef CONFIG_USER_ONLY
     g_assert_not_reached();
 #else
-    TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
+    TCGv_i64 addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
     if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
         record_unaligned_ess(dc, arg->rd, MO_16, true);
     }
@@ -992,7 +992,7 @@ static bool trans_swea(DisasContext *dc, arg_typea *arg)
 #ifdef CONFIG_USER_ONLY
     g_assert_not_reached();
 #else
-    TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
+    TCGv_i64 addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
     if ((dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
         record_unaligned_ess(dc, arg->rd, MO_32, true);
     }
-- 
2.43.0

Re: [PATCH 5/9] target/microblaze: Use TCGv_i64 for compute_ldst_addr_ea

[Philippe Mathieu-Daudé]

On 12/2/25 23:01, Richard Henderson wrote:
> Use an explicit 64-bit type for extended addresses.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/microblaze/translate.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 6/9] target/microblaze: Fix printf format in mmu_translate

[Richard Henderson]

Use TARGET_FMT_lx to match the target_ulong type of vaddr.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/mmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index 2423ac6172..6152fdafd5 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -170,7 +170,8 @@ unsigned int mmu_translate(MicroBlazeCPU *cpu, MicroBlazeMMULookup *lu,
     }
 done:
     qemu_log_mask(CPU_LOG_MMU,
-                  "MMU vaddr=%" PRIx64 " rw=%d tlb_wr=%d tlb_ex=%d hit=%d\n",
+                  "MMU vaddr=0x" TARGET_FMT_lx
+                  " rw=%d tlb_wr=%d tlb_ex=%d hit=%d\n",
                   vaddr, rw, tlb_wr, tlb_ex, hit);
     return hit;
 }
-- 
2.43.0

[PATCH 7/9] target/microblaze: Use TARGET_LONG_BITS == 32 for system mode

[Richard Henderson]

Now that the extended address instructions are handled separately
from virtual addresses, we can narrow the emulation to 32-bit.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configs/targets/microblaze-softmmu.mak   | 4 +---
 configs/targets/microblazeel-softmmu.mak | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/configs/targets/microblaze-softmmu.mak b/configs/targets/microblaze-softmmu.mak
index 99a33ed44a..47e076af91 100644
--- a/configs/targets/microblaze-softmmu.mak
+++ b/configs/targets/microblaze-softmmu.mak
@@ -4,6 +4,4 @@ TARGET_SUPPORTS_MTTCG=y
 # needed by boot.c
 TARGET_NEED_FDT=y
 TARGET_XML_FILES=gdb-xml/microblaze-core.xml gdb-xml/microblaze-stack-protect.xml
-# System mode can address up to 64 bits via lea/sea instructions.
-# TODO: These bypass the mmu, so we could emulate these differently.
-TARGET_LONG_BITS=64
+TARGET_LONG_BITS=32
diff --git a/configs/targets/microblazeel-softmmu.mak b/configs/targets/microblazeel-softmmu.mak
index 52cdeae1a2..1b0e86c4be 100644
--- a/configs/targets/microblazeel-softmmu.mak
+++ b/configs/targets/microblazeel-softmmu.mak
@@ -3,6 +3,4 @@ TARGET_SUPPORTS_MTTCG=y
 # needed by boot.c
 TARGET_NEED_FDT=y
 TARGET_XML_FILES=gdb-xml/microblaze-core.xml gdb-xml/microblaze-stack-protect.xml
-# System mode can address up to 64 bits via lea/sea instructions.
-# TODO: These bypass the mmu, so we could emulate these differently.
-TARGET_LONG_BITS=64
+TARGET_LONG_BITS=32
-- 
2.43.0

[PATCH 8/9] target/microblaze: Drop DisasContext.r0

[Richard Henderson]

Return a constant 0 from reg_for_read, and a new
temporary from reg_for_write.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/translate.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index a1d81b0166..5750c45dac 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -63,9 +63,6 @@ typedef struct DisasContext {
     DisasContextBase base;
     const MicroBlazeCPUConfig *cfg;
 
-    TCGv_i32 r0;
-    bool r0_set;
-
     /* Decoder.  */
     uint32_t ext_imm;
     unsigned int tb_flags;
@@ -179,14 +176,7 @@ static TCGv_i32 reg_for_read(DisasContext *dc, int reg)
     if (likely(reg != 0)) {
         return cpu_R[reg];
     }
-    if (!dc->r0_set) {
-        if (dc->r0 == NULL) {
-            dc->r0 = tcg_temp_new_i32();
-        }
-        tcg_gen_movi_i32(dc->r0, 0);
-        dc->r0_set = true;
-    }
-    return dc->r0;
+    return tcg_constant_i32(0);
 }
 
 static TCGv_i32 reg_for_write(DisasContext *dc, int reg)
@@ -194,10 +184,7 @@ static TCGv_i32 reg_for_write(DisasContext *dc, int reg)
     if (likely(reg != 0)) {
         return cpu_R[reg];
     }
-    if (dc->r0 == NULL) {
-        dc->r0 = tcg_temp_new_i32();
-    }
-    return dc->r0;
+    return tcg_temp_new_i32();
 }
 
 static bool do_typea(DisasContext *dc, arg_typea *arg, bool side_effects,
@@ -1621,8 +1608,6 @@ static void mb_tr_init_disas_context(DisasContextBase *dcb, CPUState *cs)
     dc->cfg = &cpu->cfg;
     dc->tb_flags = dc->base.tb->flags;
     dc->ext_imm = dc->base.tb->cs_base;
-    dc->r0 = NULL;
-    dc->r0_set = false;
     dc->mem_index = cpu_mmu_index(cs, false);
     dc->jmp_cond = dc->tb_flags & D_FLAG ? TCG_COND_ALWAYS : TCG_COND_NEVER;
     dc->jmp_dest = -1;
@@ -1660,11 +1645,6 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
         trap_illegal(dc, true);
     }
 
-    if (dc->r0) {
-        dc->r0 = NULL;
-        dc->r0_set = false;
-    }
-
     /* Discard the imm global when its contents cannot be used. */
     if ((dc->tb_flags & ~dc->tb_flags_to_set) & IMM_FLAG) {
         tcg_gen_discard_i32(cpu_imm);
-- 
2.43.0

Re: [PATCH 8/9] target/microblaze: Drop DisasContext.r0

[Philippe Mathieu-Daudé]

On 12/2/25 23:01, Richard Henderson wrote:
> Return a constant 0 from reg_for_read, and a new
> temporary from reg_for_write.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/microblaze/translate.c | 24 ++----------------------
>   1 file changed, 2 insertions(+), 22 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 9/9] target/microblaze: Simplify compute_ldst_addr_type{a,b}

[Richard Henderson]

Require TCGv_i32 and TCGv be identical, so drop
the extensions.  Return constants when possible
instead of a mov into a temporary.  Return register
inputs unchanged when possible.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/translate.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 5750c45dac..5f3b94e683 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -612,19 +612,18 @@ DO_TYPEBI(xori, false, tcg_gen_xori_i32)
 
 static TCGv compute_ldst_addr_typea(DisasContext *dc, int ra, int rb)
 {
-    TCGv ret = tcg_temp_new();
+    TCGv ret;
 
     /* If any of the regs is r0, set t to the value of the other reg.  */
     if (ra && rb) {
-        TCGv_i32 tmp = tcg_temp_new_i32();
-        tcg_gen_add_i32(tmp, cpu_R[ra], cpu_R[rb]);
-        tcg_gen_extu_i32_tl(ret, tmp);
+        ret = tcg_temp_new_i32();
+        tcg_gen_add_i32(ret, cpu_R[ra], cpu_R[rb]);
     } else if (ra) {
-        tcg_gen_extu_i32_tl(ret, cpu_R[ra]);
+        ret = cpu_R[ra];
     } else if (rb) {
-        tcg_gen_extu_i32_tl(ret, cpu_R[rb]);
+        ret = cpu_R[rb];
     } else {
-        tcg_gen_movi_tl(ret, 0);
+        ret = tcg_constant_i32(0);
     }
 
     if ((ra == 1 || rb == 1) && dc->cfg->stackprot) {
@@ -635,15 +634,16 @@ static TCGv compute_ldst_addr_typea(DisasContext *dc, int ra, int rb)
 
 static TCGv compute_ldst_addr_typeb(DisasContext *dc, int ra, int imm)
 {
-    TCGv ret = tcg_temp_new();
+    TCGv ret;
 
     /* If any of the regs is r0, set t to the value of the other reg.  */
-    if (ra) {
-        TCGv_i32 tmp = tcg_temp_new_i32();
-        tcg_gen_addi_i32(tmp, cpu_R[ra], imm);
-        tcg_gen_extu_i32_tl(ret, tmp);
+    if (ra && imm) {
+        ret = tcg_temp_new_i32();
+        tcg_gen_addi_i32(ret, cpu_R[ra], imm);
+    } else if (ra) {
+        ret = cpu_R[ra];
     } else {
-        tcg_gen_movi_tl(ret, (uint32_t)imm);
+        ret = tcg_constant_i32(imm);
     }
 
     if (ra == 1 && dc->cfg->stackprot) {
-- 
2.43.0

Re: [PATCH 9/9] target/microblaze: Simplify compute_ldst_addr_type{a,b}

[Philippe Mathieu-Daudé]

On 12/2/25 23:01, Richard Henderson wrote:
> Require TCGv_i32 and TCGv be identical, so drop
> the extensions.  Return constants when possible
> instead of a mov into a temporary.  Return register
> inputs unchanged when possible.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/microblaze/translate.c | 26 +++++++++++++-------------
>   1 file changed, 13 insertions(+), 13 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Philippe Mathieu-Daudé]

+AMD folks

On 12/2/25 23:01, Richard Henderson wrote:
> Use out-of-line helpers to implement extended address memory ops.
> With this, we can reduce TARGET_LONG_BITS to the more natural 32
> for this 32-bit cpu.

I thought about something similar 2 months ago, but then realized
MicroBlaze cores can be synthetized in 64-bit, and IIRC there is
not much missing (I'd say effort would be to add 20% more of what
we currently have). Just wanted to mention before taking the
decision to restrict to 32-bit. OTOH if there are no plan for
adding 64-bit support at AMD, then I'm more than happy to simplify
by considering only 32-bit.

> Richard Henderson (9):
>    target/microblaze: Split out mb_unaligned_access_internal
>    target/microblaze: Split out mb_transaction_failed_internal
>    target/microblaze: Implement extended address load/store out of line
>    target/microblaze: Use uint64_t for CPUMBState.ear
>    target/microblaze: Use TCGv_i64 for compute_ldst_addr_ea
>    target/microblaze: Fix printf format in mmu_translate
>    target/microblaze: Use TARGET_LONG_BITS == 32 for system mode
>    target/microblaze: Drop DisasContext.r0
>    target/microblaze: Simplify compute_ldst_addr_type{a,b}

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Philippe Mathieu-Daudé]

On 13/2/25 13:37, Philippe Mathieu-Daudé wrote:
> +AMD folks
> 
> On 12/2/25 23:01, Richard Henderson wrote:
>> Use out-of-line helpers to implement extended address memory ops.
>> With this, we can reduce TARGET_LONG_BITS to the more natural 32
>> for this 32-bit cpu.
> 
> I thought about something similar 2 months ago, but then realized
> MicroBlaze cores can be synthetized in 64-bit, and IIRC there is
> not much missing (I'd say effort would be to add 20% more of what
> we currently have). Just wanted to mention before taking the
> decision to restrict to 32-bit. OTOH if there are no plan for
> adding 64-bit support at AMD, then I'm more than happy to simplify
> by considering only 32-bit.

3 weeks passed so I guess we are good to go with this simplification
series!

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Philippe Mathieu-Daudé]

Hi,

On 13/2/25 13:37, Philippe Mathieu-Daudé wrote:
> +AMD folks
> 
> On 12/2/25 23:01, Richard Henderson wrote:
>> Use out-of-line helpers to implement extended address memory ops.
>> With this, we can reduce TARGET_LONG_BITS to the more natural 32
>> for this 32-bit cpu.
> 
> I thought about something similar 2 months ago, but then realized
> MicroBlaze cores can be synthetized in 64-bit, and IIRC there is
> not much missing (I'd say effort would be to add 20% more of what
> we currently have). Just wanted to mention before taking the
> decision to restrict to 32-bit. OTOH if there are no plan for
> adding 64-bit support at AMD, then I'm more than happy to simplify
> by considering only 32-bit.

I gave this series another go, and figured the microblaze target
addition was done way before the 64-bit. C_DATA_SIZE value was fixed
as 32, and C_ADDR_SIZE was not mentioned. Later C_DATA_SIZE became
configurable as [32, 64] and C_ADDR_SIZE appeared.

Indeed what this series does is correctly implement the current
target as C_DATA_SIZE=32 (C_ADDR_SIZE=32 implied).

I had a quick look at what is missing for C_DATA_SIZE > 32 and it
is more than the 20% I first roughly estimated. So with the current
implementation, this series is doing the right thing IMHO.

Regards,

Phil.

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Philippe Mathieu-Daudé]

On 30/4/25 08:26, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 13/2/25 13:37, Philippe Mathieu-Daudé wrote:
>> +AMD folks
>>
>> On 12/2/25 23:01, Richard Henderson wrote:
>>> Use out-of-line helpers to implement extended address memory ops.
>>> With this, we can reduce TARGET_LONG_BITS to the more natural 32
>>> for this 32-bit cpu.
>>
>> I thought about something similar 2 months ago, but then realized
>> MicroBlaze cores can be synthetized in 64-bit, and IIRC there is
>> not much missing (I'd say effort would be to add 20% more of what
>> we currently have). Just wanted to mention before taking the
>> decision to restrict to 32-bit. OTOH if there are no plan for
>> adding 64-bit support at AMD, then I'm more than happy to simplify
>> by considering only 32-bit.
> 
> I gave this series another go, and figured the microblaze target
> addition was done way before the 64-bit. C_DATA_SIZE value was fixed
> as 32, and C_ADDR_SIZE was not mentioned. Later C_DATA_SIZE became
> configurable as [32, 64] and C_ADDR_SIZE appeared.

FTR C_ADDR_SIZE starts to be mentioned in Vivado 2016.1 release as

   • Included description of address extension, new in version 9.6.

Commit 72e387548534 (Jun 18 2015) made explicit supported versions
were 5.00.a up to 9.3 (per Vivado 2014.1 release).

Commit d79fcbc298b0 (Jan 11 2017) "Add CPU versions 9.4, 9.5 and 9.6",
and commit feac83af3be6 (Jun 15 2017) "Add CPU version 10.0" (released
in Vivado 2016.3, but MMU Physical Address Extension 'PAE' came in
Vivado 2017.1).

Vivado 2018.3 added MicroBlaze 64-bit implementation "new in version 11.0".

IIUC current implementation is correct w.r.t. v9.5.

I'm not so sure we can announce v9.6 and v10.0 as correctly implemented.

Looking at what our machines uses, latest is v8.40.b:

hw/microblaze/petalogix_ml605_mmu.c:88: 
object_property_set_str(OBJECT(cpu), "version", "8.10.a", &error_abort);
hw/microblaze/petalogix_s3adsp1800_mmu.c:78: 
object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
hw/microblaze/xlnx-zynqmp-pmu.c:95: 
object_property_set_str(OBJECT(&s->cpu), "version", "8.40.b",

Maybe we can deprecate / remove v9.6 & v10.0 to better add them with
a proper microblaze64 target implementation?

> Indeed what this series does is correctly implement the current
> target as C_DATA_SIZE=32 (C_ADDR_SIZE=32 implied).
> 
> I had a quick look at what is missing for C_DATA_SIZE > 32 and it
> is more than the 20% I first roughly estimated. So with the current
> implementation, this series is doing the right thing IMHO.
> 
> Regards,
> 
> Phil.

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Edgar E. Iglesias]

On Wed, Apr 30, 2025 at 09:29:20AM +0200, Philippe Mathieu-Daudé wrote:
> On 30/4/25 08:26, Philippe Mathieu-Daudé wrote:
> > Hi,
> > 
> > On 13/2/25 13:37, Philippe Mathieu-Daudé wrote:
> > > +AMD folks
> > > 
> > > On 12/2/25 23:01, Richard Henderson wrote:
> > > > Use out-of-line helpers to implement extended address memory ops.
> > > > With this, we can reduce TARGET_LONG_BITS to the more natural 32
> > > > for this 32-bit cpu.
> > > 
> > > I thought about something similar 2 months ago, but then realized
> > > MicroBlaze cores can be synthetized in 64-bit, and IIRC there is
> > > not much missing (I'd say effort would be to add 20% more of what
> > > we currently have). Just wanted to mention before taking the
> > > decision to restrict to 32-bit. OTOH if there are no plan for
> > > adding 64-bit support at AMD, then I'm more than happy to simplify
> > > by considering only 32-bit.
> > 
> > I gave this series another go, and figured the microblaze target
> > addition was done way before the 64-bit. C_DATA_SIZE value was fixed
> > as 32, and C_ADDR_SIZE was not mentioned. Later C_DATA_SIZE became
> > configurable as [32, 64] and C_ADDR_SIZE appeared.
> 
> FTR C_ADDR_SIZE starts to be mentioned in Vivado 2016.1 release as
> 
>   • Included description of address extension, new in version 9.6.
> 
> Commit 72e387548534 (Jun 18 2015) made explicit supported versions
> were 5.00.a up to 9.3 (per Vivado 2014.1 release).
> 
> Commit d79fcbc298b0 (Jan 11 2017) "Add CPU versions 9.4, 9.5 and 9.6",
> and commit feac83af3be6 (Jun 15 2017) "Add CPU version 10.0" (released
> in Vivado 2016.3, but MMU Physical Address Extension 'PAE' came in
> Vivado 2017.1).
> 
> Vivado 2018.3 added MicroBlaze 64-bit implementation "new in version 11.0".
> 
> IIUC current implementation is correct w.r.t. v9.5.
> 
> I'm not so sure we can announce v9.6 and v10.0 as correctly implemented.
> 

Hi Phil,

The version of a MicroBlaze CPU core is orthogonal with the 64bit support,
new cores can be used with or without 64bit support.

There may be optional features missing but I don't think it's necessary to
remove the versions.


> Looking at what our machines uses, latest is v8.40.b:
> 
> hw/microblaze/petalogix_ml605_mmu.c:88: object_property_set_str(OBJECT(cpu),
> "version", "8.10.a", &error_abort);
> hw/microblaze/petalogix_s3adsp1800_mmu.c:78:
> object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
> hw/microblaze/xlnx-zynqmp-pmu.c:95: object_property_set_str(OBJECT(&s->cpu),
> "version", "8.40.b",
> 
> Maybe we can deprecate / remove v9.6 & v10.0 to better add them with
> a proper microblaze64 target implementation?

IIRC, there're Xilinx internal verification suites that check for exact
versions of cores, so for the Xilinx models with platform cores
(e.g CSU, PMU, PPU's etc) we try to instantiate them with versions
matching real HW even though most of the time there's no SW visible
difference (other than the ID) and no difference to QEMU.

Cheers,
Edgar

> 
> > Indeed what this series does is correctly implement the current
> > target as C_DATA_SIZE=32 (C_ADDR_SIZE=32 implied).
> > 
> > I had a quick look at what is missing for C_DATA_SIZE > 32 and it
> > is more than the 20% I first roughly estimated. So with the current
> > implementation, this series is doing the right thing IMHO.
> > 
> > Regards,
> > 
> > Phil.
> 

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Edgar E. Iglesias]

On Wed, Apr 30, 2025 at 08:26:23AM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 13/2/25 13:37, Philippe Mathieu-Daudé wrote:
> > +AMD folks
> > 
> > On 12/2/25 23:01, Richard Henderson wrote:
> > > Use out-of-line helpers to implement extended address memory ops.
> > > With this, we can reduce TARGET_LONG_BITS to the more natural 32
> > > for this 32-bit cpu.
> > 
> > I thought about something similar 2 months ago, but then realized
> > MicroBlaze cores can be synthetized in 64-bit, and IIRC there is
> > not much missing (I'd say effort would be to add 20% more of what
> > we currently have). Just wanted to mention before taking the
> > decision to restrict to 32-bit. OTOH if there are no plan for
> > adding 64-bit support at AMD, then I'm more than happy to simplify
> > by considering only 32-bit.
> 
> I gave this series another go, and figured the microblaze target
> addition was done way before the 64-bit. C_DATA_SIZE value was fixed
> as 32, and C_ADDR_SIZE was not mentioned. Later C_DATA_SIZE became
> configurable as [32, 64] and C_ADDR_SIZE appeared.
> 
> Indeed what this series does is correctly implement the current
> target as C_DATA_SIZE=32 (C_ADDR_SIZE=32 implied).
> 
> I had a quick look at what is missing for C_DATA_SIZE > 32 and it
> is more than the 20% I first roughly estimated. So with the current
> implementation, this series is doing the right thing IMHO.
>


Hi,

Seems I lost track of this series. I agree that Richard's series looks good.

At the time I may have had some idea of prepping for full 64bit support,
I don't remember. If we ever add full 64bit support we can have have
another look and see how it makes sense to change things.

Cheers,
Edgar

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Edgar E. Iglesias]

On Wed, Feb 12, 2025 at 02:01:46PM -0800, Richard Henderson wrote:
> Use out-of-line helpers to implement extended address memory ops.
> With this, we can reduce TARGET_LONG_BITS to the more natural 32
> for this 32-bit cpu.


Series looks good to me:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> 
> 
> r~
> 
> 
> Richard Henderson (9):
>   target/microblaze: Split out mb_unaligned_access_internal
>   target/microblaze: Split out mb_transaction_failed_internal
>   target/microblaze: Implement extended address load/store out of line
>   target/microblaze: Use uint64_t for CPUMBState.ear
>   target/microblaze: Use TCGv_i64 for compute_ldst_addr_ea
>   target/microblaze: Fix printf format in mmu_translate
>   target/microblaze: Use TARGET_LONG_BITS == 32 for system mode
>   target/microblaze: Drop DisasContext.r0
>   target/microblaze: Simplify compute_ldst_addr_type{a,b}
> 
>  target/microblaze/cpu.h                  |   5 +-
>  target/microblaze/helper.h               |  16 ++-
>  target/microblaze/helper.c               |  25 ++--
>  target/microblaze/mmu.c                  |   3 +-
>  target/microblaze/op_helper.c            | 157 +++++++++++++++++++----
>  target/microblaze/translate.c            | 118 +++++++++--------
>  configs/targets/microblaze-softmmu.mak   |   4 +-
>  configs/targets/microblazeel-softmmu.mak |   4 +-
>  8 files changed, 228 insertions(+), 104 deletions(-)
> 
> -- 
> 2.43.0
> 

Re: [PATCH 0/9] target/microblaze: Always use TARGET_LONG_BITS == 32

[Philippe Mathieu-Daudé]

On 30/4/25 13:09, Edgar E. Iglesias wrote:
> On Wed, Feb 12, 2025 at 02:01:46PM -0800, Richard Henderson wrote:
>> Use out-of-line helpers to implement extended address memory ops.
>> With this, we can reduce TARGET_LONG_BITS to the more natural 32
>> for this 32-bit cpu.
> 
> 
> Series looks good to me:
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>

Thanks!

Richard, FYI I'm going to respin your series.

>> Richard Henderson (9):
>>    target/microblaze: Split out mb_unaligned_access_internal
>>    target/microblaze: Split out mb_transaction_failed_internal
>>    target/microblaze: Implement extended address load/store out of line
>>    target/microblaze: Use uint64_t for CPUMBState.ear
>>    target/microblaze: Use TCGv_i64 for compute_ldst_addr_ea
>>    target/microblaze: Fix printf format in mmu_translate
>>    target/microblaze: Use TARGET_LONG_BITS == 32 for system mode
>>    target/microblaze: Drop DisasContext.r0
>>    target/microblaze: Simplify compute_ldst_addr_type{a,b}