Re: [PATCH v3 2/6] RISC-V: add vector crypto extension validation checks
From: Clément Léger
Date: Thu Feb 06 2025 - 07:56:45 EST
On 06/02/2025 12:24, Conor Dooley wrote:
> On Thu, Feb 06, 2025 at 11:20:35AM +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.
>>
>> To which part of the commit does this refer to ?
>
> Copy-paste mistake when splitting in two, whoops.
>
>>> @@ -397,8 +414,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>>> __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
>>> __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
>>> __RISCV_ISA_EXT_DATA(ztso, RISCV_ISA_EXT_ZTSO),
>>> - __RISCV_ISA_EXT_SUPERSET(zvbb, RISCV_ISA_EXT_ZVBB, riscv_zvbb_exts),
>>> - __RISCV_ISA_EXT_DATA(zvbc, RISCV_ISA_EXT_ZVBC),
>>> + __RISCV_ISA_EXT_SUPERSET_VALIDATE(zvbb, RISCV_ISA_EXT_ZVBB, riscv_zvbb_exts, riscv_ext_vector_x_validate),
>>> + __RISCV_ISA_EXT_DATA_VALIDATE(zvbc, RISCV_ISA_EXT_ZVBC, riscv_ext_vector_crypto_validate),
>
>>
>> I'm not sure if I already made that comment, so here we go again.
>> Shouldn't Zvbb use riscv_ext_vector_crypto_validate() as well ? The
>> spec states that Zvbb is a superset of Zvkb and Zvkb uses the
>> riscv_ext_vector_crypto_validate() validation callback. I guess Zvbc
>> should also use it based on your spec excerpt in the commmit log.
>
> Zvbc does use it, no? I'll amend the Zvbb one, there should only be two
> users of the "x" variant.
Oh yes Zvbc is already ok, forget that then.
Clément