Re: [PATCH v3 04/12] target/arm: Fix VLDR helper load alignment checks

[Richard Henderson]

On 7/2/25 05:19, William Kosasih wrote:

This patch adds alignment checks in the load operations in the VLDR
instruction.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1154
Signed-off-by: William Kosasih <kosasihwilli...@gmail.com>
---
  target/arm/tcg/mve_helper.c | 24 +++++++++++++-----------
  1 file changed, 13 insertions(+), 11 deletions(-)

Much better, thanks.

Reviewed-by: Richard Henderson <richard.hender...@linaro.org>

r~

diff --git a/target/arm/tcg/mve_helper.c b/target/arm/tcg/mve_helper.c
index 506d1c3475..1db626bb26 100644
--- a/target/arm/tcg/mve_helper.c
+++ b/target/arm/tcg/mve_helper.c
@@ -148,13 +148,15 @@ static void mve_advance_vpt(CPUARMState *env)
  }

/* For loads, predicated lanes are zeroed instead of keeping their old values */

-#define DO_VLDR(OP, MSIZE, LDTYPE, ESIZE, TYPE)                         \
+#define DO_VLDR(OP, MFLAG, MSIZE, MTYPE, LDTYPE, ESIZE, TYPE)           \
      void HELPER(mve_##OP)(CPUARMState *env, void *vd, uint32_t addr)    \
      {                                                                   \
          TYPE *d = vd;                                                   \
          uint16_t mask = mve_element_mask(env);                          \
          uint16_t eci_mask = mve_eci_mask(env);                          \
          unsigned b, e;                                                  \
+        int mmu_idx = arm_to_core_mmu_idx(arm_mmu_idx(env));            \
+        MemOpIdx oi = make_memop_idx(MFLAG | MO_ALIGN, mmu_idx);        \
          /*                                                              \
           * R_SXTM allows the dest reg to become UNKNOWN for abandoned   \
           * beats so we don't care if we update part of the dest and     \
@@ -163,7 +165,7 @@ static void mve_advance_vpt(CPUARMState *env)
          for (b = 0, e = 0; b < 16; b += ESIZE, e++) {                   \
              if (eci_mask & (1 << b)) {                                  \
                  d[H##ESIZE(e)] = (mask & (1 << b)) ?                    \
-                    cpu_##LDTYPE##_data_ra(env, addr, GETPC()) : 0;     \
+                    (MTYPE)cpu_##LDTYPE##_mmu(env, addr, oi, GETPC()) : 0;\
              }                                                           \
              addr += MSIZE;                                              \
          }                                                               \
@@ -185,20 +187,20 @@ static void mve_advance_vpt(CPUARMState *env)
          mve_advance_vpt(env);                                           \
      }

-DO_VLDR(vldrb, 1, ldub, 1, uint8_t)

-DO_VLDR(vldrh, 2, lduw, 2, uint16_t)
-DO_VLDR(vldrw, 4, ldl, 4, uint32_t)
+DO_VLDR(vldrb, MO_UB, 1, uint8_t, ldb, 1, uint8_t)
+DO_VLDR(vldrh, MO_TEUW, 2, uint16_t, ldw, 2, uint16_t)
+DO_VLDR(vldrw, MO_TEUL, 4, uint32_t, ldl, 4, uint32_t)

DO_VSTR(vstrb, 1, stb, 1, uint8_t)

  DO_VSTR(vstrh, 2, stw, 2, uint16_t)
  DO_VSTR(vstrw, 4, stl, 4, uint32_t)

-DO_VLDR(vldrb_sh, 1, ldsb, 2, int16_t)

-DO_VLDR(vldrb_sw, 1, ldsb, 4, int32_t)
-DO_VLDR(vldrb_uh, 1, ldub, 2, uint16_t)
-DO_VLDR(vldrb_uw, 1, ldub, 4, uint32_t)
-DO_VLDR(vldrh_sw, 2, ldsw, 4, int32_t)
-DO_VLDR(vldrh_uw, 2, lduw, 4, uint32_t)
+DO_VLDR(vldrb_sh, MO_UB, 1, int8_t, ldb, 2, int16_t)
+DO_VLDR(vldrb_sw, MO_UB, 1, int8_t, ldb, 4, int32_t)
+DO_VLDR(vldrb_uh, MO_UB, 1, uint8_t, ldb, 2, uint16_t)
+DO_VLDR(vldrb_uw, MO_UB, 1, uint8_t, ldb, 4, uint32_t)
+DO_VLDR(vldrh_sw, MO_TEUW, 2, int16_t, ldw, 4, int32_t)
+DO_VLDR(vldrh_uw, MO_TEUW, 2, uint16_t, ldw, 4, uint32_t)

DO_VSTR(vstrb_h, 1, stb, 2, int16_t)

  DO_VSTR(vstrb_w, 1, stb, 4, int32_t)