diff mbox series

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

Message ID 20230125142056.18356-3-andy.chiu@sifive.com (mailing list archive)
State Superseded
Delegated to: Palmer Dabbelt
Headers show
Series riscv: Add vector ISA support | expand

Checks

Context Check Description
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 13 and now 13
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 2050 this patch: 2050
conchuod/alphanumeric_selects success Out of order selects before the patch: 57 and now 57
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 2 this patch: 2
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
conchuod/source_inline success Was 0 now: 0
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Andy Chiu Jan. 25, 2023, 2:20 p.m. UTC
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

Comments

Conor Dooley Jan. 25, 2023, 9:33 p.m. UTC | #1
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.
Guo Ren Jan. 28, 2023, 7:09 a.m. UTC | #2
On Thu, Jan 26, 2023 at 5:33 AM Conor Dooley <conor@kernel.org> wrote:
>
> 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
Has that patch merged? It could be solved during the rebase for-next naturally.

>
>
> > @@ -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
       if (elf_hwcap & COMPAT_HWCAP_ISA_V && !IS_ENABLED(CONFIG_RISCV_ISA_V)) {

right?
>
> 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.
>
Conor Dooley Jan. 28, 2023, 10:28 a.m. UTC | #3
On 28 January 2023 07:09:18 GMT, Guo Ren <guoren@kernel.org> wrote:
>On Thu, Jan 26, 2023 at 5:33 AM Conor Dooley <conor@kernel.org> wrote:
>>
>> 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
>Has that patch merged? It could be solved during the rebase for-next naturally.

Not merged yet. Pretty sure Andy used for-next
as his base so that CI could test it more easily
I was just pointing out it's existence in case he
hadn't seen it.
Hopefully Jishengs stuff will make 6.3 :)

>
>>
>>
>> > @@ -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
>       if (elf_hwcap & COMPAT_HWCAP_ISA_V && !IS_ENABLED(CONFIG_RISCV_ISA_V)) {
>
>right?
>>
>> 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.
>>
>
>
diff mbox series

Patch

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' */
 	RISCV_ISA_EXT_KEY_ZIHINTPAUSE,
 	RISCV_ISA_EXT_KEY_MAX,
 };
@@ -95,6 +97,8 @@  static __always_inline int riscv_isa_ext2key(int num)
 		return RISCV_ISA_EXT_KEY_FPU;
 	case RISCV_ISA_EXT_SVINVAL:
 		return RISCV_ISA_EXT_KEY_SVINVAL;
+	case RISCV_ISA_EXT_v:
+		return RISCV_ISA_EXT_KEY_VECTOR;
 	case RISCV_ISA_EXT_ZIHINTPAUSE:
 		return RISCV_ISA_EXT_KEY_ZIHINTPAUSE;
 	default:
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
new file mode 100644
index 000000000000..917c8867e702
--- /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 static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_VECTOR]);
+}
+
+#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 dde0e91d7668..c433899542ff 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -101,6 +101,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;
 
 	elf_hwcap = 0;
 
@@ -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
+	}
+
 	memset(print_str, 0, sizeof(print_str));
 	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
 		if (riscv_isa[0] & BIT_MASK(i))