linux-kernel - Re: [PATCH v3 2/3] riscv: add ISA extension parsing for bfloat16 ISA extension

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: <7qkfqzhytjq2qwo2wg3xtkoqu6id6wduckeeudbn2yt5p5p7xv@2gl5bcny26rk>
Date: Tue, 11 Feb 2025 08:42:39 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Clément Léger <cleger@...osinc.com>
Cc: linux-doc@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, Yixun Lan <dlan@...too.org>, 
	Longbin Li <looong.bin@...il.com>, Jesse Taube <jesse@...osinc.com>, 
	Yong-Xuan Wang <yongxuan.wang@...ive.com>, Samuel Holland <samuel.holland@...ive.com>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Evan Green <evan@...osinc.com>, 
	Andrew Jones <ajones@...tanamicro.com>, Alexandre Ghiti <alexghiti@...osinc.com>, 
	Andy Chiu <andybnac@...il.com>, Charlie Jenkins <charlie@...osinc.com>, 
	Conor Dooley <conor+dt@...nel.org>, Rob Herring <robh@...nel.org>, Albert Ou <aou@...s.berkeley.edu>, 
	Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Jonathan Corbet <corbet@....net>, Chen Wang <unicorn_wang@...look.com>, 
	Inochi Amaoto <inochiama@...il.com>
Subject: Re: [PATCH v3 2/3] riscv: add ISA extension parsing for bfloat16 ISA
 extension

On Mon, Feb 10, 2025 at 03:38:58PM +0100, Clément Léger wrote:
> 
> 
> On 06/12/2024 06:58, Inochi Amaoto wrote:
> > Add parsing for Zfbmin, Zvfbfmin, Zvfbfwma ISA extension which
> > were ratified in 4dc23d62 ("Added Chapter title to BF16") of
> > the riscv-isa-manual.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@...il.com>
> > ---
> >  arch/riscv/include/asm/hwcap.h | 3 +++
> >  arch/riscv/kernel/cpufeature.c | 3 +++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 869da082252a..14cc29f2a723 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -100,6 +100,9 @@
> >  #define RISCV_ISA_EXT_ZICCRSE		91
> >  #define RISCV_ISA_EXT_SVADE		92
> >  #define RISCV_ISA_EXT_SVADU		93
> > +#define RISCV_ISA_EXT_ZFBFMIN		94
> > +#define RISCV_ISA_EXT_ZVFBFMIN		95
> > +#define RISCV_ISA_EXT_ZVFBFWMA		96
> >  
> >  #define RISCV_ISA_EXT_XLINUXENVCFG	127
> >  
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index c0916ed318c2..5cfcab139568 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -341,6 +341,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> >  	__RISCV_ISA_EXT_DATA(zacas, RISCV_ISA_EXT_ZACAS),
> >  	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> >  	__RISCV_ISA_EXT_DATA(zfa, RISCV_ISA_EXT_ZFA),
> > +	__RISCV_ISA_EXT_DATA(zfbfmin, RISCV_ISA_EXT_ZFBFMIN),
> 
> Hi Inochi,
> 
> You could add a validation callback to that extension:
> 
> static int riscv_ext_f_depends(const struct riscv_isa_ext_data *data,
> 			       const unsigned long *isa_bitmap)
> {
> 	if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_f))
> 		return 0;
> 
> 	return -EPROBE_DEFER;
> }
> 
>   ...
>   __RISCV_ISA_EXT_DATA_VALIDATE(zfbfmin, RISCV_ISA_EXT_ZFBFMIN,
> riscv_ext_f_depends),
> 
> 
> But I'm ok with the current state of that patch since I have the same
> thing coming for other extensions as well. 


I think it is good for me to add the check, and I wonder it is possible
to add the extra check for zvfbfmin and zvfbfwma like this:

static int riscv_ext_zvfbfmin_validate(const struct riscv_isa_ext_data *data,
				       const unsigned long *isa_bitmap)
{
	if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_v))
		return 0;

	if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZVE32F))
		return 0;

	return -EPROBE_DEFER;
}

static int riscv_ext_zvfbfwma_validate(const struct riscv_isa_ext_data *data,
				       const unsigned long *isa_bitmap)
{
	if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZFBFMIN) &&
	    __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZVFBFMIN))
		return 0;

	return -EPROBE_DEFER;
}

> So with or without my previous comment fixed:
> 
> Reviewed-by: Clément Léger <cleger@...osinc.com>
> 
> Thanks,
> 
> Clément
> 

Thanks,

Regards,
Inochi

> >  	__RISCV_ISA_EXT_DATA(zfh, RISCV_ISA_EXT_ZFH),
> >  	__RISCV_ISA_EXT_DATA(zfhmin, RISCV_ISA_EXT_ZFHMIN),
> >  	__RISCV_ISA_EXT_DATA(zca, RISCV_ISA_EXT_ZCA),
> > @@ -373,6 +374,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> >  	__RISCV_ISA_EXT_SUPERSET(zve64d, RISCV_ISA_EXT_ZVE64D, riscv_zve64d_exts),
> >  	__RISCV_ISA_EXT_SUPERSET(zve64f, RISCV_ISA_EXT_ZVE64F, riscv_zve64f_exts),
> >  	__RISCV_ISA_EXT_SUPERSET(zve64x, RISCV_ISA_EXT_ZVE64X, riscv_zve64x_exts),
> > +	__RISCV_ISA_EXT_DATA(zvfbfmin, RISCV_ISA_EXT_ZVFBFMIN),
> > +	__RISCV_ISA_EXT_DATA(zvfbfwma, RISCV_ISA_EXT_ZVFBFWMA),
> >  	__RISCV_ISA_EXT_DATA(zvfh, RISCV_ISA_EXT_ZVFH),
> >  	__RISCV_ISA_EXT_DATA(zvfhmin, RISCV_ISA_EXT_ZVFHMIN),
> >  	__RISCV_ISA_EXT_DATA(zvkb, RISCV_ISA_EXT_ZVKB),
>