Re: [PATCH v6 11/14] target/riscv: rvk: add support for zksed/zksh extension

[Richard Henderson]

On 2/27/22 04:25, Weiwei Li wrote:
>   - add sm3p0, sm3p1, sm4ed and sm4ks instructions
>
> Co-authored-by: Ruibo Lu <luruibo2000@163.com>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> ---
>   target/riscv/crypto_helper.c            | 49 +++++++++++++++++
>   target/riscv/helper.h                   |  6 +++
>   target/riscv/insn32.decode              |  6 +++
>   target/riscv/insn_trans/trans_rvk.c.inc | 72 +++++++++++++++++++++++++
>   4 files changed, 133 insertions(+)
>
> diff --git a/target/riscv/crypto_helper.c b/target/riscv/crypto_helper.c
> index fd50a034a3..d712854a9c 100644
> --- a/target/riscv/crypto_helper.c
> +++ b/target/riscv/crypto_helper.c
> @@ -391,4 +391,53 @@ target_ulong HELPER(sha512sum1)(target_ulong rs1)
>       return ROR64(a, 14) ^ ROR64(a, 18) ^ ROR64(a, 41);
>   }
>   #undef ROR64
> +
> +#define ROL32(a, amt) ((a >> (-amt & 31)) | (a << (amt & 31)))
> +
> +target_ulong HELPER(sm3p0)(target_ulong rs1)
> +{
> +    uint32_t src = rs1;
> +    uint32_t result = src ^ ROL32(src, 9) ^ ROL32(src, 17);
> +
> +    return sext_xlen(result);
> +}
> +
> +target_ulong HELPER(sm3p1)(target_ulong rs1)
> +{
> +    uint32_t src = rs1;
> +    uint32_t result = src ^ ROL32(src, 15) ^ ROL32(src, 23);
> +
> +    return sext_xlen(result);
> +}
> +#undef ROL32
> +

Same comments as before, with the sticker being that you've defined ROL32 twice.

> +target_ulong HELPER(sm4ed)(target_ulong rs2, target_ulong rt, target_ulong bs)
> +{
> +    uint8_t bs_t = bs;

Why the intermediate?  And again, perhaps better to pass in shamt.

> +
> +    uint32_t sb_in = (uint8_t)(rs2 >> (8 * bs_t));
> +    uint32_t sb_out = (uint32_t)sm4_sbox[sb_in];
> +
> +    uint32_t linear = sb_out ^ (sb_out << 8) ^ (sb_out << 2) ^ (sb_out << 18) ^
> +        ((sb_out & 0x3f) << 26) ^ ((sb_out & 0xC0) << 10);
> +
> +    uint32_t rotl = (linear << (8 * bs_t)) | (linear >> (32 - 8 * bs_t));

Again, broken rotate expression.

> +target_ulong HELPER(sm4ks)(target_ulong rs2, target_ulong rs1, target_ulong bs)

Same.

> +static bool trans_sm3p0(DisasContext *ctx, arg_sm3p0 *a)
> +{
> +    REQUIRE_ZKSH(ctx);
> +
> +    TCGv dest = dest_gpr(ctx, a->rd);
> +    TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
> +
> +    gen_helper_sm3p0(dest, src1);
> +    gen_set_gpr(ctx, a->rd, dest);
> +
> +    return true;
> +}

gen_unary, etc.

> +static bool trans_sm4ks(DisasContext *ctx, arg_sm4ks *a)
> +{
> +    REQUIRE_ZKSED(ctx);
> +
> +    TCGv bs = tcg_const_tl(a->bs);
> +    TCGv dest = dest_gpr(ctx, a->rd);
> +    TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
> +    TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> +
> +    gen_helper_sm4ks(dest, src2, src1, bs);
> +    gen_set_gpr(ctx, a->rd, dest);
> +
> +    tcg_temp_free(bs);
> +    return true;
> +}

Reuse that helper you created for aes32esmi et al back in patch 6.


r~