On Fri, 21 Jul 2023 at 13:39, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> On Fri, 21 Jul 2023 at 07:40, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> >
> > On Tue, Jul 11, 2023 at 05:37:41PM +0200, Heiko Stuebner wrote:
> ...
> > > +static int riscv64_aes_setkey_zvkned(struct crypto_tfm *tfm, const u8 *key,
> > > + unsigned int keylen)
> > > +{
> > > + struct riscv_aes_ctx *ctx = crypto_tfm_ctx(tfm);
> > > + int ret;
> > > +
> > > + ctx->keylen = keylen;
> > > +
> > > + if (keylen == 16 || keylen == 32) {
> > > + kernel_rvv_begin();
> > > + ret = rv64i_zvkned_set_encrypt_key(key, keylen * 8, &ctx->enc_key);
> > > + if (ret != 1) {
> > > + kernel_rvv_end();
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = rv64i_zvkned_set_decrypt_key(key, keylen * 8, &ctx->dec_key);
>
> The asm suggests that the encryption and decryption key schedules are
> the same, and the decryption algorithm does not implement the
> Equivalent Inverse Cipher, but simply iterates over they key schedule
> in reverse order. This makes much more sense for instruction based
> AES, so it doesn't surprise me but it does mean you can just drop this
> part, and pass enc_key everywhere.
>
> > > + kernel_rvv_end();
> > > + if (ret != 1)
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = crypto_cipher_setkey(ctx->fallback, key, keylen);
> > > +
> > > + return ret ? -EINVAL : 0;
> > > +}
> >
> > It's a bit annoying that RISC-V doesn't support AES-192, though also not
> > particularly surprising, seeing as AES-192 is almost never used. (Intel's Key
> > Locker, for example, is another recent CPU feature that doesn't support
> > AES-192.) IMO the issue here is really with the kernel crypto API -- it should
> > treat AES-128, AES-192, and AES-256 as separate algorithms so that
> > implementations aren't forced to support all three key sizes...
> >
>
> Why is this a fundamental limitation? AES-192 uses the same AES block
> size and round structure, the only difference is the number of rounds
> and how the round keys are calculated.
>
> Creating the key schedule should never be performance critical, so if
> the lack of AES-192 support is due to a limitation in the key schedule
> generation instructions, I'd suggest to avoid those if possible and
> just use the generic library code to derive the key schedule. If that
> works, I'm pretty sure AES-192 support is just a matter of
> implementing a 12-round variant modeled after the existing 10/14 round
> ones.
This seems to work:
https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=riscv-crypto
Feel free to incorporate/squash any of those changes into your series.
