[PATCH 00/12] accel/tcg: Fix cross-page pointer wrapping issue

[PATCH 00/12] accel/tcg: Fix cross-page pointer wrapping issue

[Richard Henderson]

As detailed in

https://lore.kernel.org/qemu-devel/174595764300.3422.13156465553505851834-0@git.sr.ht/

there's an issue with an unaligned access that falls off
the end of the last page.  To solve this, we need to know
about the state of the cpu, so add a new target hook.

There are arguments to the hook that are currently unused,
but would appear to come in handy for AArch64 v9.5 FEAT_CPA2,
which we do not yet implement.


r~


Richard Henderson (12):
  accel/tcg: Add TCGCPUOps.pointer_wrap
  target: Use cpu_pointer_wrap_notreached for strict align targets
  target: Use cpu_pointer_wrap_uint32 for 32-bit targets
  target/arm: Fill in TCGCPUOps.pointer_wrap
  target/i386: Fill in TCGCPUOps.pointer_wrap
  target/loongarch: Fill in TCGCPUOps.pointer_wrap
  target/mips: Fill in TCGCPUOps.pointer_wrap
  target/ppc: Fill in TCGCPUOps.pointer_wrap
  target/riscv: Fill in TCGCPUOps.pointer_wrap
  target/s390x: Fill in TCGCPUOps.pointer_wrap
  target/sparc: Fill in TCGCPUOps.pointer_wrap
  accel/tcg: Assert TCGCPUOps.pointer_wrap is set

 include/accel/tcg/cpu-ops.h | 13 +++++++++++++
 accel/tcg/cpu-exec.c        |  1 +
 accel/tcg/cputlb.c          | 22 ++++++++++++++++++++++
 target/alpha/cpu.c          |  1 +
 target/arm/cpu.c            | 24 ++++++++++++++++++++++++
 target/arm/tcg/cpu-v7m.c    |  1 +
 target/avr/cpu.c            |  6 ++++++
 target/hppa/cpu.c           |  1 +
 target/i386/tcg/tcg-cpu.c   |  7 +++++++
 target/loongarch/cpu.c      |  7 +++++++
 target/m68k/cpu.c           |  1 +
 target/microblaze/cpu.c     |  1 +
 target/mips/cpu.c           |  9 +++++++++
 target/openrisc/cpu.c       |  1 +
 target/ppc/cpu_init.c       |  7 +++++++
 target/riscv/tcg/tcg-cpu.c  | 26 ++++++++++++++++++++++++++
 target/rx/cpu.c             |  1 +
 target/s390x/cpu.c          |  9 +++++++++
 target/sh4/cpu.c            |  1 +
 target/sparc/cpu.c          | 13 +++++++++++++
 target/tricore/cpu.c        |  1 +
 target/xtensa/cpu.c         |  1 +
 22 files changed, 154 insertions(+)

-- 
2.43.0

[PATCH 01/12] accel/tcg: Add TCGCPUOps.pointer_wrap

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/accel/tcg/cpu-ops.h | 7 +++++++
 accel/tcg/cputlb.c          | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h
