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

On 2026-08-17 17:00, Ilya Leoshkevich wrote:
> 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.
> 
> [...]

It it ok then to assume 32 bit mode if not 64 bit? Like this snippet:

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)
{
     ...
     int i, keysize, addr_reg_size = 64;
     ...

     ...

     if (!(env->psw.mask & PSW_MASK_64)) {
         len = (uint32_t)len;
         addr_reg_size = 32;
     }

     ...

Or check for 32 bit and if that is also not the case abort() or 
something ?