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

[...]

> +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?

> +
> +    /* 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);

Another weird nit, also here and everywhere else. POp says:


When the second-operand length is initially zero, the
following occurs. The parameter block, first, and sec-
ond operands are not accessed, and general regis-
ters R1, R2, and R2 + 1 are not changed.


I guess we need to bail early if length is 0.

[...]