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

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
> 

[...]

> +/*
> + * read exactly one AES block from guest memory into a local buffer
> + */
> +static inline void aes_read_block(CPUS390XState *env, const int mmu_idx,
> +                                  const uintptr_t ra, uint64_t guest_addr,
> +                                  uint8_t *buf)
> +{
> +    read_guest_wrap_u8(env, mmu_idx, ra, guest_addr, buf, AES_BLOCK_SIZE);
> +}
> +
> +/*
> + * write exactly one AES block from local buffer to guest memory
> + */
> +static void aes_write_block(CPUS390XState *env, const int mmu_idx,
> +                            const uintptr_t ra, uint64_t guest_addr,
> +                            uint8_t *buf)
> +{
> +    write_guest_wrap_u8(env, mmu_idx, ra, guest_addr, buf, AES_BLOCK_SIZE);
> +}

nit: I would these two functions, I don't think they deserve a wrapper.

> +
> +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;
> +    }
> +
> +    /* length has to be properly aligned. */
> +    if (!QEMU_IS_ALIGNED(len, AES_BLOCK_SIZE)) {
> +        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> +    }
> +
> +    /* fetch key from param block */
> +    read_guest_wrap_u8(env, mmu_idx, ra, param_addr, key, keysize);
> +
> +    /* expand key */
> +    if (mod) {
> +        AES_set_decrypt_key(key, keysize * 8, &exkey);
> +    } else {
> +        AES_set_encrypt_key(key, keysize * 8, &exkey);
> +    }
> +
> +    /* process up to MAX_BLOCKS_PER_RUN aes blocks */
> +    for (i = 0; i < MAX_BLOCKS_PER_RUN && len >= AES_BLOCK_SIZE; i++) {
> +        aes_read_block(env, mmu_idx, ra, *src_ptr_reg + done, in);
> +        if (mod) {
> +            AES_decrypt(in, out, &exkey);
> +        } else {
> +            AES_encrypt(in, out, &exkey);
> +        }
> +        aes_write_block(env, mmu_idx, ra, *dst_ptr_reg + done, out);
> +        len -= AES_BLOCK_SIZE;
> +        done += AES_BLOCK_SIZE;
> +    }
> +
> +    *src_ptr_reg = deposit64(*src_ptr_reg, 0, addr_reg_size,
> +                             *src_ptr_reg + done);
> +    *dst_ptr_reg = deposit64(*dst_ptr_reg, 0, addr_reg_size,
> +                             *dst_ptr_reg + done);
> +    *src_len_reg -= done;
> +
> +    return !len ? 0 : 3;
> +}

The issue with writing memory, but not registers, on fault is still
there, but I think today's behavior is good enough for emulating real
sane code.


With our without the above addressed:

Reviewed-by: Ilya Leoshkevich <[email protected]>