index cd22e5d5b9..83b2c2c864 100644
--- a/include/accel/tcg/cpu-ops.h
+++ b/include/accel/tcg/cpu-ops.h
@@ -222,6 +222,13 @@ struct TCGCPUOps {
     bool (*tlb_fill)(CPUState *cpu, vaddr address, int size,
                      MMUAccessType access_type, int mmu_idx,
                      bool probe, uintptr_t retaddr);
+    /**
+     * @pointer_wrap:
+     *
+     * We have incremented @base to @result, resulting in a page change.
+     * For the current cpu state, adjust @result for possible overflow.
+     */
+    vaddr (*pointer_wrap)(CPUState *cpu, int mmu_idx, vaddr result, vaddr base);
     /**
      * @do_transaction_failed: Callback for handling failed memory transactions
      * (ie bus faults or external aborts; not MMU faults)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 5f6d7c601c..c394293d33 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1773,6 +1773,12 @@ static bool mmu_lookup(CPUState *cpu, vaddr addr, MemOpIdx oi,
         l->page[1].size = l->page[0].size - size0;
         l->page[0].size = size0;
 
+        if (cpu->cc->tcg_ops->pointer_wrap) {
+            l->page[1].addr = cpu->cc->tcg_ops->pointer_wrap(cpu, l->mmu_idx,
+                                                             l->page[1].addr,
+                                                             addr);
+        }
+
         /*
          * Lookup both pages, recognizing exceptions from either.  If the
          * second lookup potentially resized, refresh first CPUTLBEntryFull.
-- 
2.43.0

[PATCH 02/12] target: Use cpu_pointer_wrap_notreached for strict align targets

[Richard Henderson]

Alpha, HPPA, and SH4 always use aligned addresses,
and therefore never produce accesses that cross pages.

Cc: Helge Deller <deller@gmx.de>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/accel/tcg/cpu-ops.h |  5 +++++
 accel/tcg/cputlb.c          | 13 +++++++++++++
 target/alpha/cpu.c          |  1 +
 target/hppa/cpu.c           |  1 +
 target/sh4/cpu.c            |  1 +
 5 files changed, 21 insertions(+)

diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h
index 83b2c2c864..4f3b4fd3bc 100644
--- a/include/accel/tcg/cpu-ops.h
+++ b/include/accel/tcg/cpu-ops.h
@@ -322,6 +322,11 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
  */
 int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
 
+/*
+ * Common pointer_wrap implementations.
+ */
+vaddr cpu_pointer_wrap_notreached(CPUState *, int, vaddr, vaddr);
+
 #endif
 
 #endif /* TCG_CPU_OPS_H */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index c394293d33..75cd875948 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2932,3 +2932,16 @@ uint64_t cpu_ldq_code_mmu(CPUArchState *env, vaddr addr,
 {
     return do_ld8_mmu(env_cpu(env), addr, oi, retaddr, MMU_INST_FETCH);
 }
+
+/*
+ * Common pointer_wrap implementations.
+ */
+
+/*
+ * To be used for strict alignment targets.
+ * Because no accesses are unaligned, no accesses wrap either.
+ */
+vaddr cpu_pointer_wrap_notreached(CPUState *cs, int idx, vaddr res, vaddr base)
+{
+    g_assert_not_reached();
+}
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 890b84c032..2082db45ea 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -261,6 +261,7 @@ static const TCGCPUOps alpha_tcg_ops = {
     .record_sigbus = alpha_cpu_record_sigbus,
 #else
     .tlb_fill = alpha_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_notreached,
     .cpu_exec_interrupt = alpha_cpu_exec_interrupt,
     .cpu_exec_halt = alpha_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 6465181543..24777727e6 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -269,6 +269,7 @@ static const TCGCPUOps hppa_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill_align = hppa_cpu_tlb_fill_align,
+    .pointer_wrap = cpu_pointer_wrap_notreached,
     .cpu_exec_interrupt = hppa_cpu_exec_interrupt,
     .cpu_exec_halt = hppa_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index b35f18e250..4f561e8c91 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -296,6 +296,7 @@ static const TCGCPUOps superh_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = superh_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_notreached,
     .cpu_exec_interrupt = superh_cpu_exec_interrupt,
     .cpu_exec_halt = superh_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 03/12] target: Use cpu_pointer_wrap_uint32 for 32-bit targets

[Richard Henderson]

M68K, MicroBlaze, OpenRISC, RX, TriCore and Xtensa are
all 32-bit targets.  AVR is more complicated, but using
a 32-bit wrap preserves current behaviour.

Cc: Michael Rolnik <mrolnik@gmail.com>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Cc: Stafford Horne <shorne@gmail.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/accel/tcg/cpu-ops.h | 1 +
 accel/tcg/cputlb.c          | 6 ++++++
 target/avr/cpu.c            | 6 ++++++
 target/m68k/cpu.c           | 1 +
 target/microblaze/cpu.c     | 1 +
 target/openrisc/cpu.c       | 1 +
 target/rx/cpu.c             | 1 +
 target/tricore/cpu.c        | 1 +
 target/xtensa/cpu.c         | 1 +
 9 files changed, 19 insertions(+)

diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h
index 4f3b4fd3bc..dd8ea30016 100644
--- a/include/accel/tcg/cpu-ops.h
+++ b/include/accel/tcg/cpu-ops.h
@@ -326,6 +326,7 @@ int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
  * Common pointer_wrap implementations.
  */
 vaddr cpu_pointer_wrap_notreached(CPUState *, int, vaddr, vaddr);
+vaddr cpu_pointer_wrap_uint32(CPUState *, int, vaddr, vaddr);
 
 #endif
 
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 75cd875948..022d555f48 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2945,3 +2945,9 @@ vaddr cpu_pointer_wrap_notreached(CPUState *cs, int idx, vaddr res, vaddr base)
 {
     g_assert_not_reached();
 }
+
+/* To be used for strict 32-bit targets. */
+vaddr cpu_pointer_wrap_uint32(CPUState *cs, int idx, vaddr res, vaddr base)
+{
+    return (uint32_t)res;
+}
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 250241541b..6995de6a12 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -250,6 +250,12 @@ static const TCGCPUOps avr_tcg_ops = {
     .cpu_exec_reset = cpu_reset,
     .tlb_fill = avr_cpu_tlb_fill,
     .do_interrupt = avr_cpu_do_interrupt,
+    /*
+     * TODO: code and data wrapping are different, but for the most part
+     * AVR only references bytes or aligned code fetches.  But we use
+     * non-aligned MO_16 accesses for stack push/pop.
+     */
+    .pointer_wrap = cpu_pointer_wrap_uint32,
 };
 
 static void avr_cpu_class_init(ObjectClass *oc, const void *data)
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index c5196a612e..6a09db3a6f 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -619,6 +619,7 @@ static const TCGCPUOps m68k_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = m68k_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_uint32,
     .cpu_exec_interrupt = m68k_cpu_exec_interrupt,
     .cpu_exec_halt = m68k_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index d069e40e70..5eff1610c2 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -449,6 +449,7 @@ static const TCGCPUOps mb_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = mb_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_uint32,
     .cpu_exec_interrupt = mb_cpu_exec_interrupt,
     .cpu_exec_halt = mb_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 054ad33360..dfbb2df643 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -265,6 +265,7 @@ static const TCGCPUOps openrisc_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = openrisc_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_uint32,
     .cpu_exec_interrupt = openrisc_cpu_exec_interrupt,
     .cpu_exec_halt = openrisc_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 36eba75545..c6dd5d6f83 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -225,6 +225,7 @@ static const TCGCPUOps rx_tcg_ops = {
     .restore_state_to_opc = rx_restore_state_to_opc,
     .mmu_index = rx_cpu_mmu_index,
     .tlb_fill = rx_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_uint32,
 
     .cpu_exec_interrupt = rx_cpu_exec_interrupt,
     .cpu_exec_halt = rx_cpu_has_work,
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index e56f90fde9..4f035b6f76 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -190,6 +190,7 @@ static const TCGCPUOps tricore_tcg_ops = {
     .restore_state_to_opc = tricore_restore_state_to_opc,
     .mmu_index = tricore_cpu_mmu_index,
     .tlb_fill = tricore_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_uint32,
     .cpu_exec_interrupt = tricore_cpu_exec_interrupt,
     .cpu_exec_halt = tricore_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 91b71b6caa..ea9b6df3aa 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -318,6 +318,7 @@ static const TCGCPUOps xtensa_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = xtensa_cpu_tlb_fill,
+    .pointer_wrap = cpu_pointer_wrap_uint32,
     .cpu_exec_interrupt = xtensa_cpu_exec_interrupt,
     .cpu_exec_halt = xtensa_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 04/12] target/arm: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

For a-profile, check A32 vs A64 state.
For m-profile, use cpu_pointer_wrap_uint32.

Cc: qemu-arm@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c         | 24 ++++++++++++++++++++++++
 target/arm/tcg/cpu-v7m.c |  1 +
 2 files changed, 25 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 45cb6fd7ee..18edcf49c6 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2710,6 +2710,29 @@ static const struct SysemuCPUOps arm_sysemu_ops = {
 #endif
 
 #ifdef CONFIG_TCG
+#ifndef CONFIG_USER_ONLY
+static vaddr aprofile_pointer_wrap(CPUState *cs, int mmu_idx,
+                                   vaddr result, vaddr base)
+{
+    /*
+     * The Stage2 and Phys indexes are only used for ptw on arm32,
+     * and all pte's are aligned, so we never produce a wrap for these.
+     * Double check that we're not truncating a 40-bit physical address.
+     */
+    assert((unsigned)mmu_idx < (ARMMMUIdx_Stage2_S & ARM_MMU_IDX_COREIDX_MASK));
+
+    if (!is_a64(cpu_env(cs))) {
+        return (uint32_t)result;
+    }
+
+    /*
+     * TODO: For FEAT_CPA2, decide how to we want to resolve
+     * Unpredictable_CPACHECK in AddressIncrement.
+     */
+    return result;
+}
+#endif /* !CONFIG_USER_ONLY */
+
 static const TCGCPUOps arm_tcg_ops = {
     .mttcg_supported = true,
     /* ARM processors have a weak memory model */
@@ -2729,6 +2752,7 @@ static const TCGCPUOps arm_tcg_ops = {
     .untagged_addr = aarch64_untagged_addr,
 #else
     .tlb_fill_align = arm_cpu_tlb_fill_align,
+    .pointer_wrap = aprofile_pointer_wrap,
     .cpu_exec_interrupt = arm_cpu_exec_interrupt,
     .cpu_exec_halt = arm_cpu_exec_halt,
     .cpu_exec_reset = cpu_reset,
diff --git a/target/arm/tcg/cpu-v7m.c b/target/arm/tcg/cpu-v7m.c
index 95b23d9b55..8e1a083b91 100644
--- a/target/arm/tcg/cpu-v7m.c
+++ b/target/arm/tcg/cpu-v7m.c
@@ -249,6 +249,7 @@ static const TCGCPUOps arm_v7m_tcg_ops = {
     .record_sigbus = arm_cpu_record_sigbus,
 #else
     .tlb_fill_align = arm_cpu_tlb_fill_align,
+    .pointer_wrap = cpu_pointer_wrap_uint32,
     .cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt,
     .cpu_exec_halt = arm_cpu_exec_halt,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 05/12] target/i386: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

Check 32 vs 64-bit state.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/tcg/tcg-cpu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index 179dfdf064..6f5dc06b3b 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -149,6 +149,12 @@ static void x86_cpu_exec_reset(CPUState *cs)
     do_cpu_init(env_archcpu(env));
     cs->exception_index = EXCP_HALTED;
 }
+
+static vaddr x86_pointer_wrap(CPUState *cs, int mmu_idx,
+                              vaddr result, vaddr base)
+{
+    return cpu_env(cs)->hflags & HF_CS64_MASK ? result : (uint32_t)result;
+}
 #endif
 
 const TCGCPUOps x86_tcg_ops = {
@@ -172,6 +178,7 @@ const TCGCPUOps x86_tcg_ops = {
     .record_sigbus = x86_cpu_record_sigbus,
 #else
     .tlb_fill = x86_cpu_tlb_fill,
+    .pointer_wrap = x86_pointer_wrap,
     .do_interrupt = x86_cpu_do_interrupt,
     .cpu_exec_halt = x86_cpu_exec_halt,
     .cpu_exec_interrupt = x86_cpu_exec_interrupt,
-- 
2.43.0

[PATCH 06/12] target/loongarch: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

Check va32 state.

Cc: Song Gao <gaosong@loongson.cn>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/loongarch/cpu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index f7535d1be7..abad84c054 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -334,6 +334,12 @@ static bool loongarch_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
     }
     return false;
 }
+
+static vaddr loongarch_pointer_wrap(CPUState *cs, int mmu_idx,
+                                    vaddr result, vaddr base)
+{
+    return is_va32(cpu_env(cs)) ? (uint32_t)result : result;
+}
 #endif
 
 static TCGTBCPUState loongarch_get_tb_cpu_state(CPUState *cs)
@@ -889,6 +895,7 @@ static const TCGCPUOps loongarch_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = loongarch_cpu_tlb_fill,
+    .pointer_wrap = loongarch_pointer_wrap,
     .cpu_exec_interrupt = loongarch_cpu_exec_interrupt,
     .cpu_exec_halt = loongarch_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 07/12] target/mips: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

Check 32 vs 64-bit addressing state.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/cpu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 4cbfb9435a..1f6c41fd34 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -560,6 +560,14 @@ static TCGTBCPUState mips_get_tb_cpu_state(CPUState *cs)
     };
 }
 
+#ifndef CONFIG_USER_ONLY
+static vaddr mips_pointer_wrap(CPUState *cs, int mmu_idx,
+                               vaddr result, vaddr base)
+{
+    return cpu_env(cs)->hflags & MIPS_HFLAG_AWRAP ? (int32_t)result : result;
+}
+#endif
+
 static const TCGCPUOps mips_tcg_ops = {
     .mttcg_supported = TARGET_LONG_BITS == 32,
     .guest_default_memory_order = 0,
@@ -573,6 +581,7 @@ static const TCGCPUOps mips_tcg_ops = {
 
 #if !defined(CONFIG_USER_ONLY)
     .tlb_fill = mips_cpu_tlb_fill,
+    .pointer_wrap = mips_pointer_wrap,
     .cpu_exec_interrupt = mips_cpu_exec_interrupt,
     .cpu_exec_halt = mips_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 08/12] target/ppc: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

Check 32 vs 64-bit state.

Cc: qemu-ppc@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/cpu_init.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index cf88a18244..1f18967b62 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7386,6 +7386,12 @@ static void ppc_cpu_exec_exit(CPUState *cs)
         cpu->vhyp_class->cpu_exec_exit(cpu->vhyp, cpu);
     }
 }
