HELPER_LD_ATOMIC() now has a MemOpIdx, from which we
can extract the MemOp size. Use it to check the address
alignment.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/tcg/ldst_helper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/mips/tcg/ldst_helper.c b/target/mips/tcg/ldst_helper.c
index 013cd825dac..bef384575c2 100644
--- a/target/mips/tcg/ldst_helper.c
+++ b/target/mips/tcg/ldst_helper.c
@@ -29,12 +29,14 @@
 
 #ifndef CONFIG_USER_ONLY
 
-#define HELPER_LD_ATOMIC(name, almask, cpu_load)                              \
+#define HELPER_LD_ATOMIC(name, cpu_load)                                      \
 target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int memop_idx)\
 {                                                                             \
     MemOpIdx oi = memop_idx;                                                  \
     unsigned mem_idx = get_mmuidx(oi);                                        \
-    if (arg & almask) {                                                       \
+    MemOp op = get_memop(oi);                                                 \
+    unsigned size = memop_size(op);                                           \
+    if (arg & (size - 1)) {                                                   \
         if (!(env->hflags & MIPS_HFLAG_DM)) {                                 \
             env->CP0_BadVAddr = arg;                                          \
         }                                                                     \
@@ -52,14 +54,14 @@ static target_ulong loads4(CPUMIPSState *env, target_ulong arg,
 {
     return (target_long)(int32_t)cpu_ldl_mmuidx_ra(env, arg, mem_idx, ra);
 }
-HELPER_LD_ATOMIC(ll, 0x3, loads4)
+HELPER_LD_ATOMIC(ll, loads4)
 #ifdef TARGET_MIPS64
 static target_ulong loadu8(CPUMIPSState *env, target_ulong arg,
                            unsigned mem_idx, uintptr_t ra)
 {
     return (target_ulong)cpu_ldq_mmuidx_ra(env, arg, mem_idx, ra);
 }
-HELPER_LD_ATOMIC(lld, 0x7, loadu8)
+HELPER_LD_ATOMIC(lld, loadu8)
 #endif
 #undef HELPER_LD_ATOMIC
 
-- 
2.51.0

On 11/26/25 14:21, Philippe Mathieu-Daudé wrote:
> HELPER_LD_ATOMIC() now has a MemOpIdx, from which we
> can extract the MemOp size. Use it to check the address
> alignment.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/tcg/ldst_helper.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)

True, but adding MO_ALIGN to the MemOpIdx (and thus to the MemOp) might be more useful, 
allowing you to remove this explicit check.


r~

> 
> diff --git a/target/mips/tcg/ldst_helper.c b/target/mips/tcg/ldst_helper.c
> index 013cd825dac..bef384575c2 100644
> --- a/target/mips/tcg/ldst_helper.c
> +++ b/target/mips/tcg/ldst_helper.c
> @@ -29,12 +29,14 @@
>   
>   #ifndef CONFIG_USER_ONLY
>   
> -#define HELPER_LD_ATOMIC(name, almask, cpu_load)                              \
> +#define HELPER_LD_ATOMIC(name, cpu_load)                                      \
>   target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int memop_idx)\
>   {                                                                             \
>       MemOpIdx oi = memop_idx;                                                  \
>       unsigned mem_idx = get_mmuidx(oi);                                        \
> -    if (arg & almask) {                                                       \
> +    MemOp op = get_memop(oi);                                                 \
> +    unsigned size = memop_size(op);                                           \
> +    if (arg & (size - 1)) {                                                   \
>           if (!(env->hflags & MIPS_HFLAG_DM)) {                                 \
>               env->CP0_BadVAddr = arg;                                          \
>           }                                                                     \
> @@ -52,14 +54,14 @@ static target_ulong loads4(CPUMIPSState *env, target_ulong arg,
>   {
>       return (target_long)(int32_t)cpu_ldl_mmuidx_ra(env, arg, mem_idx, ra);
>   }
> -HELPER_LD_ATOMIC(ll, 0x3, loads4)
> +HELPER_LD_ATOMIC(ll, loads4)
>   #ifdef TARGET_MIPS64
>   static target_ulong loadu8(CPUMIPSState *env, target_ulong arg,
>                              unsigned mem_idx, uintptr_t ra)
>   {
>       return (target_ulong)cpu_ldq_mmuidx_ra(env, arg, mem_idx, ra);
>   }
> -HELPER_LD_ATOMIC(lld, 0x7, loadu8)
> +HELPER_LD_ATOMIC(lld, loadu8)
>   #endif
>   #undef HELPER_LD_ATOMIC
>   

On 12/12/25 10:35, Richard Henderson wrote:
> On 11/26/25 14:21, Philippe Mathieu-Daudé wrote:
>> HELPER_LD_ATOMIC() now has a MemOpIdx, from which we
>> can extract the MemOp size. Use it to check the address
>> alignment.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/mips/tcg/ldst_helper.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> True, but adding MO_ALIGN to the MemOpIdx (and thus to the MemOp) might be more useful, 
> allowing you to remove this explicit check.

Follow-up: You'd also reorg the function such that

      env->llval = cpu_load(env, arg, oi, GETPC());                             \
      env->CP0_LLAddr = cpu_mips_translate_address(env, arg, MMU_DATA_LOAD,     \
                                                   GETPC());                    \
      env->lladdr = arg;                                                        \
      return env->llval;                                                        \

so that lladdr and CP0_LLAddr do not get assigned when there is an alignment fault.

Also, the cpu_load will have either faulted, or pulled in the translation to CPUTLB.  You 
don't need the full PTW of cpu_mips_translate_address.  Using probe_access_full will 
quickly grab the CPUTLBEntryFull, from which phys_addr is your required value.


r~