Re: [PATCH -next v19 02/24] riscv: Extending cpufeature.c to detect V-extension

Palmer Dabbelt
2023-05-12T00:01:21Z

On Tue, 09 May 2023 03:30:11 PDT (-0700), andy.chiu@sifive.com wrote:
> From: Guo Ren <ren_guo@c-sky.com>
>
> Add V-extension into riscv_isa_ext_keys array and detect it with isa
> string parsing.
>
> Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> Suggested-by: Vineet Gupta <vineetg@rivosinc.com>
> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> Tested-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/asm/vector.h | 26 ++++++++++++++++++++++++++
> arch/riscv/include/uapi/asm/hwcap.h | 1 +
> arch/riscv/kernel/cpufeature.c | 11 +++++++++++
> 4 files changed, 39 insertions(+)
> create mode 100644 arch/riscv/include/asm/vector.h
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e0c40a4c63d5..574385930ba7 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -22,6 +22,7 @@
> #define RISCV_ISA_EXT_m ('m' - 'a')
> #define RISCV_ISA_EXT_s ('s' - 'a')
> #define RISCV_ISA_EXT_u ('u' - 'a')
> +#define RISCV_ISA_EXT_v ('v' - 'a')
>
> /*
> * These macros represent the logical IDs of each multi-letter RISC-V ISA
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> new file mode 100644
> index 000000000000..427a3b51df72
> --- /dev/null
> +++ b/arch/riscv/include/asm/vector.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020 SiFive
> + */
> +
> +#ifndef __ASM_RISCV_VECTOR_H
> +#define __ASM_RISCV_VECTOR_H
> +
> +#include <linux/types.h>
> +
> +#ifdef CONFIG_RISCV_ISA_V
> +
> +#include <asm/hwcap.h>
> +
> +static __always_inline bool has_vector(void)
> +{
> + return riscv_has_extension_likely(RISCV_ISA_EXT_v);

Nothing publicly availiable has V yet, so it's not likely.

> +}
> +
> +#else /* ! CONFIG_RISCV_ISA_V */
> +
> +static __always_inline bool has_vector(void) { return false; }
> +
> +#endif /* CONFIG_RISCV_ISA_V */
> +
> +#endif /* ! __ASM_RISCV_VECTOR_H */
> diff --git a/arch/riscv/include/uapi/asm/hwcap.h b/arch/riscv/include/uapi/asm/hwcap.h
> index 46dc3f5ee99f..c52bb7bbbabe 100644
> --- a/arch/riscv/include/uapi/asm/hwcap.h
> +++ b/arch/riscv/include/uapi/asm/hwcap.h
> @@ -21,5 +21,6 @@
> #define COMPAT_HWCAP_ISA_F (1 << ('F' - 'A'))
> #define COMPAT_HWCAP_ISA_D (1 << ('D' - 'A'))
> #define COMPAT_HWCAP_ISA_C (1 << ('C' - 'A'))
> +#define COMPAT_HWCAP_ISA_V (1 << ('V' - 'A'))
>
> #endif /* _UAPI_ASM_RISCV_HWCAP_H */
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b1d6b7e4b829..7aaf92fff64e 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -107,6 +107,7 @@ void __init riscv_fill_hwcap(void)
> isa2hwcap['f' - 'a'] = COMPAT_HWCAP_ISA_F;
> isa2hwcap['d' - 'a'] = COMPAT_HWCAP_ISA_D;
> isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
> + isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;

IMO it's OK to provide V in hwcap, as there is a "V" extension defined
(unlike "B", for example).

>
> elf_hwcap = 0;
>
> @@ -267,6 +268,16 @@ void __init riscv_fill_hwcap(void)
> elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
> }
>
> + 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))

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>