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@xxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxx>
> > ---
> > 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.
Attachment:
signature.asc
Description: PGP signature
