Re: [PATCH v14 07/19] target/s390x: Support AES ECB for cpacf km instruction

On 8/17/26 16:23, Harald Freudenberger wrote:
> On 2026-08-17 11:17, Ilya Leoshkevich wrote:
>> On 8/6/26 17:12, Harald Freudenberger wrote:
>>> Support the subfunctions CPACF_KM_AES_128, CPACF_KM_AES_192
>>> and CPACF_KM_AES_256 for the cpacf km instruction.
>>>
>>> Tested-by: Holger Dengler <[email protected]>
>>> Reviewed-by: Finn Callies <[email protected]>
>>> Signed-off-by: Harald Freudenberger <[email protected]>
>>> ---
>>>   target/s390x/gen-features.c      |   3 +
>>>   target/s390x/tcg/cpacf.h         |   6 ++
>>>   target/s390x/tcg/cpacf_aes.c     | 107 +++++++++++++++++++++++++++++++
>>>   target/s390x/tcg/crypto_helper.c |  24 +++++++
>>>   target/s390x/tcg/meson.build     |   1 +
>>>   5 files changed, 141 insertions(+)
>>>   create mode 100644 target/s390x/tcg/cpacf_aes.c
>>
>> [...]
>>
>>> +int cpacf_aes_ecb(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
>>> +                  uint64_t param_addr, uint64_t *dst_ptr_reg,
>>> +                  uint64_t *src_ptr_reg, uint64_t *src_len_reg,
>>> +                  uint32_t type, uint8_t fc, uint8_t mod)
>>> +{
>>> +    enum { MAX_BLOCKS_PER_RUN = 8192 / AES_BLOCK_SIZE };
>>> +    uint8_t in[AES_BLOCK_SIZE], out[AES_BLOCK_SIZE];
>>> +    uint64_t len = *src_len_reg, done = 0;
>>> +    int i, keysize, addr_reg_size = 64;
>>> +    uint8_t key[32];
>>> +    AES_KEY exkey;
>>> +
>>> +    g_assert(type == S390_FEAT_TYPE_KM);
>>> +    switch (fc) {
>>> +    case CPACF_KM_AES_128:
>>> +        keysize = 16;
>>> +        break;
>>> +    case CPACF_KM_AES_192:
>>> +        keysize = 24;
>>> +        break;
>>> +    case CPACF_KM_AES_256:
>>> +        keysize = 32;
>>> +        break;
>>> +    default:
>>> +        g_assert_not_reached();
>>> +    }
>>> +
>>> +    if (!(env->psw.mask & PSW_MASK_64)) {
>>> +        len = (uint32_t)len;
>>> +        addr_reg_size = (env->psw.mask & PSW_MASK_32) ? 32 : 24;
>>> +    }
>>
>> Just noticed something, here and in all other patches. POp says:
>>
>>
>> In the 24-bit addressing mode, the contents of bit
>> positions 40-63 of general registers R1 and R2 consti-
>> tute the addresses of the first and second operands,
>> respectively, and the contents of bit positions 0-39
>> are ignored; bits 40-63 of the updated addresses
>> replace the corresponding bits in general registers R1
>> and R2, carries out of bit position 40 of the updated
>> address are ignored, and the contents of bit positions
>> 32-39 of general registers R1 and R 2 are set to zeros.
>>
>>
>> Seems like we are not zeroing out bits 32-39?
>>
> 
> Do we support 24 bit mode? Isn't that ditched some years ago?
> At least we definitely do not support s390 kernel in 24 bit mode.
> Maybe userspace is valid.

No, userspace doesn't support it either.
So neither qemu-user nor qemu-system + Linux will need it.
Some weird non-Linux setup might, I'm not sure if we want to go out of
our way to support them. I think we should either drop it or
implement it correctly.

[...]