Re: [PATCH v8 07/18] target/s390x: Support AES CTR for cpacf kmctr instruction

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

On 2026-06-24 19:26, Ilya Leoshkevich wrote:
> On 6/24/26 10:10, Harald Freudenberger wrote:
>> Support the subfunctions CPACF_KMCTR_AES_128, CPACF_KMCTR_AES_192
>> and CPACF_KMCTR_AES_256 for the cpacf kmctr instruction.
>> 
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>> Reviewed-by: Finn Callies <fcallies@linux.ibm.com>
>> ---
>>   target/s390x/gen-features.c      |  3 ++
>>   target/s390x/tcg/cpacf.h         |  5 +++
>>   target/s390x/tcg/cpacf_aes.c     | 76 
>> ++++++++++++++++++++++++++++++++
>>   target/s390x/tcg/crypto_helper.c | 24 ++++++++++
>>   4 files changed, 108 insertions(+)
> 
> [...]
> 
>> --- a/target/s390x/tcg/cpacf_aes.c
>> +++ b/target/s390x/tcg/cpacf_aes.c
>> @@ -213,3 +213,79 @@ int cpacf_aes_cbc(CPUS390XState *env, const int 
>> mmu_idx, uintptr_t ra,
>>         return !len ? 0 : 3;
>>   }
>> +
>> +int cpacf_aes_ctr(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,
>> +                  uint64_t *ctr_ptr_reg, uint32_t type,
>> +                  uint8_t fc, uint8_t mod)
>> +{
>> +    enum { MAX_BLOCKS_PER_RUN = 8192 / AES_BLOCK_SIZE };
>> +    const MemOpIdx oi = make_memop_idx(MO_8, mmu_idx);
>> +    uint8_t ctr[AES_BLOCK_SIZE], buf[AES_BLOCK_SIZE];
>> +    uint8_t in[AES_BLOCK_SIZE], out[AES_BLOCK_SIZE];
>> +    uint64_t addr, 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_KMCTR);
>> +
>> +    switch (fc) {
>> +    case 0x12: /* CPACF_KMCTR_AES_128 */
>> +        keysize = 16;
>> +        break;
>> +    case 0x13: /* CPACF_KMCTR_AES_192 */
>> +        keysize = 24;
>> +        break;
>> +    case 0x14: /* CPACF_KMCTR_AES_256 */
>> +        keysize = 32;
>> +        break;
>> +    default:
>> +        g_assert_not_reached();
>> +    }
> 
> A general comment; I think I've seen this in previous patches as well.
> Would it make sense to properly define constants like
> CPACF_KMCTR_AES_128 and drop comments? Or, if the goal is easier
> cross-checking with POp, hardcode decimal numbers, since POp specifies
> them in decimal?
> 

As I am about to write some tests for all this CPACF support there would
be a benefit in having all this as constants or defines in a header file
instead of hardcoded values on every switch statement. So I am about to
introduce a some cpacf.h with all these values for the next version.

>> +
>> +    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 */
>> +    for (i = 0; i < keysize; i++) {
>> +        addr = wrap_address(env, param_addr + i);
>> +        key[i] = cpu_ldb_mmu(env, addr, oi, ra);
>> +    }
>> +
>> +    /* expand key */
>> +    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++) 
>> {
>> +        /* read in nonce/ctr => ctr */
>> +        aes_read_block(env, mmu_idx, *ctr_ptr_reg + done, ctr, ra);
>> +        /* encrypt ctr => buf */
>> +        AES_encrypt(ctr, buf, &exkey);
>> +        /* read in one block of input data => in */
>> +        aes_read_block(env, mmu_idx, *src_ptr_reg + done, in, ra);
>> +        /* xor input data with encrypted ctr => out */
>> +        aes_xor(in, buf, out);
>> +        /* write out the processed block */
>> +        aes_write_block(env, mmu_idx, *dst_ptr_reg + done, out, ra);
>> +        len -= AES_BLOCK_SIZE, done += AES_BLOCK_SIZE;
> 
> Same comment as in the previous patch.
> 
> 
> Otherwise:
> 
> Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
> 
> [...]