Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.7\))
Subject: Re: [PATCH v2 04/13] RISC-V: crypto: add Zvkned accelerated AES
 implementation
From:   Jerry Shih <jerry.shih@sifive.com>
In-Reply-To: <20231128201228.GE1148@sol.localdomain>
Date:   Wed, 29 Nov 2023 10:39:56 +0800
Cc:     Conor Dooley <conor@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>, palmer@dabbelt.com,
        aou@eecs.berkeley.edu, herbert@gondor.apana.org.au,
        davem@davemloft.net, conor.dooley@microchip.com, ardb@kernel.org,
        heiko@sntech.de, phoebe.chen@sifive.com, hongrong.hsu@sifive.com,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-crypto@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <E78B3BF9-8E49-417B-A89E-05F72690A92F@sifive.com>
References: <20231127070703.1697-1-jerry.shih@sifive.com>
 <20231127070703.1697-5-jerry.shih@sifive.com>
 <20231128-await-tipper-2094715466f2@spud>
 <20231128201228.GE1148@sol.localdomain>
To:     Eric Biggers <ebiggers@kernel.org>
Precedence: bulk

On Nov 29, 2023, at 04:12, Eric Biggers <ebiggers@kernel.org> wrote:
> On Tue, Nov 28, 2023 at 05:54:49PM +0000, Conor Dooley wrote:
>>> +static inline bool check_aes_ext(void)
>>> +{
>>> +	return riscv_isa_extension_available(NULL, ZVKNED) &&
>>> +	       riscv_vector_vlen() >=3D 128;
>>> +}
>>=20
>> I'm not keen on this construct, where you are checking vlen greater =
than
>> 128 and the presence of Zvkned without checking for the presence of V
>> itself. Can you use "has_vector()" in any places where you depend on =
the
>> presence of vector please?
>=20
> Shouldn't both of those things imply vector support already?

The vector crypto extensions imply `V` extension. Should we still need =
to check
the `V` explicitly?
=
https://github.com/riscv/riscv-crypto/blob/main/doc/vector/riscv-crypto-sp=
ec-vector.adoc#1-extensions-overview

>> Also, there are potentially a lot of places in this drivers where you
>> can replace "riscv_isa_extension_available()" with
>> "riscv_has_extension_likely()". The latter is optimised with
>> alternatives, so in places that are going to be evaluated frequently =
it
>> may be beneficial for you.
>=20
> These extension checks are only executed in module_init functions, so =
they're
> not performance critical.

All `riscv_isa_extension_available()` calls in crypto drivers are called =
once
in the module init calls. Should we still need that =
`riscv_has_extension_likely()`
with a little more code size?

> - Eric