1. Patchew
  2. QEMU
  3. [PATCH v3 00/12] target/arm: Fix M-profile helper loads/stores alignment checks
  4. View patch

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

William Kosasih posted 12 patches 4 months, 2 weeks ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v3 04/12] target/arm: Fix VLDR helper load alignment checks
Posted by William Kosasih 4 months, 2 weeks ago 
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 <kosasihwilliam4@gmail.com>
---
 target/arm/tcg/mve_helper.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

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)
-- 
2.48.1
Re: [PATCH v3 04/12] target/arm: Fix VLDR helper load alignment checks
Posted by Richard Henderson 4 months, 2 weeks ago 
On 7/2/25 05:19, William Kosasih wrote:
> +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(vldrh_sw, MO_TEUW, 2, int16_t, ldw, 4, int32_t)

Use MO_SB, MO_TESW here.

It won't matter for normal operation, but the sign of the operation is exposed to plugins.


r~
Re: [PATCH v3 04/12] target/arm: Fix VLDR helper load alignment checks
Posted by William Kosasih 4 months, 2 weeks ago 
>
> Much better, thanks.

Cheers! :-)

Use MO_SB, MO_TESW here.

It won't matter for normal operation, but the sign of the operation is
> exposed to plugins.


Ah okay. I'll update accordingly and copy over your Reviewed-by line.
Thanks!

On Thu, Jul 3, 2025 at 12:14 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 7/2/25 05:19, William Kosasih wrote:
> > +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(vldrh_sw, MO_TEUW, 2, int16_t, ldw, 4, int32_t)
>
> Use MO_SB, MO_TESW here.
>
> It won't matter for normal operation, but the sign of the operation is
> exposed to plugins.
>
>
> r~
>
Re: [PATCH v3 04/12] target/arm: Fix VLDR helper load alignment checks
Posted by Richard Henderson 4 months, 2 weeks ago 
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 <kosasihwilliam4@gmail.com>
> ---
>   target/arm/tcg/mve_helper.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)

Much better, thanks.

Reviewed-by: Richard Henderson <richard.henderson@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)

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.