Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.7\))
Subject: Re: [PATCH v2 13/13] RISC-V: crypto: add Zvkb accelerated ChaCha20
 implementation
From:   Jerry Shih <jerry.shih@sifive.com>
In-Reply-To: <20231128042503.GL1463@sol.localdomain>
Date:   Tue, 28 Nov 2023 16:57:38 +0800
Cc:     Paul Walmsley <paul.walmsley@sifive.com>, palmer@dabbelt.com,
        Albert Ou <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: <3B3784E0-0DE2-4A99-878E-657BB0E0765D@sifive.com>
References: <20231127070703.1697-1-jerry.shih@sifive.com>
 <20231127070703.1697-14-jerry.shih@sifive.com>
 <20231128042503.GL1463@sol.localdomain>
To:     Eric Biggers <ebiggers@kernel.org>
Precedence: bulk

On Nov 28, 2023, at 12:25, Eric Biggers <ebiggers@kernel.org> wrote:
> On Mon, Nov 27, 2023 at 03:07:03PM +0800, Jerry Shih wrote:
>> +config CRYPTO_CHACHA20_RISCV64
>=20
> Can you call this kconfig option just CRYPTO_CHACHA_RISCV64?  I.e. =
drop the
> "20".  The ChaCha family of ciphers includes more than just ChaCha20.
>=20
> The other architectures do use "CHACHA20" in their equivalent option, =
even when
> they implement XChaCha12 too.  But that's for historical reasons -- we =
didn't
> want to break anything by renaming the kconfig options.  For a new =
option we
> should use the more general name from the beginning, even if initially =
only
> ChaCha20 is implemented (which is fine).

I will use `CRYPTO_CHACHA_RISCV64` instead.

>> +static int chacha20_encrypt(struct skcipher_request *req)
>=20
> riscv64_chacha_crypt(), please.  chacha20_encrypt() is dangerously =
close to
> being the same name as chacha20_crypt() which already exists in =
crypto/chacha.h.

The function will will have additional prefix/suffix.

>> +static inline bool check_chacha20_ext(void)
>> +{
>> +	return riscv_isa_extension_available(NULL, ZVKB) &&
>> +	       riscv_vector_vlen() >=3D 128;
>> +}
>=20
> Just to double check: your intent is to simply require VLEN >=3D 128 =
for all the
> RISC-V vector crypto code, even when some might work with a shorter =
VLEN?  I
> don't see anything in chacha-riscv64-zvkb.pl that assumes VLEN >=3D =
128, for
> example.  I think it would even work with VLEN =3D=3D 32.

Yes, the chacha algorithm here only needs the VLEN>=3D32. But I think we =
will not get
benefits with that kind of hw.

> I think requiring VLEN >=3D 128 anyway makes sense so that we don't =
have to worry
> about validating the code with shorter VLEN.  And "application =
processors" are
> supposed to have VLEN >=3D 128.  But I just wanted to make sure this =
is what you
> intended too.

The standard "V" extension assumes VLEN>=3D128. I just follow that =
assumption.
=
https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#183-v-vector=
-extension-for-application-processors

-Jerry