diff mbox series

[v3,4/6] RISC-V: Implement multi-letter ISA extension probing framework

Message ID 20220215090211.911366-5-atishp@rivosinc.com (mailing list archive)
State New, archived
Headers show
Series Provide a fraemework for RISC-V ISA extensions | expand

Commit Message

Atish Patra Feb. 15, 2022, 9:02 a.m. UTC
Multi-letter extensions can be probed using exising
riscv_isa_extension_available API now. It doesn't support versioning
right now as there is no use case for it.
Individual extension specific implementation will be added during
each extension support.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/include/asm/hwcap.h | 18 ++++++++++++++++++
 arch/riscv/kernel/cpufeature.c | 27 ++++++++++++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

Comments

Anup Patel Feb. 15, 2022, 10:23 a.m. UTC | #1
On Tue, Feb 15, 2022 at 2:32 PM Atish Patra <atishp@rivosinc.com> wrote:
>
> Multi-letter extensions can be probed using exising
> riscv_isa_extension_available API now. It doesn't support versioning
> right now as there is no use case for it.
> Individual extension specific implementation will be added during
> each extension support.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  arch/riscv/include/asm/hwcap.h | 18 ++++++++++++++++++
>  arch/riscv/kernel/cpufeature.c | 27 ++++++++++++++++++++++++---
>  2 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5ce50468aff1..170bd80da520 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -34,7 +34,25 @@ extern unsigned long elf_hwcap;
>  #define RISCV_ISA_EXT_s                ('s' - 'a')
>  #define RISCV_ISA_EXT_u                ('u' - 'a')
>
> +/*
> + * Increse this to higher value as kernel support more ISA extensions.
> + */
>  #define RISCV_ISA_EXT_MAX      64
> +#define RISCV_ISA_EXT_NAME_LEN_MAX 32
> +
> +/* The base ID for multi-letter ISA extensions */
> +#define RISCV_ISA_EXT_BASE 26
> +
> +/*
> + * This enum represent the logical ID for each multi-letter RISC-V ISA extension.
> + * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed
> + * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter
> + * extensions while all the multi-letter extensions should define the next
> + * available logical extension id.
> + */
> +enum riscv_isa_ext_id {
> +       RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> +};
>
>  unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index cd9eb34f8d11..af9a57ad3d4e 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -83,7 +83,7 @@ void __init riscv_fill_hwcap(void)
>
>         for_each_of_cpu_node(node) {
>                 unsigned long this_hwcap = 0;
> -               unsigned long this_isa = 0;
> +               uint64_t this_isa = 0;

Why not use a bitmap here ?

>
>                 if (riscv_of_processor_hartid(node) < 0)
>                         continue;
> @@ -167,12 +167,22 @@ void __init riscv_fill_hwcap(void)
>                         if (*isa != '_')
>                                 --isa;
>
> +#define SET_ISA_EXT_MAP(name, bit)                                             \

Where is this macro used ?

> +                       do {                                                    \
> +                               if ((ext_end - ext == sizeof(name) - 1) &&      \
> +                                    !memcmp(ext, name, sizeof(name) - 1)) {    \
> +                                       this_isa |= (1UL << bit);               \

You can use set_bit() here when using bitmap.

> +                                       pr_info("Found ISA extension %s", name);\
> +                               }                                               \
> +                       } while (false)                                         \
> +
>                         if (unlikely(ext_err))
>                                 continue;
>                         if (!ext_long) {
>                                 this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
>                                 this_isa |= (1UL << (*ext - 'a'));
>                         }
> +#undef SET_ISA_EXT_MAP
>                 }
>
>                 /*
> @@ -185,10 +195,21 @@ void __init riscv_fill_hwcap(void)
>                 else
>                         elf_hwcap = this_hwcap;
>
> -               if (riscv_isa[0])
> +               if (riscv_isa[0]) {

You can use bitmap_weight() here

> +#if IS_ENABLED(CONFIG_32BIT)
> +                       riscv_isa[0] &= this_isa & 0xFFFFFFFF;
> +                       riscv_isa[1] &= this_isa >> 32;
> +#else
>                         riscv_isa[0] &= this_isa;
> -               else
> +#endif
> +               } else {
> +#if IS_ENABLED(CONFIG_32BIT)
> +                       riscv_isa[0] = this_isa & 0xFFFFFFFF;
> +                       riscv_isa[1] = this_isa >> 32;
> +#else
>                         riscv_isa[0] = this_isa;
> +#endif
> +               }
>         }
>
>         /* We don't support systems with F but without D, so mask those out
> --
> 2.30.2
>

Regards,
Anup
Atish Patra Feb. 15, 2022, 11:11 p.m. UTC | #2
On Tue, Feb 15, 2022 at 2:24 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Feb 15, 2022 at 2:32 PM Atish Patra <atishp@rivosinc.com> wrote:
> >
> > Multi-letter extensions can be probed using exising
> > riscv_isa_extension_available API now. It doesn't support versioning
> > right now as there is no use case for it.
> > Individual extension specific implementation will be added during
> > each extension support.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/hwcap.h | 18 ++++++++++++++++++
> >  arch/riscv/kernel/cpufeature.c | 27 ++++++++++++++++++++++++---
> >  2 files changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 5ce50468aff1..170bd80da520 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -34,7 +34,25 @@ extern unsigned long elf_hwcap;
> >  #define RISCV_ISA_EXT_s                ('s' - 'a')
> >  #define RISCV_ISA_EXT_u                ('u' - 'a')
> >
> > +/*
> > + * Increse this to higher value as kernel support more ISA extensions.
> > + */
> >  #define RISCV_ISA_EXT_MAX      64
> > +#define RISCV_ISA_EXT_NAME_LEN_MAX 32
> > +
> > +/* The base ID for multi-letter ISA extensions */
> > +#define RISCV_ISA_EXT_BASE 26
> > +
> > +/*
> > + * This enum represent the logical ID for each multi-letter RISC-V ISA extension.
> > + * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed
> > + * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter
> > + * extensions while all the multi-letter extensions should define the next
> > + * available logical extension id.
> > + */
> > +enum riscv_isa_ext_id {
> > +       RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> > +};
> >
> >  unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index cd9eb34f8d11..af9a57ad3d4e 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -83,7 +83,7 @@ void __init riscv_fill_hwcap(void)
> >
> >         for_each_of_cpu_node(node) {
> >                 unsigned long this_hwcap = 0;
> > -               unsigned long this_isa = 0;
> > +               uint64_t this_isa = 0;
>
> Why not use a bitmap here ?
>

Yeah. That will simplify things for both RV32 & RV64. Thanks.

> >
> >                 if (riscv_of_processor_hartid(node) < 0)
> >                         continue;
> > @@ -167,12 +167,22 @@ void __init riscv_fill_hwcap(void)
> >                         if (*isa != '_')
> >                                 --isa;
> >
> > +#define SET_ISA_EXT_MAP(name, bit)                                             \
>
> Where is this macro used ?

It will be used in the future where individual extension support will use it.
Here is an example from my debug patch

https://github.com/atishp04/linux/commit/e9e240c9a854dceb434ceb53bdbe82a657bee5f2

>
> > +                       do {                                                    \
> > +                               if ((ext_end - ext == sizeof(name) - 1) &&      \
> > +                                    !memcmp(ext, name, sizeof(name) - 1)) {    \
> > +                                       this_isa |= (1UL << bit);               \
>
> You can use set_bit() here when using bitmap.
>
> > +                                       pr_info("Found ISA extension %s", name);\
> > +                               }                                               \
> > +                       } while (false)                                         \
> > +
> >                         if (unlikely(ext_err))
> >                                 continue;
> >                         if (!ext_long) {
> >                                 this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
> >                                 this_isa |= (1UL << (*ext - 'a'));
> >                         }
> > +#undef SET_ISA_EXT_MAP
> >                 }
> >
> >                 /*
> > @@ -185,10 +195,21 @@ void __init riscv_fill_hwcap(void)
> >                 else
> >                         elf_hwcap = this_hwcap;
> >
> > -               if (riscv_isa[0])
> > +               if (riscv_isa[0]) {
>
> You can use bitmap_weight() here
>
> > +#if IS_ENABLED(CONFIG_32BIT)
> > +                       riscv_isa[0] &= this_isa & 0xFFFFFFFF;
> > +                       riscv_isa[1] &= this_isa >> 32;
> > +#else
> >                         riscv_isa[0] &= this_isa;
> > -               else
> > +#endif
> > +               } else {
> > +#if IS_ENABLED(CONFIG_32BIT)
> > +                       riscv_isa[0] = this_isa & 0xFFFFFFFF;
> > +                       riscv_isa[1] = this_isa >> 32;
> > +#else
> >                         riscv_isa[0] = this_isa;
> > +#endif
> > +               }
> >         }
> >
> >         /* We don't support systems with F but without D, so mask those out
> > --
> > 2.30.2
> >
>
> Regards,
> Anup
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 5ce50468aff1..170bd80da520 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -34,7 +34,25 @@  extern unsigned long elf_hwcap;
 #define RISCV_ISA_EXT_s		('s' - 'a')
 #define RISCV_ISA_EXT_u		('u' - 'a')
 
+/*
+ * Increse this to higher value as kernel support more ISA extensions.
+ */
 #define RISCV_ISA_EXT_MAX	64
