Re: [PATCH v8 01/18] target/s390x: Fix wrong address handling in address loops

[Ilya Leoshkevich <iii@linux.ibm.com>]

On 6/24/26 10:09, Harald Freudenberger wrote:
> With the introduction of the address wrapping function
> wrap_address() the result can't be used to walk the
> source address any more. So introduce a new local variable
> to hold the wrapped address to avoid mixing source and
> wrapped address value.
> 
> Fixes: fcc2699d41 ("target/s390x: Have MSA helper pass a mmu_idx argument")
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> ---
>   target/s390x/tcg/crypto_helper.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)

If I take as an example AMODE 24 and look at the third loop iteration,
with the current code in master I would get:

     ((((addr & 0xffffff) + 8) & 0xffffff) + 8) & 0xffffff

and with your patch it would be:

     (addr + 8 + 8) & 0xffffff

which is undeniably more elegant, but otherwise looks equivalent to me.

What is the functional issue here?

> diff --git a/target/s390x/tcg/crypto_helper.c b/target/s390x/tcg/crypto_helper.c
> index ae392bce0e..29ad2aff43 100644
> --- a/target/s390x/tcg/crypto_helper.c
> +++ b/target/s390x/tcg/crypto_helper.c
> @@ -126,8 +126,8 @@ static void sha512_read_icv(CPUS390XState *env, const int mmu_idx,
>       const MemOpIdx oi = make_memop_idx(MO_BE | MO_64 | MO_UNALN, mmu_idx);
>   
>       for (int i = 0; i < 8; i++, addr += 8) {
> -        addr = wrap_address(env, addr);
> -        a[i] = cpu_ldq_mmu(env, addr, oi, ra);
> +        uint64_t _addr = wrap_address(env, addr);
> +        a[i] = cpu_ldq_mmu(env, _addr, oi, ra);
>       }
>   }

[...]