+
+static vaddr ppc_pointer_wrap(CPUState *cs, int mmu_idx,
+                              vaddr result, vaddr base)
+{
+    return (cpu_env(cs)->hflags >> HFLAGS_64) & 1 ? result : (uint32_t)result;
+}
 #endif /* CONFIG_TCG */
 
 #endif /* !CONFIG_USER_ONLY */
@@ -7490,6 +7496,7 @@ static const TCGCPUOps ppc_tcg_ops = {
   .record_sigsegv = ppc_cpu_record_sigsegv,
 #else
   .tlb_fill = ppc_cpu_tlb_fill,
+  .pointer_wrap = ppc_pointer_wrap,
   .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
   .cpu_exec_halt = ppc_cpu_has_work,
   .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 09/12] target/riscv: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

Check 32 vs 64-bit and pointer masking state.

Cc: qemu-riscv@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 55e00972b7..267186e5e3 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -237,6 +237,31 @@ static void riscv_restore_state_to_opc(CPUState *cs,
     env->excp_uw2 = data[2];
 }
 
+#ifndef CONFIG_USER_ONLY
+static vaddr riscv_pointer_wrap(CPUState *cs, int mmu_idx,
+                                vaddr result, vaddr base)
+{
+    CPURISCVState *env = cpu_env(cs);
+    uint32_t pm_len;
+    bool pm_signext;
+
+    if (cpu_address_xl(env) == MXL_RV32) {
+        return (uint32_t)result;
+    }
+
+    pm_len = riscv_pm_get_pmlen(riscv_pm_get_pmm(env));
+    if (pm_len == 0) {
+        return result;
+    }
+
+    pm_signext = riscv_cpu_virt_mem_enabled(env);
+    if (pm_signext) {
+        return sextract64(result, 0, 64 - pm_len);
+    }
+    return extract64(result, 0, 64 - pm_len);
+}
+#endif
+
 const TCGCPUOps riscv_tcg_ops = {
     .mttcg_supported = true,
     .guest_default_memory_order = 0,
@@ -250,6 +275,7 @@ const TCGCPUOps riscv_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = riscv_cpu_tlb_fill,
+    .pointer_wrap = riscv_pointer_wrap,
     .cpu_exec_interrupt = riscv_cpu_exec_interrupt,
     .cpu_exec_halt = riscv_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 10/12] target/s390x: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

Use the existing wrap_address function.

Cc: qemu-s390x@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/cpu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 9c1158ebcc..f05ce317da 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -347,6 +347,14 @@ static TCGTBCPUState s390x_get_tb_cpu_state(CPUState *cs)
     };
 }
 