+#define RISCV_ISA_EXT_NAME_LEN_MAX 32
+
+/* The base ID for multi-letter ISA extensions */
+#define RISCV_ISA_EXT_BASE 26
+
+/*
+ * This enum represent the logical ID for each multi-letter RISC-V ISA extension.
+ * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed
+ * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter
+ * extensions while all the multi-letter extensions should define the next
+ * available logical extension id.
+ */
+enum riscv_isa_ext_id {
+	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
+};
 
 unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
 
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index cd9eb34f8d11..af9a57ad3d4e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -83,7 +83,7 @@  void __init riscv_fill_hwcap(void)
 
 	for_each_of_cpu_node(node) {
 		unsigned long this_hwcap = 0;
-		unsigned long this_isa = 0;
+		uint64_t this_isa = 0;
 
 		if (riscv_of_processor_hartid(node) < 0)
 			continue;
@@ -167,12 +167,22 @@  void __init riscv_fill_hwcap(void)
 			if (*isa != '_')
 				--isa;
 
+#define SET_ISA_EXT_MAP(name, bit)						\
+			do {							\
+				if ((ext_end - ext == sizeof(name) - 1) &&	\
+				     !memcmp(ext, name, sizeof(name) - 1)) {    \
+					this_isa |= (1UL << bit);		\
+					pr_info("Found ISA extension %s", name);\
+				}						\
+			} while (false)						\
+
 			if (unlikely(ext_err))
 				continue;
 			if (!ext_long) {
 				this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
 				this_isa |= (1UL << (*ext - 'a'));
 			}
+#undef SET_ISA_EXT_MAP
 		}
 
 		/*
@@ -185,10 +195,21 @@  void __init riscv_fill_hwcap(void)
 		else
 			elf_hwcap = this_hwcap;
 
-		if (riscv_isa[0])
+		if (riscv_isa[0]) {
+#if IS_ENABLED(CONFIG_32BIT)
+			riscv_isa[0] &= this_isa & 0xFFFFFFFF;
+			riscv_isa[1] &= this_isa >> 32;
+#else
 			riscv_isa[0] &= this_isa;
-		else
+#endif
+		} else {
+#if IS_ENABLED(CONFIG_32BIT)
+			riscv_isa[0] = this_isa & 0xFFFFFFFF;
+			riscv_isa[1] = this_isa >> 32;
+#else
 			riscv_isa[0] = this_isa;
+#endif
+		}
 	}
 
 	/* We don't support systems with F but without D, so mask those out