Re: [PATCH v3 2/3] riscv: add ISA extension parsing for bfloat16 ISA extension
From: Conor Dooley
Date: Tue Feb 11 2025 - 08:45:20 EST
On Tue, Feb 11, 2025 at 08:42:39AM +0800, Inochi Amaoto wrote:
> 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@xxxxxxxxx>
> > > ---
> > > 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;
This is not needed I think, V "turns on" Zve32f. If anything, you should
be checking for CONFIG_RISCV_ISA_V here ^^
You /could/ call the resulting riscv_vector_f_validate(), since this is
nothing specific to Zvfvfmin, and could be used for another extension
that requires a Zve32f or Zve64 minimum base.
>
> 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@xxxxxxxxxxxx>
> >
> > 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),
> >
Attachment:
signature.asc
Description: PGP signature