linux-kernel - Re: [PATCH v3 1/6] RISC-V: add vector extension validation checks

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] [day] [month] [year] [list]

Message-ID: <20250211-crier-gas-7efc51783cf3@spud>
Date: Tue, 11 Feb 2025 14:43:17 +0000
From: Conor Dooley <conor@...nel.org>
To: Clément Léger <cleger@...osinc.com>
Cc: linux-riscv@...ts.infradead.org,
	Conor Dooley <conor.dooley@...rochip.com>,
	Eric Biggers <ebiggers@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>, Andy Chiu <andybnac@...il.com>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/6] RISC-V: add vector extension validation checks

On Tue, Feb 11, 2025 at 11:16:45AM +0100, Clément Léger wrote:
> 
> 
> On 05/02/2025 17:05, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@...rochip.com>
> > 
> > Using Clement's new validation callbacks, support checking that
> > dependencies have been satisfied for the vector extensions. From the
> > kernel's perfective, it's not required to differentiate between the
> > conditions for all the various vector subsets - it's the firmware's job
> > to not report impossible combinations. Instead, the kernel only has to
> > check that the correct config options are enabled and to enforce its
> > requirement of the d extension being present for FPU support.
> > 
> > Since vector will now be disabled proactively, there's no need to clear
> > the bit in elf_hwcap in riscv_fill_hwcap() any longer.
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> > ---
> >  arch/riscv/include/asm/cpufeature.h |  3 ++
> >  arch/riscv/kernel/cpufeature.c      | 57 +++++++++++++++++++----------
> >  2 files changed, 40 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> > index 569140d6e639..5d9427ccbc7a 100644
> > --- a/arch/riscv/include/asm/cpufeature.h
> > +++ b/arch/riscv/include/asm/cpufeature.h
> > @@ -56,6 +56,9 @@ void __init riscv_user_isa_enable(void);
> >  #define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \
> >  	_RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, \
> >  			    ARRAY_SIZE(_bundled_exts), NULL)
> > +#define __RISCV_ISA_EXT_BUNDLE_VALIDATE(_name, _bundled_exts, _validate) \
> > +	_RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, \
> > +			    ARRAY_SIZE(_bundled_exts), _validate)
> >  
> >  /* Used to declare extensions that are a superset of other extensions (Zvbb for instance) */
> >  #define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index c6ba750536c3..40a24b08d905 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -109,6 +109,35 @@ static int riscv_ext_zicboz_validate(const struct riscv_isa_ext_data *data,
> >  	return 0;
> >  }
> >  
> > +static int riscv_ext_vector_x_validate(const struct riscv_isa_ext_data *data,
> > +				       const unsigned long *isa_bitmap)
> > +{
> > +	if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int riscv_ext_vector_float_validate(const struct riscv_isa_ext_data *data,
> > +					   const unsigned long *isa_bitmap)
> > +{
> > +	if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
> > +		return -EINVAL;
> > +
> > +	if (!IS_ENABLED(CONFIG_FPU))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * The kernel doesn't support systems that don't implement both of
> > +	 * F and D, so if any of the vector extensions that do floating point
> > +	 * are to be usable, both floating point extensions need to be usable.
> > +	 */
> > +	if (!__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_d))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> 
> I think this should also be modified to be like this:
> 
>   if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_d))
> 	  return 0;
> 
>   return -EPROBEDEFER;
> 
> That won't actually change the way it works since RISCV_ISA_EXT_d (and
> all single letter extensions) is always probed before the others though.

I don't think so, there's no point deferring since we know that the
extensions this is used for cannot become true afterwards. I'll add a
comment justifying it.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)