Received-SPF: pass (google.com: domain of linux-crypto+bounces-1235-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 4 Jan 2024 09:18:21 -0800
From: Eric Biggers <ebiggers@kernel.org>
To: Jerry Shih <jerry.shih@sifive.com>
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>
Subject: Re: [RFC PATCH 07/13] crypto: riscv - add vector crypto accelerated
 AES-{ECB,CBC,CTR,XTS}
Message-ID: <20240104171821.GA1127@sol.localdomain>
References: <20240102064743.220490-1-ebiggers@kernel.org>
 <20240102064743.220490-8-ebiggers@kernel.org>
 <20240103145043.GB773@quark.localdomain>
 <905D43CF-B01A-49DE-9046-51A370B6F680@sifive.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <905D43CF-B01A-49DE-9046-51A370B6F680@sifive.com>

On Thu, Jan 04, 2024 at 04:47:16PM +0800, Jerry Shih wrote:
> 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
> >> #
> >> 
> >> obj-$(CONFIG_CRYPTO_AES_RISCV64) += aes-riscv64.o
> >> aes-riscv64-y := aes-riscv64-glue.o aes-riscv64-zvkned.o
> >> +
> >> +obj-$(CONFIG_CRYPTO_AES_BLOCK_RISCV64) += aes-block-riscv64.o
> >> +aes-block-riscv64-y := aes-riscv64-block-mode-glue.o aes-riscv64-zvkned-zvbb-zvkg.o aes-riscv64-zvkned-zvkb.o
> > 
> > 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':
> > 
> >    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!
> > 
> > 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.
> > 
> > - 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).

My proposal to merge the two modules still results in the single-block AES
cipher being registered when zvkned alone is present.  It just won't be
selectable separately via kconfig.  I think that's fine.

- Eric