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

[Harald Freudenberger <freude@linux.ibm.com>]

On 2026-06-24 12:05, Philippe Mathieu-Daudé wrote:
> Hi Harald,
> 
> On 24/6/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")
> 
> This commit merely added an argument. I suppose you meant
> 9f17bfdab42 ("target/s390x: support SHA-512 extensions").

Yep, you are right. This addr overwrite was introduced with the
original code - thus the commit you mentioned. I'll correct it.

> 
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>> ---
>>   target/s390x/tcg/crypto_helper.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>> 
>> 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);
> 
> s/_addr/wa/ or even directly inline?
> 
>            a[i] = cpu_ldq_mmu(env, wrap_address(env, addr), oi, ra);
> 
>> +        a[i] = cpu_ldq_mmu(env, _addr, oi, ra);
>>       }
>>   }

Yes, let's make all these occurrences directly inline.