linux-kernel - Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 18 Apr 2024 10:39:46 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: Conor Dooley <conor@...nel.org>
Cc: Conor Dooley <conor.dooley@...rochip.com>,
	Andy Chiu <andy.chiu@...ive.com>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>, Heiko Stuebner <heiko@...ech.de>,
	Guo Ren <guoren@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Jonathan Corbet <corbet@....net>, Evan Green <evan@...osinc.com>,
	Clément Léger <cleger@...osinc.com>,
	Shuah Khan <shuah@...nel.org>, linux-riscv@...ts.infradead.org,
	linux-kernel@...r.kernel.org, Palmer Dabbelt <palmer@...osinc.com>,
	Vincent Chen <vincent.chen@...ive.com>,
	Greentime Hu <greentime.hu@...ive.com>, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org,
	Joel Granados <j.granados@...sung.com>,
	Jerry Shih <jerry.shih@...ive.com>
Subject: Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement
 to ZVE32X

On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > If it would be useful to do so, we should be able to enable some of the code
> > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > SHA512 instructions require EEW==64.)
> > > 
> > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > need to be revised and tested in that configuration.
> > > 
> > > > Eric/Jerry (although read the previous paragraph too):
> > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > does not call either, which seems to violate the edict in
> > > > kernel_vector_begin()'s kerneldoc:
> > > > "Must not be called unless may_use_simd() returns true."
> > > 
> > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > differs from shash algorithms which can be invoked in any context.
> > > 
> > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > this was intentionally done in order to support use cases like this.  So that's
> > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > kernel_vector_begin().
> > 
> > I see, thanks for explaining that. I think you should probably check
> > somewhere if has_vector() returns true in that driver though before
> > using vector instructions. Only checking vlen seems to me like relying on
> > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > systems actually support Zvkb, but I hope you get my drift.
> 
> All the algorithms check for at least one of the vector crypto extensions being
> supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> should return whether the ratified version of Zvkb is supported, and likewise
> for the other vector crypto extensions.  The ratified version of the vector
> crypto extensions depends on the ratified version of the vector extension.  So
> there should be no issue.  If there is, the RISC-V core architecture code needs
> to be fixed to not declare that extensions are supported when they are actually
> incompatible non-standard versions of those extensions.  Incompatible
> non-standard extensions should be represented as separate extensions.
> 

It probably makes sense to check has_vector() to exclude Zve* for now, though.

I am just concerned about how you're suggesting that non-standard extensions
might be pretending to be standard ones and individual users of kernel-mode
vector would need to work around that.  I think that neither has_vector() nor
'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
vector extension is non-standard.

- Eric