On Tue, Feb 11, 2025 at 09:45:44AM +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 crpyto extensions.
> > Currently riscv_isa_extension_available(<vector crypto>) will return
> > true on systems that support the extensions but vector itself has been
> > disabled by the kernel, adding validation callbacks will prevent such a
> > scenario from occuring and make the behaviour of the extension detection
> > functions more consistent with user expectations - it's not expected to
> > have to check for vector AND the specific crypto extension.
> >
> > The 1.0.0 Vector crypto spec states:
> > The Zvknhb and Zvbc Vector Crypto Extensions --and accordingly
> > the composite extensions Zvkn and Zvks-- require a Zve64x base,
> > or application ("V") base Vector Extension. All of the other
> > Vector Crypto Extensions can be built on any embedded (Zve*) or
> > application ("V") base Vector Extension.
> > and this could be used as the basis for checking that the correct base
> > for individual crypto extensions, but that's not really the kernel's job
> > in my opinion and it is sufficient to leave that sort of precision to
> > the dt-bindings. The kernel only needs to make sure that vector, in some
> > form, is available.
> >
> > Since vector will now be disabled proactively, there's no need to clear
> > the bit in elf_hwcap in riscv_fill_hwcap() any longer.
> >
> > Link: https://github.com/riscv/riscv-crypto/releases/tag/v1.0.0
> > Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > ---
> > arch/riscv/kernel/cpufeature.c | 49 +++++++++++++++++++++++-----------
> > 1 file changed, 33 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 40a24b08d905..1c148ecea612 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -138,6 +138,23 @@ static int riscv_ext_vector_float_validate(const struct riscv_isa_ext_data *data
> > return 0;
> > }
> >
> > +static int riscv_ext_vector_crypto_validate(const struct riscv_isa_ext_data *data,
> > + const unsigned long *isa_bitmap)
> > +{
> > + if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
> > + return -EINVAL;
> > +
> > + /*
> > + * It isn't the kernel's job to check that the binding is correct, so
> > + * it should be enough to check that any of the vector extensions are
> > + * enabled, which in-turn means that vector is usable in this kernel
> > + */
> > + if (!__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZVE32X))
> > + return -EINVAL;
>
> After a second thought, I think it should be this:
>
> if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZVE32X))
> return 0;
>
> return -EPROBEDEFER;
>
> Extensions can be enabled later (but can not be "reverted") so check for
> the extension to be present (in which case it's ok), or wait for it to
> be (potentially) enabled.
Ah, of course it is operating on the /resolved/ isa, not the source one.
Makes me thing the parameter of all the validate callbacks should be
"resolved_isa_bitmap" instead of "isa_bitmap" to make things clearer?
Attachment:
signature.asc
Description: PGP signature
