Received-SPF: pass (google.com: domain of linux-kernel+bounces-16394-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Content-Type: text/plain;
	charset=us-ascii
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.7\))
Subject: Re: [RFC PATCH 07/13] crypto: riscv - add vector crypto accelerated
 AES-{ECB,CBC,CTR,XTS}
From: Jerry Shih <jerry.shih@sifive.com>
In-Reply-To: <20240103145043.GB773@quark.localdomain>
Date: Thu, 4 Jan 2024 16:47:16 +0800
Cc: linux-crypto@vger.kernel.org,
 linux-riscv@lists.infradead.org,
 linux-kernel@vger.kernel.org,
 Ard Biesheuvel <ardb@kernel.org>,
 Heiko Stuebner <heiko@sntech.de>,
 Phoebe Chen <phoebe.chen@sifive.com>,
 hongrong.hsu@sifive.com,
 Paul Walmsley <paul.walmsley@sifive.com>,
 Palmer Dabbelt <palmer@dabbelt.com>,
 Albert Ou <aou@eecs.berkeley.edu>,
 Andy Chiu <andy.chiu@sifive.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <905D43CF-B01A-49DE-9046-51A370B6F680@sifive.com>
References: <20240102064743.220490-1-ebiggers@kernel.org>
 <20240102064743.220490-8-ebiggers@kernel.org>
 <20240103145043.GB773@quark.localdomain>
To: Eric Biggers <ebiggers@kernel.org>

On Jan 3, 2024, at 22:50, Eric Biggers <ebiggers@kernel.org> wrote:
> On Tue, Jan 02, 2024 at 12:47:33AM -0600, Eric Biggers wrote:
>> diff --git a/arch/riscv/crypto/Makefile b/arch/riscv/crypto/Makefile
>> index dca698c5cba3e..5dd91f34f0d52 100644
>> --- a/arch/riscv/crypto/Makefile
>> +++ b/arch/riscv/crypto/Makefile
>> @@ -1,7 +1,10 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> #
>> # linux/arch/riscv/crypto/Makefile
>> #
>>=20
>> obj-$(CONFIG_CRYPTO_AES_RISCV64) +=3D aes-riscv64.o
>> aes-riscv64-y :=3D aes-riscv64-glue.o aes-riscv64-zvkned.o
>> +
>> +obj-$(CONFIG_CRYPTO_AES_BLOCK_RISCV64) +=3D aes-block-riscv64.o
>> +aes-block-riscv64-y :=3D aes-riscv64-block-mode-glue.o =
aes-riscv64-zvkned-zvbb-zvkg.o aes-riscv64-zvkned-zvkb.o
>=20
> A bug I noticed (which is also present in Jerry's patchset) is that =
some of the
> code of the aes-block-riscv64 module is located in =
aes-riscv64-zvkned.S, which
> isn't built into that module but rather into aes-riscv64.  This causes =
a build
> error when both CONFIG_CRYPTO_AES_RISCV64 and =
CONFIG_CRYPTO_AES_BLOCK_RISCV64
> are set to 'm':
>=20
>    ERROR: modpost: "aes_cbc_decrypt_zvkned" =
[arch/riscv/crypto/aes-block-riscv64.ko] undefined!
>    ERROR: modpost: "aes_ecb_decrypt_zvkned" =
[arch/riscv/crypto/aes-block-riscv64.ko] undefined!
>    ERROR: modpost: "aes_cbc_encrypt_zvkned" =
[arch/riscv/crypto/aes-block-riscv64.ko] undefined!
>    ERROR: modpost: "aes_ecb_encrypt_zvkned" =
[arch/riscv/crypto/aes-block-riscv64.ko] undefined!
>=20
> To fix this, I think we should just merge the two modules and kconfig =
options
> together so that there is one module that provides both the AES modes =
and the
> AES single-block cipher.  That's how x86's aesni-intel works, for =
example.
>=20
> - Eric

That's a bug in my patchset.
I don't test with all options with `M` settings since I can't boot to =
qemu with all `M` settings.

Could we move the cbc and ecb from `aes-riscv64-zvkned` to =
`aes-block-riscv64` instead of merging
these two modules?
Thus, we could still enable the single aes block cipher without other =
extensions(e.g. zvbb or zvkg).

-Jerry=