On 11/02/2025 13:34, Conor Dooley wrote:
> 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?
Yeah that would be helpful I guess.
Clément
