[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0df559c7-920d-4d89-b1c8-75380750e7e1@rivosinc.com>
Date: Thu, 6 Feb 2025 11:08:57 +0100
From: Clément Léger <cleger@...osinc.com>
To: Conor Dooley <conor@...nel.org>, linux-riscv@...ts.infradead.org
Cc: Conor Dooley <conor.dooley@...rochip.com>,
Eric Biggers <ebiggers@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
<palmer@...belt.com>, Andy Chiu <andybnac@...il.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/6] RISC-V: add vector extension validation checks
On 05/02/2025 17:05, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.com>
>
> 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@...rochip.com>
> ---
> 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;
> +}
> +
> static int riscv_ext_zca_depends(const struct riscv_isa_ext_data *data,
> const unsigned long *isa_bitmap)
> {
> @@ -326,12 +355,10 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> __RISCV_ISA_EXT_DATA(d, RISCV_ISA_EXT_d),
> __RISCV_ISA_EXT_DATA(q, RISCV_ISA_EXT_q),
> __RISCV_ISA_EXT_SUPERSET(c, RISCV_ISA_EXT_c, riscv_c_exts),
> - __RISCV_ISA_EXT_SUPERSET(v, RISCV_ISA_EXT_v, riscv_v_exts),
> + __RISCV_ISA_EXT_SUPERSET_VALIDATE(v, RISCV_ISA_EXT_v, riscv_v_exts, riscv_ext_vector_float_validate),
> __RISCV_ISA_EXT_DATA(h, RISCV_ISA_EXT_h),
> - __RISCV_ISA_EXT_SUPERSET_VALIDATE(zicbom, RISCV_ISA_EXT_ZICBOM, riscv_xlinuxenvcfg_exts,
> - riscv_ext_zicbom_validate),
> - __RISCV_ISA_EXT_SUPERSET_VALIDATE(zicboz, RISCV_ISA_EXT_ZICBOZ, riscv_xlinuxenvcfg_exts,
> - riscv_ext_zicboz_validate),
> + __RISCV_ISA_EXT_SUPERSET_VALIDATE(zicbom, RISCV_ISA_EXT_ZICBOM, riscv_xlinuxenvcfg_exts, riscv_ext_zicbom_validate),
> + __RISCV_ISA_EXT_SUPERSET_VALIDATE(zicboz, RISCV_ISA_EXT_ZICBOZ, riscv_xlinuxenvcfg_exts, riscv_ext_zicboz_validate),
Hey Conor,
This chunk seems to just remove line wrapping for existing code, not
sure if this is intended.
Thanks,
Clément
> __RISCV_ISA_EXT_DATA(ziccrse, RISCV_ISA_EXT_ZICCRSE),
> __RISCV_ISA_EXT_DATA(zicntr, RISCV_ISA_EXT_ZICNTR),
> __RISCV_ISA_EXT_DATA(zicond, RISCV_ISA_EXT_ZICOND),
> @@ -372,11 +399,11 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> __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(zve32f, RISCV_ISA_EXT_ZVE32F, riscv_zve32f_exts),
> - __RISCV_ISA_EXT_DATA(zve32x, RISCV_ISA_EXT_ZVE32X),
> - __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_SUPERSET_VALIDATE(zve32f, RISCV_ISA_EXT_ZVE32F, riscv_zve32f_exts, riscv_ext_vector_float_validate),
> + __RISCV_ISA_EXT_DATA_VALIDATE(zve32x, RISCV_ISA_EXT_ZVE32X, riscv_ext_vector_x_validate),
> + __RISCV_ISA_EXT_SUPERSET_VALIDATE(zve64d, RISCV_ISA_EXT_ZVE64D, riscv_zve64d_exts, riscv_ext_vector_float_validate),
> + __RISCV_ISA_EXT_SUPERSET_VALIDATE(zve64f, RISCV_ISA_EXT_ZVE64F, riscv_zve64f_exts, riscv_ext_vector_float_validate),
> + __RISCV_ISA_EXT_SUPERSET_VALIDATE(zve64x, RISCV_ISA_EXT_ZVE64X, riscv_zve64x_exts, riscv_ext_vector_x_validate),
> __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),
> @@ -960,16 +987,6 @@ void __init riscv_fill_hwcap(void)
> riscv_v_setup_vsize();
> }
>
> - if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
> - /*
> - * ISA string in device tree might have 'v' flag, but
> - * CONFIG_RISCV_ISA_V is disabled in kernel.
> - * Clear V flag in elf_hwcap if CONFIG_RISCV_ISA_V is disabled.
> - */
> - if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
> - elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> - }
> -
> memset(print_str, 0, sizeof(print_str));
> for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
> if (riscv_isa[0] & BIT_MASK(i))
Powered by blists - more mailing lists