+#ifndef CONFIG_USER_ONLY
+static vaddr s390_pointer_wrap(CPUState *cs, int mmu_idx,
+                               vaddr result, vaddr base)
+{
+    return wrap_address(cpu_env(cs), result);
+}
+#endif
+
 static const TCGCPUOps s390_tcg_ops = {
     .mttcg_supported = true,
     .precise_smc = true,
@@ -367,6 +375,7 @@ static const TCGCPUOps s390_tcg_ops = {
     .record_sigbus = s390_cpu_record_sigbus,
 #else
     .tlb_fill = s390_cpu_tlb_fill,
+    .pointer_wrap = s390_pointer_wrap,
     .cpu_exec_interrupt = s390_cpu_exec_interrupt,
     .cpu_exec_halt = s390_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 11/12] target/sparc: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

Check address masking state for sparc64.

Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/cpu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 2a3e408923..ed7701b02f 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -1002,6 +1002,18 @@ static const struct SysemuCPUOps sparc_sysemu_ops = {
 #ifdef CONFIG_TCG
 #include "accel/tcg/cpu-ops.h"
 
+#ifndef CONFIG_USER_ONLY
+static vaddr sparc_pointer_wrap(CPUState *cs, int mmu_idx,
+                                vaddr result, vaddr base)
+{
+#ifdef TARGET_SPARC64
+    return cpu_env(cs)->pstate & PS_AM ? (uint32_t)result : result;
+#else
+    return (uint32_t)result;
+#endif
+}
+#endif
+
 static const TCGCPUOps sparc_tcg_ops = {
     /*
      * From Oracle SPARC Architecture 2015:
@@ -1036,6 +1048,7 @@ static const TCGCPUOps sparc_tcg_ops = {
 
 #ifndef CONFIG_USER_ONLY
     .tlb_fill = sparc_cpu_tlb_fill,
+    .pointer_wrap = sparc_pointer_wrap,
     .cpu_exec_interrupt = sparc_cpu_exec_interrupt,
     .cpu_exec_halt = sparc_cpu_has_work,
     .cpu_exec_reset = cpu_reset,
-- 
2.43.0

[PATCH 12/12] accel/tcg: Assert TCGCPUOps.pointer_wrap is set

[Richard Henderson]

All targets now provide the function, so we can
make the call unconditional.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cpu-exec.c | 1 +
 accel/tcg/cputlb.c   | 7 ++-----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index cc5f362305..713bdb2056 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -1039,6 +1039,7 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
         assert(tcg_ops->cpu_exec_halt);
         assert(tcg_ops->cpu_exec_interrupt);
         assert(tcg_ops->cpu_exec_reset);
+        assert(tcg_ops->pointer_wrap);
 #endif /* !CONFIG_USER_ONLY */
         assert(tcg_ops->translate_code);
         assert(tcg_ops->get_tb_cpu_state);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 022d555f48..b49f0dcc29 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1773,11 +1773,8 @@ static bool mmu_lookup(CPUState *cpu, vaddr addr, MemOpIdx oi,
         l->page[1].size = l->page[0].size - size0;
         l->page[0].size = size0;
 
-        if (cpu->cc->tcg_ops->pointer_wrap) {
-            l->page[1].addr = cpu->cc->tcg_ops->pointer_wrap(cpu, l->mmu_idx,
-                                                             l->page[1].addr,
-                                                             addr);
-        }
+        l->page[1].addr = cpu->cc->tcg_ops->pointer_wrap(cpu, l->mmu_idx,
+                                                         l->page[1].addr, addr);
 
         /*
          * Lookup both pages, recognizing exceptions from either.  If the
-- 
2.43.0

Re: [PATCH 01/12] accel/tcg: Add TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/accel/tcg/cpu-ops.h | 7 +++++++
>   accel/tcg/cputlb.c          | 6 ++++++
>   2 files changed, 13 insertions(+)

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

Re: [PATCH 02/12] target: Use cpu_pointer_wrap_notreached for strict align targets

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Alpha, HPPA, and SH4 always use aligned addresses,
> and therefore never produce accesses that cross pages.
> 
> Cc: Helge Deller <deller@gmx.de>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/accel/tcg/cpu-ops.h |  5 +++++
>   accel/tcg/cputlb.c          | 13 +++++++++++++
>   target/alpha/cpu.c          |  1 +
>   target/hppa/cpu.c           |  1 +
>   target/sh4/cpu.c            |  1 +
>   5 files changed, 21 insertions(+)

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

Re: [PATCH 03/12] target: Use cpu_pointer_wrap_uint32 for 32-bit targets

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> M68K, MicroBlaze, OpenRISC, RX, TriCore and Xtensa are
> all 32-bit targets.  AVR is more complicated, but using
> a 32-bit wrap preserves current behaviour.
> 
> Cc: Michael Rolnik <mrolnik@gmail.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/accel/tcg/cpu-ops.h | 1 +
>   accel/tcg/cputlb.c          | 6 ++++++
>   target/avr/cpu.c            | 6 ++++++
>   target/m68k/cpu.c           | 1 +
>   target/microblaze/cpu.c     | 1 +
>   target/openrisc/cpu.c       | 1 +
>   target/rx/cpu.c             | 1 +
>   target/tricore/cpu.c        | 1 +
>   target/xtensa/cpu.c         | 1 +
>   9 files changed, 19 insertions(+)

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

Re: [PATCH 12/12] accel/tcg: Assert TCGCPUOps.pointer_wrap is set

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> All targets now provide the function, so we can
> make the call unconditional.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/cpu-exec.c | 1 +
>   accel/tcg/cputlb.c   | 7 ++-----
>   2 files changed, 3 insertions(+), 5 deletions(-)

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

Re: [PATCH 03/12] target: Use cpu_pointer_wrap_uint32 for 32-bit targets

[Edgar E. Iglesias]

On Sun, May 04, 2025 at 01:57:04PM -0700, Richard Henderson wrote:
> M68K, MicroBlaze, OpenRISC, RX, TriCore and Xtensa are
> all 32-bit targets.  AVR is more complicated, but using
> a 32-bit wrap preserves current behaviour.
> 
> Cc: Michael Rolnik <mrolnik@gmail.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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



> ---
>  include/accel/tcg/cpu-ops.h | 1 +
>  accel/tcg/cputlb.c          | 6 ++++++
>  target/avr/cpu.c            | 6 ++++++
>  target/m68k/cpu.c           | 1 +
>  target/microblaze/cpu.c     | 1 +
>  target/openrisc/cpu.c       | 1 +
>  target/rx/cpu.c             | 1 +
>  target/tricore/cpu.c        | 1 +
>  target/xtensa/cpu.c         | 1 +
>  9 files changed, 19 insertions(+)
> 
> diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h
> index 4f3b4fd3bc..dd8ea30016 100644
> --- a/include/accel/tcg/cpu-ops.h
> +++ b/include/accel/tcg/cpu-ops.h
> @@ -326,6 +326,7 @@ int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
>   * Common pointer_wrap implementations.
>   */
>  vaddr cpu_pointer_wrap_notreached(CPUState *, int, vaddr, vaddr);
> +vaddr cpu_pointer_wrap_uint32(CPUState *, int, vaddr, vaddr);
>  
>  #endif
>  
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 75cd875948..022d555f48 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -2945,3 +2945,9 @@ vaddr cpu_pointer_wrap_notreached(CPUState *cs, int idx, vaddr res, vaddr base)
>  {
>      g_assert_not_reached();
>  }
> +
> +/* To be used for strict 32-bit targets. */
> +vaddr cpu_pointer_wrap_uint32(CPUState *cs, int idx, vaddr res, vaddr base)
> +{
> +    return (uint32_t)res;
> +}
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 250241541b..6995de6a12 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -250,6 +250,12 @@ static const TCGCPUOps avr_tcg_ops = {
>      .cpu_exec_reset = cpu_reset,
>      .tlb_fill = avr_cpu_tlb_fill,
>      .do_interrupt = avr_cpu_do_interrupt,
> +    /*
> +     * TODO: code and data wrapping are different, but for the most part
> +     * AVR only references bytes or aligned code fetches.  But we use
> +     * non-aligned MO_16 accesses for stack push/pop.
> +     */
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>  };
>  
>  static void avr_cpu_class_init(ObjectClass *oc, const void *data)
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index c5196a612e..6a09db3a6f 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -619,6 +619,7 @@ static const TCGCPUOps m68k_tcg_ops = {
>  
>  #ifndef CONFIG_USER_ONLY
>      .tlb_fill = m68k_cpu_tlb_fill,
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>      .cpu_exec_interrupt = m68k_cpu_exec_interrupt,
>      .cpu_exec_halt = m68k_cpu_has_work,
>      .cpu_exec_reset = cpu_reset,
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index d069e40e70..5eff1610c2 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -449,6 +449,7 @@ static const TCGCPUOps mb_tcg_ops = {
>  
>  #ifndef CONFIG_USER_ONLY
>      .tlb_fill = mb_cpu_tlb_fill,
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>      .cpu_exec_interrupt = mb_cpu_exec_interrupt,
>      .cpu_exec_halt = mb_cpu_has_work,
>      .cpu_exec_reset = cpu_reset,
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index 054ad33360..dfbb2df643 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -265,6 +265,7 @@ static const TCGCPUOps openrisc_tcg_ops = {
>  
>  #ifndef CONFIG_USER_ONLY
>      .tlb_fill = openrisc_cpu_tlb_fill,
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>      .cpu_exec_interrupt = openrisc_cpu_exec_interrupt,
>      .cpu_exec_halt = openrisc_cpu_has_work,
>      .cpu_exec_reset = cpu_reset,
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 36eba75545..c6dd5d6f83 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -225,6 +225,7 @@ static const TCGCPUOps rx_tcg_ops = {
>      .restore_state_to_opc = rx_restore_state_to_opc,
>      .mmu_index = rx_cpu_mmu_index,
>      .tlb_fill = rx_cpu_tlb_fill,
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>  
>      .cpu_exec_interrupt = rx_cpu_exec_interrupt,
>      .cpu_exec_halt = rx_cpu_has_work,
> diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
> index e56f90fde9..4f035b6f76 100644
> --- a/target/tricore/cpu.c
> +++ b/target/tricore/cpu.c
> @@ -190,6 +190,7 @@ static const TCGCPUOps tricore_tcg_ops = {
>      .restore_state_to_opc = tricore_restore_state_to_opc,
>      .mmu_index = tricore_cpu_mmu_index,
>      .tlb_fill = tricore_cpu_tlb_fill,
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>      .cpu_exec_interrupt = tricore_cpu_exec_interrupt,
>      .cpu_exec_halt = tricore_cpu_has_work,
>      .cpu_exec_reset = cpu_reset,
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 91b71b6caa..ea9b6df3aa 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -318,6 +318,7 @@ static const TCGCPUOps xtensa_tcg_ops = {
>  
>  #ifndef CONFIG_USER_ONLY
>      .tlb_fill = xtensa_cpu_tlb_fill,
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>      .cpu_exec_interrupt = xtensa_cpu_exec_interrupt,
>      .cpu_exec_halt = xtensa_cpu_has_work,
>      .cpu_exec_reset = cpu_reset,
> -- 
> 2.43.0
> 

Re: [PATCH 10/12] target/s390x: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Use the existing wrap_address function.
> 
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/cpu.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 9c1158ebcc..f05ce317da 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -347,6 +347,14 @@ static TCGTBCPUState s390x_get_tb_cpu_state(CPUState *cs)
>       };
>   }
>   
> +#ifndef CONFIG_USER_ONLY
> +static vaddr s390_pointer_wrap(CPUState *cs, int mmu_idx,
> +                               vaddr result, vaddr base)
> +{
> +    return wrap_address(cpu_env(cs), result);
> +}
> +#endif
> +
>   static const TCGCPUOps s390_tcg_ops = {
>       .mttcg_supported = true,
>       .precise_smc = true,
> @@ -367,6 +375,7 @@ static const TCGCPUOps s390_tcg_ops = {
>       .record_sigbus = s390_cpu_record_sigbus,
>   #else
>       .tlb_fill = s390_cpu_tlb_fill,
> +    .pointer_wrap = s390_pointer_wrap,

As future cleanup, we might now remove the wrap_address() calls
in target/s390x/tcg/crypto_helper.c, and target/s390x/tcg/vec_helper.c,
is that correct?

Also some uses in target/s390x/tcg/mem_helper.c.

Meanwhile for this patch:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 11/12] target/sparc: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Check address masking state for sparc64.
> 
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/cpu.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 2a3e408923..ed7701b02f 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -1002,6 +1002,18 @@ static const struct SysemuCPUOps sparc_sysemu_ops = {
>   #ifdef CONFIG_TCG
>   #include "accel/tcg/cpu-ops.h"
>   
> +#ifndef CONFIG_USER_ONLY
> +static vaddr sparc_pointer_wrap(CPUState *cs, int mmu_idx,
> +                                vaddr result, vaddr base)
> +{
> +#ifdef TARGET_SPARC64
> +    return cpu_env(cs)->pstate & PS_AM ? (uint32_t)result : result;
> +#else
> +    return (uint32_t)result;

Alternatively expose AM_CHECK()?

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

> +#endif
> +}
> +#endif

Re: [PATCH 07/12] target/mips: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Check 32 vs 64-bit addressing state.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/mips/cpu.c | 9 +++++++++
>   1 file changed, 9 insertions(+)

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

Re: [PATCH 10/12] target/s390x: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

On 5/5/25 07:41, Philippe Mathieu-Daudé wrote:
> On 4/5/25 22:57, Richard Henderson wrote:
>> Use the existing wrap_address function.
>>
>> Cc: qemu-s390x@nongnu.org
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/s390x/cpu.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 9c1158ebcc..f05ce317da 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -347,6 +347,14 @@ static TCGTBCPUState s390x_get_tb_cpu_state(CPUState *cs)
>>       };
>>   }
>> +#ifndef CONFIG_USER_ONLY
>> +static vaddr s390_pointer_wrap(CPUState *cs, int mmu_idx,
>> +                               vaddr result, vaddr base)
>> +{
>> +    return wrap_address(cpu_env(cs), result);
>> +}
>> +#endif
>> +
>>   static const TCGCPUOps s390_tcg_ops = {
>>       .mttcg_supported = true,
>>       .precise_smc = true,
>> @@ -367,6 +375,7 @@ static const TCGCPUOps s390_tcg_ops = {
>>       .record_sigbus = s390_cpu_record_sigbus,
>>   #else
>>       .tlb_fill = s390_cpu_tlb_fill,
>> +    .pointer_wrap = s390_pointer_wrap,
> 
> As future cleanup, we might now remove the wrap_address() calls
> in target/s390x/tcg/crypto_helper.c, and target/s390x/tcg/vec_helper.c,
> is that correct?
> 
> Also some uses in target/s390x/tcg/mem_helper.c.

No, not correct.

The new pointer_wrap hook is only used for unaligned accesses that cross page boundaries. 
It does not apply to a sequence of individual accesses like we have in the s390x helpers.


r~

Re: [PATCH 11/12] target/sparc: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

On 5/5/25 07:54, Philippe Mathieu-Daudé wrote:
> On 4/5/25 22:57, Richard Henderson wrote:
>> Check address masking state for sparc64.
>>
>> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/sparc/cpu.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
>> index 2a3e408923..ed7701b02f 100644
>> --- a/target/sparc/cpu.c
>> +++ b/target/sparc/cpu.c
>> @@ -1002,6 +1002,18 @@ static const struct SysemuCPUOps sparc_sysemu_ops = {
>>   #ifdef CONFIG_TCG
>>   #include "accel/tcg/cpu-ops.h"
>> +#ifndef CONFIG_USER_ONLY
>> +static vaddr sparc_pointer_wrap(CPUState *cs, int mmu_idx,
>> +                                vaddr result, vaddr base)
>> +{
>> +#ifdef TARGET_SPARC64
>> +    return cpu_env(cs)->pstate & PS_AM ? (uint32_t)result : result;
>> +#else
>> +    return (uint32_t)result;
> 
> Alternatively expose AM_CHECK()?

No, AM_CHECK uses DisasContext.


r~

Re: [PATCH 09/12] target/riscv: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Check 32 vs 64-bit and pointer masking state.
> 
> Cc: qemu-riscv@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 55e00972b7..267186e5e3 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -237,6 +237,31 @@ static void riscv_restore_state_to_opc(CPUState *cs,
>       env->excp_uw2 = data[2];
>   }
>   
> +#ifndef CONFIG_USER_ONLY
> +static vaddr riscv_pointer_wrap(CPUState *cs, int mmu_idx,
> +                                vaddr result, vaddr base)
> +{
> +    CPURISCVState *env = cpu_env(cs);
> +    uint32_t pm_len;
> +    bool pm_signext;
> +
> +    if (cpu_address_xl(env) == MXL_RV32) {
> +        return (uint32_t)result;
> +    }
> +
> +    pm_len = riscv_pm_get_pmlen(riscv_pm_get_pmm(env));
> +    if (pm_len == 0) {
> +        return result;
> +    }
> +
> +    pm_signext = riscv_cpu_virt_mem_enabled(env);
> +    if (pm_signext) {
> +        return sextract64(result, 0, 64 - pm_len);
> +    }
> +    return extract64(result, 0, 64 - pm_len);

Is this safe for MXL_RV128?

> +}
> +#endif
> +
>   const TCGCPUOps riscv_tcg_ops = {
>       .mttcg_supported = true,
>       .guest_default_memory_order = 0,
> @@ -250,6 +275,7 @@ const TCGCPUOps riscv_tcg_ops = {
>   
>   #ifndef CONFIG_USER_ONLY
>       .tlb_fill = riscv_cpu_tlb_fill,
> +    .pointer_wrap = riscv_pointer_wrap,
>       .cpu_exec_interrupt = riscv_cpu_exec_interrupt,
>       .cpu_exec_halt = riscv_cpu_has_work,
>       .cpu_exec_reset = cpu_reset,

Re: [PATCH 08/12] target/ppc: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Check 32 vs 64-bit state.
> 
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/ppc/cpu_init.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index cf88a18244..1f18967b62 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7386,6 +7386,12 @@ static void ppc_cpu_exec_exit(CPUState *cs)
>           cpu->vhyp_class->cpu_exec_exit(cpu->vhyp, cpu);
>       }
>   }
> +
> +static vaddr ppc_pointer_wrap(CPUState *cs, int mmu_idx,
> +                              vaddr result, vaddr base)
> +{
> +    return (cpu_env(cs)->hflags >> HFLAGS_64) & 1 ? result : (uint32_t)result;

Alternatively expose and use NARROW_MODE().

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

> +}
>   #endif /* CONFIG_TCG */
>   
>   #endif /* !CONFIG_USER_ONLY */
> @@ -7490,6 +7496,7 @@ static const TCGCPUOps ppc_tcg_ops = {
>     .record_sigsegv = ppc_cpu_record_sigsegv,
>   #else
>     .tlb_fill = ppc_cpu_tlb_fill,
> +  .pointer_wrap = ppc_pointer_wrap,
>     .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
>     .cpu_exec_halt = ppc_cpu_has_work,
>     .cpu_exec_reset = cpu_reset,

Re: [PATCH 05/12] target/i386: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Check 32 vs 64-bit state.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/i386/tcg/tcg-cpu.c | 7 +++++++
>   1 file changed, 7 insertions(+)

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

Re: [PATCH 09/12] target/riscv: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

On 5/5/25 09:47, Philippe Mathieu-Daudé wrote:
> On 4/5/25 22:57, Richard Henderson wrote:
>> Check 32 vs 64-bit and pointer masking state.
>>
>> Cc: qemu-riscv@nongnu.org
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>> index 55e00972b7..267186e5e3 100644
>> --- a/target/riscv/tcg/tcg-cpu.c
>> +++ b/target/riscv/tcg/tcg-cpu.c
>> @@ -237,6 +237,31 @@ static void riscv_restore_state_to_opc(CPUState *cs,
>>       env->excp_uw2 = data[2];
>>   }
>> +#ifndef CONFIG_USER_ONLY
>> +static vaddr riscv_pointer_wrap(CPUState *cs, int mmu_idx,
>> +                                vaddr result, vaddr base)
>> +{
>> +    CPURISCVState *env = cpu_env(cs);
>> +    uint32_t pm_len;
>> +    bool pm_signext;
>> +
>> +    if (cpu_address_xl(env) == MXL_RV32) {
>> +        return (uint32_t)result;
>> +    }
>> +
>> +    pm_len = riscv_pm_get_pmlen(riscv_pm_get_pmm(env));
>> +    if (pm_len == 0) {
>> +        return result;
>> +    }
>> +
>> +    pm_signext = riscv_cpu_virt_mem_enabled(env);
>> +    if (pm_signext) {
>> +        return sextract64(result, 0, 64 - pm_len);
>> +    }
>> +    return extract64(result, 0, 64 - pm_len);
> 
> Is this safe for MXL_RV128?

The RV128 implementation only uses 64-bit pointers, so, yes.


r~

Re: [PATCH 08/12] target/ppc: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

On 5/5/25 09:50, Philippe Mathieu-Daudé wrote:
> On 4/5/25 22:57, Richard Henderson wrote:
>> Check 32 vs 64-bit state.
>>
>> Cc: qemu-ppc@nongnu.org
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/ppc/cpu_init.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index cf88a18244..1f18967b62 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -7386,6 +7386,12 @@ static void ppc_cpu_exec_exit(CPUState *cs)
>>           cpu->vhyp_class->cpu_exec_exit(cpu->vhyp, cpu);
>>       }
>>   }
>> +
>> +static vaddr ppc_pointer_wrap(CPUState *cs, int mmu_idx,
>> +                              vaddr result, vaddr base)
>> +{
>> +    return (cpu_env(cs)->hflags >> HFLAGS_64) & 1 ? result : (uint32_t)result;
> 
> Alternatively expose and use NARROW_MODE().

Again, that's a DisasContext thing, only valid during translation.


r~

Re: [PATCH 00/12] accel/tcg: Fix cross-page pointer wrapping issue

[FOSS]

[-- Attachment #1: Type: text/plain, Size: 3599 bytes --]

Thank you for working on a fix for this! Should we include our functional test as a patch to test for this in the future or do anything else to help with this?

-Percival Engineering
________________________________
From: Richard Henderson <richard.henderson@linaro.org>
Sent: Sunday, May 4, 2025 8:57 PM
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Cc: FOSS <foss@percivaleng.com>
Subject: [PATCH 00/12] accel/tcg: Fix cross-page pointer wrapping issue

[You don't often get email from richard.henderson@linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

As detailed in

https://usg02.safelinks.protection.office365.us/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2F174595764300.3422.13156465553505851834-0%40git.sr.ht%2F&data=05%7C02%7Cfoss%40percivaleng.com%7Cbcd8ed34f3e342df86f008dd8b4e40b7%7C7e469936b9c44e65a905faf8e5ffac80%7C0%7C0%7C638819890414363571%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=i%2FTD9xTTxa4eRseMhadW%2FLw3hPMDp2sPDJF%2BzIzjfJw%3D&reserved=0<https://lore.kernel.org/qemu-devel/174595764300.3422.13156465553505851834-0@git.sr.ht/>

there's an issue with an unaligned access that falls off
the end of the last page.  To solve this, we need to know
about the state of the cpu, so add a new target hook.

There are arguments to the hook that are currently unused,
but would appear to come in handy for AArch64 v9.5 FEAT_CPA2,
which we do not yet implement.


r~


Richard Henderson (12):
  accel/tcg: Add TCGCPUOps.pointer_wrap
  target: Use cpu_pointer_wrap_notreached for strict align targets
  target: Use cpu_pointer_wrap_uint32 for 32-bit targets
  target/arm: Fill in TCGCPUOps.pointer_wrap
  target/i386: Fill in TCGCPUOps.pointer_wrap
  target/loongarch: Fill in TCGCPUOps.pointer_wrap
  target/mips: Fill in TCGCPUOps.pointer_wrap
  target/ppc: Fill in TCGCPUOps.pointer_wrap
  target/riscv: Fill in TCGCPUOps.pointer_wrap
  target/s390x: Fill in TCGCPUOps.pointer_wrap
  target/sparc: Fill in TCGCPUOps.pointer_wrap
  accel/tcg: Assert TCGCPUOps.pointer_wrap is set

 include/accel/tcg/cpu-ops.h | 13 +++++++++++++
 accel/tcg/cpu-exec.c        |  1 +
 accel/tcg/cputlb.c          | 22 ++++++++++++++++++++++
 target/alpha/cpu.c          |  1 +
 target/arm/cpu.c            | 24 ++++++++++++++++++++++++
 target/arm/tcg/cpu-v7m.c    |  1 +
 target/avr/cpu.c            |  6 ++++++
 target/hppa/cpu.c           |  1 +
 target/i386/tcg/tcg-cpu.c   |  7 +++++++
 target/loongarch/cpu.c      |  7 +++++++
 target/m68k/cpu.c           |  1 +
 target/microblaze/cpu.c     |  1 +
 target/mips/cpu.c           |  9 +++++++++
 target/openrisc/cpu.c       |  1 +
 target/ppc/cpu_init.c       |  7 +++++++
 target/riscv/tcg/tcg-cpu.c  | 26 ++++++++++++++++++++++++++
 target/rx/cpu.c             |  1 +
 target/s390x/cpu.c          |  9 +++++++++
 target/sh4/cpu.c            |  1 +
 target/sparc/cpu.c          | 13 +++++++++++++
 target/tricore/cpu.c        |  1 +
 target/xtensa/cpu.c         |  1 +
 22 files changed, 154 insertions(+)

--
2.43.0



This electronic message and any files transmitted with it contain confidential and proprietary information, and is intended for use only by the person(s) to whom it is addressed. Any use, distribution, copying or disclosure to any other person is strictly prohibited. If you have received this message in error, please notify the e-mail sender immediately, and delete the original message without making a copy.

[-- Attachment #2: Type: text/html, Size: 6175 bytes --]

Re: [PATCH 00/12] accel/tcg: Fix cross-page pointer wrapping issue

[Richard Henderson]

On 5/7/25 09:38, FOSS wrote:
> Thank you for working on a fix for this! Should we include our functional test as a patch 
> to test for this in the future or do anything else to help with this?

The test didn't work for me.  It's probably better to write a stand-alone test case.


r~

> 
> -Percival Engineering
> ------------------------------------------------------------------------------------------
> *From:* Richard Henderson <richard.henderson@linaro.org>
> *Sent:* Sunday, May 4, 2025 8:57 PM
> *To:* qemu-devel@nongnu.org <qemu-devel@nongnu.org>
> *Cc:* FOSS <foss@percivaleng.com>
> *Subject:* [PATCH 00/12] accel/tcg: Fix cross-page pointer wrapping issue
> [You don't often get email from richard.henderson@linaro.org. Learn why this is important 
> at https://aka.ms/LearnAboutSenderIdentification <https://aka.ms/ 
> LearnAboutSenderIdentification> ]
> 
> As detailed in
> 
> https://usg02.safelinks.protection.office365.us/?url=https%3A%2F%2Flore.kernel.org%2Fqemu- 
> devel%2F174595764300.3422.13156465553505851834-0%40git.sr.ht%2F&data=05%7C02%7Cfoss%40percivaleng.com%7Cbcd8ed34f3e342df86f008dd8b4e40b7%7C7e469936b9c44e65a905faf8e5ffac80%7C0%7C0%7C638819890414363571%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=i%2FTD9xTTxa4eRseMhadW%2FLw3hPMDp2sPDJF%2BzIzjfJw%3D&reserved=0 <https://lore.kernel.org/qemu-devel/174595764300.3422.13156465553505851834-0@git.sr.ht/>
> 
> there's an issue with an unaligned access that falls off
> the end of the last page.  To solve this, we need to know
> about the state of the cpu, so add a new target hook.
> 
> There are arguments to the hook that are currently unused,
> but would appear to come in handy for AArch64 v9.5 FEAT_CPA2,
> which we do not yet implement.
> 
> 
> r~
> 
> 
> Richard Henderson (12):
>    accel/tcg: Add TCGCPUOps.pointer_wrap
>    target: Use cpu_pointer_wrap_notreached for strict align targets
>    target: Use cpu_pointer_wrap_uint32 for 32-bit targets
>    target/arm: Fill in TCGCPUOps.pointer_wrap
>    target/i386: Fill in TCGCPUOps.pointer_wrap
>    target/loongarch: Fill in TCGCPUOps.pointer_wrap
>    target/mips: Fill in TCGCPUOps.pointer_wrap
>    target/ppc: Fill in TCGCPUOps.pointer_wrap
>    target/riscv: Fill in TCGCPUOps.pointer_wrap
>    target/s390x: Fill in TCGCPUOps.pointer_wrap
>    target/sparc: Fill in TCGCPUOps.pointer_wrap
>    accel/tcg: Assert TCGCPUOps.pointer_wrap is set
> 
>   include/accel/tcg/cpu-ops.h | 13 +++++++++++++
>   accel/tcg/cpu-exec.c        |  1 +
>   accel/tcg/cputlb.c          | 22 ++++++++++++++++++++++
>   target/alpha/cpu.c          |  1 +
>   target/arm/cpu.c            | 24 ++++++++++++++++++++++++
>   target/arm/tcg/cpu-v7m.c    |  1 +
>   target/avr/cpu.c            |  6 ++++++
>   target/hppa/cpu.c           |  1 +
>   target/i386/tcg/tcg-cpu.c   |  7 +++++++
>   target/loongarch/cpu.c      |  7 +++++++
>   target/m68k/cpu.c           |  1 +
>   target/microblaze/cpu.c     |  1 +
>   target/mips/cpu.c           |  9 +++++++++
>   target/openrisc/cpu.c       |  1 +
>   target/ppc/cpu_init.c       |  7 +++++++
>   target/riscv/tcg/tcg-cpu.c  | 26 ++++++++++++++++++++++++++
>   target/rx/cpu.c             |  1 +
>   target/s390x/cpu.c          |  9 +++++++++
>   target/sh4/cpu.c            |  1 +
>   target/sparc/cpu.c          | 13 +++++++++++++
>   target/tricore/cpu.c        |  1 +
>   target/xtensa/cpu.c         |  1 +
>   22 files changed, 154 insertions(+)
> 
> --
> 2.43.0
> 
> This electronic message and any files transmitted with it contain confidential and 
> proprietary information, and is intended for use only by the person(s) to whom it is 
> addressed. Any use, distribution, copying or disclosure to any other person is strictly 
> prohibited. If you have received this message in error, please notify the e-mail sender 
> immediately, and delete the original message without making a copy.
> 

Re: [PATCH 03/12] target: Use cpu_pointer_wrap_uint32 for 32-bit targets

[Bastian Koppelmann]

On Sun, May 04, 2025 at 01:57:04PM -0700, Richard Henderson wrote:
> M68K, MicroBlaze, OpenRISC, RX, TriCore and Xtensa are
> all 32-bit targets.  AVR is more complicated, but using
> a 32-bit wrap preserves current behaviour.
> 
> Cc: Michael Rolnik <mrolnik@gmail.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/accel/tcg/cpu-ops.h | 1 +
>  accel/tcg/cputlb.c          | 6 ++++++
>  target/avr/cpu.c            | 6 ++++++
>  target/m68k/cpu.c           | 1 +
>  target/microblaze/cpu.c     | 1 +
>  target/openrisc/cpu.c       | 1 +
>  target/rx/cpu.c             | 1 +
>  target/tricore/cpu.c        | 1 +
>  target/xtensa/cpu.c         | 1 +
>  9 files changed, 19 insertions(+)

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

For the TriCore part:
Tested-by Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian

Re: [PATCH 09/12] target/riscv: Fill in TCGCPUOps.pointer_wrap

[Alistair Francis]

On Mon, May 5, 2025 at 6:59 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Check 32 vs 64-bit and pointer masking state.
>
> Cc: qemu-riscv@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 55e00972b7..267186e5e3 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -237,6 +237,31 @@ static void riscv_restore_state_to_opc(CPUState *cs,
>      env->excp_uw2 = data[2];
>  }
>
> +#ifndef CONFIG_USER_ONLY
> +static vaddr riscv_pointer_wrap(CPUState *cs, int mmu_idx,
> +                                vaddr result, vaddr base)
> +{
> +    CPURISCVState *env = cpu_env(cs);
> +    uint32_t pm_len;
> +    bool pm_signext;
> +
> +    if (cpu_address_xl(env) == MXL_RV32) {
> +        return (uint32_t)result;
> +    }
> +
> +    pm_len = riscv_pm_get_pmlen(riscv_pm_get_pmm(env));
> +    if (pm_len == 0) {
> +        return result;
> +    }
> +
> +    pm_signext = riscv_cpu_virt_mem_enabled(env);
> +    if (pm_signext) {
> +        return sextract64(result, 0, 64 - pm_len);
> +    }
> +    return extract64(result, 0, 64 - pm_len);
> +}
> +#endif
> +
>  const TCGCPUOps riscv_tcg_ops = {
>      .mttcg_supported = true,
>      .guest_default_memory_order = 0,
> @@ -250,6 +275,7 @@ const TCGCPUOps riscv_tcg_ops = {
>
>  #ifndef CONFIG_USER_ONLY
>      .tlb_fill = riscv_cpu_tlb_fill,
> +    .pointer_wrap = riscv_pointer_wrap,
>      .cpu_exec_interrupt = riscv_cpu_exec_interrupt,
>      .cpu_exec_halt = riscv_cpu_has_work,
>      .cpu_exec_reset = cpu_reset,
> --
> 2.43.0
>
>

Re: [PATCH 09/12] target/riscv: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 5/5/25 20:59, Richard Henderson wrote:
> On 5/5/25 09:47, Philippe Mathieu-Daudé wrote:
>> On 4/5/25 22:57, Richard Henderson wrote:
>>> Check 32 vs 64-bit and pointer masking state.
>>>
>>> Cc: qemu-riscv@nongnu.org
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>>
>>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>>> index 55e00972b7..267186e5e3 100644
>>> --- a/target/riscv/tcg/tcg-cpu.c
>>> +++ b/target/riscv/tcg/tcg-cpu.c
>>> @@ -237,6 +237,31 @@ static void riscv_restore_state_to_opc(CPUState 
>>> *cs,
>>>       env->excp_uw2 = data[2];
>>>   }
>>> +#ifndef CONFIG_USER_ONLY
>>> +static vaddr riscv_pointer_wrap(CPUState *cs, int mmu_idx,
>>> +                                vaddr result, vaddr base)
>>> +{
>>> +    CPURISCVState *env = cpu_env(cs);
>>> +    uint32_t pm_len;
>>> +    bool pm_signext;
>>> +
>>> +    if (cpu_address_xl(env) == MXL_RV32) {
>>> +        return (uint32_t)result;
>>> +    }
>>> +
>>> +    pm_len = riscv_pm_get_pmlen(riscv_pm_get_pmm(env));
>>> +    if (pm_len == 0) {
>>> +        return result;
>>> +    }
>>> +
>>> +    pm_signext = riscv_cpu_virt_mem_enabled(env);
>>> +    if (pm_signext) {
>>> +        return sextract64(result, 0, 64 - pm_len);
>>> +    }
>>> +    return extract64(result, 0, 64 - pm_len);
>>
>> Is this safe for MXL_RV128?
> 
> The RV128 implementation only uses 64-bit pointers, so, yes.

Thanks.

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

Re: [PATCH 10/12] target/s390x: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 5/5/25 18:16, Richard Henderson wrote:
> On 5/5/25 07:41, Philippe Mathieu-Daudé wrote:
>> On 4/5/25 22:57, Richard Henderson wrote:
>>> Use the existing wrap_address function.
>>>
>>> Cc: qemu-s390x@nongnu.org
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   target/s390x/cpu.c | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>>> index 9c1158ebcc..f05ce317da 100644
>>> --- a/target/s390x/cpu.c
>>> +++ b/target/s390x/cpu.c
>>> @@ -347,6 +347,14 @@ static TCGTBCPUState 
>>> s390x_get_tb_cpu_state(CPUState *cs)
>>>       };
>>>   }
>>> +#ifndef CONFIG_USER_ONLY
>>> +static vaddr s390_pointer_wrap(CPUState *cs, int mmu_idx,
>>> +                               vaddr result, vaddr base)
>>> +{
>>> +    return wrap_address(cpu_env(cs), result);
>>> +}
>>> +#endif
>>> +
>>>   static const TCGCPUOps s390_tcg_ops = {
>>>       .mttcg_supported = true,
>>>       .precise_smc = true,
>>> @@ -367,6 +375,7 @@ static const TCGCPUOps s390_tcg_ops = {
>>>       .record_sigbus = s390_cpu_record_sigbus,
>>>   #else
>>>       .tlb_fill = s390_cpu_tlb_fill,
>>> +    .pointer_wrap = s390_pointer_wrap,
>>
>> As future cleanup, we might now remove the wrap_address() calls
>> in target/s390x/tcg/crypto_helper.c, and target/s390x/tcg/vec_helper.c,
>> is that correct?
>>
>> Also some uses in target/s390x/tcg/mem_helper.c.
> 
> No, not correct.
> 
> The new pointer_wrap hook is only used for unaligned accesses that cross 
> page boundaries. It does not apply to a sequence of individual accesses 
> like we have in the s390x helpers.

Ah, now I understand, thanks!

Re: [PATCH 06/12] target/loongarch: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

On 4/5/25 22:57, Richard Henderson wrote:
> Check va32 state.
> 
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: Bibo Mao <maobibo@loongson.cn>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/loongarch/cpu.c | 7 +++++++
>   1 file changed, 7 insertions(+)

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

Re: [PATCH 04/12] target/arm: Fill in TCGCPUOps.pointer_wrap

[Philippe Mathieu-Daudé]

+Gustavo

On 4/5/25 22:57, Richard Henderson wrote:
> For a-profile, check A32 vs A64 state.
> For m-profile, use cpu_pointer_wrap_uint32.
> 
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/cpu.c         | 24 ++++++++++++++++++++++++
>   target/arm/tcg/cpu-v7m.c |  1 +
>   2 files changed, 25 insertions(+)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 45cb6fd7ee..18edcf49c6 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2710,6 +2710,29 @@ static const struct SysemuCPUOps arm_sysemu_ops = {
>   #endif
>   
>   #ifdef CONFIG_TCG
> +#ifndef CONFIG_USER_ONLY
> +static vaddr aprofile_pointer_wrap(CPUState *cs, int mmu_idx,
> +                                   vaddr result, vaddr base)
> +{
> +    /*
> +     * The Stage2 and Phys indexes are only used for ptw on arm32,
> +     * and all pte's are aligned, so we never produce a wrap for these.
> +     * Double check that we're not truncating a 40-bit physical address.
> +     */
> +    assert((unsigned)mmu_idx < (ARMMMUIdx_Stage2_S & ARM_MMU_IDX_COREIDX_MASK));
> +
> +    if (!is_a64(cpu_env(cs))) {
> +        return (uint32_t)result;
> +    }
> +
> +    /*
> +     * TODO: For FEAT_CPA2, decide how to we want to resolve
> +     * Unpredictable_CPACHECK in AddressIncrement.
> +     */
> +    return result;
> +}
> +#endif /* !CONFIG_USER_ONLY */
> +
>   static const TCGCPUOps arm_tcg_ops = {
>       .mttcg_supported = true,
>       /* ARM processors have a weak memory model */
> @@ -2729,6 +2752,7 @@ static const TCGCPUOps arm_tcg_ops = {
>       .untagged_addr = aarch64_untagged_addr,
>   #else
>       .tlb_fill_align = arm_cpu_tlb_fill_align,
> +    .pointer_wrap = aprofile_pointer_wrap,

IIUC this is also used by non A-profiles (R-profiles and
non Cortex cores).

Patch LGTM but I'd rather someone else to look at it.

>       .cpu_exec_interrupt = arm_cpu_exec_interrupt,
>       .cpu_exec_halt = arm_cpu_exec_halt,
>       .cpu_exec_reset = cpu_reset,
> diff --git a/target/arm/tcg/cpu-v7m.c b/target/arm/tcg/cpu-v7m.c
> index 95b23d9b55..8e1a083b91 100644
> --- a/target/arm/tcg/cpu-v7m.c
> +++ b/target/arm/tcg/cpu-v7m.c
> @@ -249,6 +249,7 @@ static const TCGCPUOps arm_v7m_tcg_ops = {
>       .record_sigbus = arm_cpu_record_sigbus,
>   #else
>       .tlb_fill_align = arm_cpu_tlb_fill_align,
> +    .pointer_wrap = cpu_pointer_wrap_uint32,
>       .cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt,
>       .cpu_exec_halt = arm_cpu_exec_halt,
>       .cpu_exec_reset = cpu_reset,

Re: [PATCH 06/12] target/loongarch: Fill in TCGCPUOps.pointer_wrap

[Bibo Mao]

On 2025/5/5 上午4:57, Richard Henderson wrote:
> Check va32 state.
> 
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: Bibo Mao <maobibo@loongson.cn>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/loongarch/cpu.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index f7535d1be7..abad84c054 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -334,6 +334,12 @@ static bool loongarch_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>       }
>       return false;
>   }
> +
> +static vaddr loongarch_pointer_wrap(CPUState *cs, int mmu_idx,
> +                                    vaddr result, vaddr base)
> +{
> +    return is_va32(cpu_env(cs)) ? (uint32_t)result : result;
> +}
>   #endif
>   
>   static TCGTBCPUState loongarch_get_tb_cpu_state(CPUState *cs)
> @@ -889,6 +895,7 @@ static const TCGCPUOps loongarch_tcg_ops = {
>   
>   #ifndef CONFIG_USER_ONLY
>       .tlb_fill = loongarch_cpu_tlb_fill,
> +    .pointer_wrap = loongarch_pointer_wrap,
>       .cpu_exec_interrupt = loongarch_cpu_exec_interrupt,
>       .cpu_exec_halt = loongarch_cpu_has_work,
>       .cpu_exec_reset = cpu_reset,
> 
Reviewed-by: Bibo Mao <maobibo@loongson.cn>

Re: [PATCH 06/12] target/loongarch: Fill in TCGCPUOps.pointer_wrap

[gaosong]

在 2025/5/5 上午4:57, Richard Henderson 写道:
> Check va32 state.
>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: Bibo Mao <maobibo@loongson.cn>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/loongarch/cpu.c | 7 +++++++
>   1 file changed, 7 insertions(+)
Reviewed-by: Song Gao <gaosong@loongson.cn>

Thanks.
Song Gao

Re: [PATCH 04/12] target/arm: Fill in TCGCPUOps.pointer_wrap

[Richard Henderson]

On 5/26/25 19:21, Philippe Mathieu-Daudé wrote:
> +Gustavo
> 
> On 4/5/25 22:57, Richard Henderson wrote:
>> For a-profile, check A32 vs A64 state.
>> For m-profile, use cpu_pointer_wrap_uint32.
>>
>> Cc: qemu-arm@nongnu.org
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/cpu.c         | 24 ++++++++++++++++++++++++
>>   target/arm/tcg/cpu-v7m.c |  1 +
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 45cb6fd7ee..18edcf49c6 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -2710,6 +2710,29 @@ static const struct SysemuCPUOps arm_sysemu_ops = {
>>   #endif
>>   #ifdef CONFIG_TCG
>> +#ifndef CONFIG_USER_ONLY
>> +static vaddr aprofile_pointer_wrap(CPUState *cs, int mmu_idx,
>> +                                   vaddr result, vaddr base)
>> +{
>> +    /*
>> +     * The Stage2 and Phys indexes are only used for ptw on arm32,
>> +     * and all pte's are aligned, so we never produce a wrap for these.
>> +     * Double check that we're not truncating a 40-bit physical address.
>> +     */
>> +    assert((unsigned)mmu_idx < (ARMMMUIdx_Stage2_S & ARM_MMU_IDX_COREIDX_MASK));
>> +
>> +    if (!is_a64(cpu_env(cs))) {
>> +        return (uint32_t)result;
>> +    }
>> +
>> +    /*
>> +     * TODO: For FEAT_CPA2, decide how to we want to resolve
>> +     * Unpredictable_CPACHECK in AddressIncrement.
>> +     */
>> +    return result;
>> +}
>> +#endif /* !CONFIG_USER_ONLY */
>> +
>>   static const TCGCPUOps arm_tcg_ops = {
>>       .mttcg_supported = true,
>>       /* ARM processors have a weak memory model */
>> @@ -2729,6 +2752,7 @@ static const TCGCPUOps arm_tcg_ops = {
>>       .untagged_addr = aarch64_untagged_addr,
>>   #else
>>       .tlb_fill_align = arm_cpu_tlb_fill_align,
>> +    .pointer_wrap = aprofile_pointer_wrap,
> 
> IIUC this is also used by non A-profiles (R-profiles and
> non Cortex cores).

Yes, r-profile is mostly a-profile.  Those non-cortex cores are also a-profile: armv[456].

The point is the separation between m-profile and not. In particular, the mmu indexes are 
different between A and M (see ARM_MMU_IDX_TYPE_MASK). The assert would not be valid for 
m-profile. We can avoid a check vs ARM_FEATURE_M by only using this function for 
not-m-profile.


r~