Re: [RFC v1 2/5] RISC-V: add f & d extension validation checks

Clément Léger
2024-10-03T07:59:14Z

On 02/10/2024 18:10, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Using Clement's new validation callbacks, support checking that
> dependencies have been satisfied for the floating point extensions.
>
> The check for "d" might be slightly confusingly shorter than that of "f",
> despite "d" depending on "f". This is because the requirement that a
> hart supporting double precision must also support single precision,
> should be validated by dt-bindings etc, not the kernel but lack of
> support for single precision only is a limitation of the kernel.
>
> 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@microchip.com>
> ---
> arch/riscv/kernel/cpufeature.c | 36 +++++++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 84a2ad2581cb0..b8a22ee76c2ef 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -101,6 +101,29 @@ static int riscv_ext_zicboz_validate(const struct riscv_isa_ext_data *data,
> return 0;
> }
>
> +static int riscv_ext_f_validate(const struct riscv_isa_ext_data *data,
> + const unsigned long *isa_bitmap)
> +{
> + if (!__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_d)) {
> + pr_warn_once("This kernel does not support systems with F but not D\n");
> + return -EINVAL;
> + }
> +
> + if (IS_ENABLED(CONFIG_FPU))
> + return -EINVAL;

Hey Conor,

Shouldn't this be !IS_ENABLED(CONFIG_FPU)) ? I mean, if the f extension
is enabled but not CONFIG_FPU, then disable it.

Clément

> +
> + return 0;
> +}
> +
> +static int riscv_ext_d_validate(const struct riscv_isa_ext_data *data,
> + const unsigned long *isa_bitmap)
> +{
> + if (IS_ENABLED(CONFIG_FPU))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int riscv_ext_vector_x_validate(const struct riscv_isa_ext_data *data,
> const unsigned long *isa_bitmap)
> {
> @@ -351,8 +374,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> __RISCV_ISA_EXT_DATA(i, RISCV_ISA_EXT_i),
> __RISCV_ISA_EXT_DATA(m, RISCV_ISA_EXT_m),
> __RISCV_ISA_EXT_DATA(a, RISCV_ISA_EXT_a),
> - __RISCV_ISA_EXT_DATA(f, RISCV_ISA_EXT_f),
> - __RISCV_ISA_EXT_DATA(d, RISCV_ISA_EXT_d),
> + __RISCV_ISA_EXT_DATA_VALIDATE(f, RISCV_ISA_EXT_f, riscv_ext_f_validate),
> + __RISCV_ISA_EXT_DATA_VALIDATE(d, RISCV_ISA_EXT_d, riscv_ext_d_validate),
> __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_VALIDATE(v, RISCV_ISA_EXT_v, riscv_v_exts, riscv_ext_vector_float_validate),
> @@ -912,15 +935,6 @@ void __init riscv_fill_hwcap(void)
> }
> }
>
> - /*
> - * We don't support systems with F but without D, so mask those out
> - * here.
> - */
> - if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) {
> - pr_info("This kernel does not support systems with F but not D\n");
> - elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
> - }
> -
> if (__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_ZVE32X)) {
> /*
> * This cannot fail when called on the boot hart