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

From: Conor Dooley <conor@kernel.org>
To: Andy Chiu <andy.chiu@sifive.com>
Cc: linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	anup@brainfault.org, atishp@atishpatra.org,
	kvm-riscv@lists.infradead.org, kvm@vger.kernel.org,
	vineetg@rivosinc.com, greentime.hu@sifive.com,
	guoren@linux.alibaba.com, Guo Ren <ren_guo@c-sky.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Heiko Stuebner <heiko@sntech.de>,
	Atish Patra <atishp@rivosinc.com>,
	Anup Patel <apatel@ventanamicro.com>, Guo Ren <guoren@kernel.org>,
	Mayuresh Chitale <mchitale@ventanamicro.com>,
	Conor Dooley <conor.dooley@microchip.com>,
	Dao Lu <daolu@rivosinc.com>, Jisheng Zhang <jszhang@kernel.org>,
	Andrew Jones <ajones@ventanamicro.com>,
	Vincent Chen <vincent.chen@sifive.com>,
	Tsukasa OI <research_trasio@irq.a4lg.com>
Subject: Re: [PATCH -next v13 02/19] riscv: Extending cpufeature.c to detect V-extension
Date: Wed, 25 Jan 2023 21:33:30 +0000	[thread overview]
Message-ID: <Y9GgKiF8q2k9eRdh@spud> (raw)
In-Reply-To: <20230125142056.18356-3-andy.chiu@sifive.com>

[-- Attachment #1: Type: text/plain, Size: 2545 bytes --]

On Wed, Jan 25, 2023 at 02:20:39PM +0000, Andy Chiu 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>
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>  arch/riscv/include/asm/hwcap.h      |  4 ++++
>  arch/riscv/include/asm/vector.h     | 26 ++++++++++++++++++++++++++
>  arch/riscv/include/uapi/asm/hwcap.h |  1 +
>  arch/riscv/kernel/cpufeature.c      | 12 ++++++++++++
>  4 files changed, 43 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 57439da71c77..f413db6118e5 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -35,6 +35,7 @@ extern unsigned long elf_hwcap;
>  #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')
>  
>  /*
>   * Increse this to higher value as kernel support more ISA extensions.
> @@ -73,6 +74,7 @@ static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
>  enum riscv_isa_ext_key {
>  	RISCV_ISA_EXT_KEY_FPU,		/* For 'F' and 'D' */
>  	RISCV_ISA_EXT_KEY_SVINVAL,
> +	RISCV_ISA_EXT_KEY_VECTOR,	/* For 'V' */

That's obvious surely, no?

>  	RISCV_ISA_EXT_KEY_ZIHINTPAUSE,
>  	RISCV_ISA_EXT_KEY_MAX,
>  };
> @@ -95,6 +97,8 @@ static __always_inline int riscv_isa_ext2key(int num)

You should probably check out Jisheng's series that deletes whole
sections of this code, including this whole function.
https://lore.kernel.org/all/20230115154953.831-3-jszhang@kernel.org/T/#u

> @@ -256,6 +257,17 @@ void __init riscv_fill_hwcap(void)
>  		elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
>  	}
>  
> +	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
> +#ifndef CONFIG_RISCV_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.
> +		 */
> +		elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> +#endif

I know that a later patch in this series calls rvv_enable() here, which
I'll comment on there, but I'd rather see IS_ENABLED as opposed to
ifdefs in C files where possible.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]