diff mbox series

[v2,2/5] RISC-V: Add riscv_isa reprensenting ISA features common across CPUs

Message ID 20190731012418.24565-3-atish.patra@wdc.com (mailing list archive)
State New, archived
Headers show
Series Miscellaneous fixes | expand

Commit Message

Atish Patra July 31, 2019, 1:24 a.m. UTC 
From: Anup Patel <anup.patel@wdc.com>

This patch adds riscv_isa integer to represent ISA features common
across all CPUs. The riscv_isa is not same as elf_hwcap because
elf_hwcap will only have ISA features relevant for user-space apps
whereas riscv_isa will have ISA features relevant to both kernel
and user-space apps.

One of the use case is KVM hypervisor where riscv_isa will be used
to do following operations:

1. Check whether hypervisor extension is available
2. Find ISA features that need to be virtualized (e.g. floating
   point support, vector extension, etc.)

Signed-off-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/include/asm/hwcap.h | 25 +++++++++++++++++++++
 arch/riscv/kernel/cpufeature.c | 41 +++++++++++++++++++++++++++++++---
 2 files changed, 63 insertions(+), 3 deletions(-)

Comments

Paul Walmsley July 31, 2019, 4:23 a.m. UTC | #1 
On Tue, 30 Jul 2019, Atish Patra wrote:

> From: Anup Patel <anup.patel@wdc.com>
> 
> This patch adds riscv_isa integer to represent ISA features common
> across all CPUs. The riscv_isa is not same as elf_hwcap because
> elf_hwcap will only have ISA features relevant for user-space apps
> whereas riscv_isa will have ISA features relevant to both kernel
> and user-space apps.
> 
> One of the use case is KVM hypervisor where riscv_isa will be used
> to do following operations:
> 
> 1. Check whether hypervisor extension is available
> 2. Find ISA features that need to be virtualized (e.g. floating
>    point support, vector extension, etc.)
> 
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/include/asm/hwcap.h | 25 +++++++++++++++++++++
>  arch/riscv/kernel/cpufeature.c | 41 +++++++++++++++++++++++++++++++---
>  2 files changed, 63 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 7ecb7c6a57b1..e069f60ad5d2 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -22,5 +22,30 @@ enum {
>  };
>  
>  extern unsigned long elf_hwcap;
> +
> +#define RISCV_ISA_EXT_A		(1UL << ('A' - 'A'))

Are these uppercase variants still needed if we define the ISA string to 
be all lowercase, per our recent discussion?

> +#define RISCV_ISA_EXT_a		RISCV_ISA_EXT_A
> +#define RISCV_ISA_EXT_C		(1UL << ('C' - 'A'))
> +#define RISCV_ISA_EXT_c		RISCV_ISA_EXT_C
> +#define RISCV_ISA_EXT_D		(1UL << ('D' - 'A'))
> +#define RISCV_ISA_EXT_d		RISCV_ISA_EXT_D
> +#define RISCV_ISA_EXT_F		(1UL << ('F' - 'A'))
> +#define RISCV_ISA_EXT_f		RISCV_ISA_EXT_F
> +#define RISCV_ISA_EXT_H		(1UL << ('H' - 'A'))
> +#define RISCV_ISA_EXT_h		RISCV_ISA_EXT_H
> +#define RISCV_ISA_EXT_I		(1UL << ('I' - 'A'))
> +#define RISCV_ISA_EXT_i		RISCV_ISA_EXT_I
> +#define RISCV_ISA_EXT_M		(1UL << ('M' - 'A'))
> +#define RISCV_ISA_EXT_m		RISCV_ISA_EXT_M
> +#define RISCV_ISA_EXT_S		(1UL << ('S' - 'A'))
> +#define RISCV_ISA_EXT_s		RISCV_ISA_EXT_S
> +#define RISCV_ISA_EXT_U		(1UL << ('U' - 'A'))
> +#define RISCV_ISA_EXT_u		RISCV_ISA_EXT_U
> +
> +extern unsigned long riscv_isa;
> +
> +#define riscv_isa_extension_available(ext_char)	\
> +		(riscv_isa & RISCV_ISA_EXT_##ext_char)
> +
>  #endif
>  #endif
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b1ade9a49347..177529d48d87 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c

[ ... ]

> @@ -43,8 +49,22 @@ void riscv_fill_hwcap(void)
>  			continue;
>  		}
>  
> -		for (i = 0; i < strlen(isa); ++i)
> +		i = 0;
> +		isa_len = strlen(isa);
> +#if defined(CONFIG_32BIT)
> +		if (strncasecmp(isa, "rv32", 4) != 0)

strcmp()?

> +			i += 4;
> +#elif defined(CONFIG_64BIT)
> +		if (strncasecmp(isa, "rv64", 4) != 0)

And again here?

> +			i += 4;
> +#endif
> +		for (; i < isa_len; ++i) {
>  			this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
> +			if ('a' <= isa[i] && isa[i] <= 'z')
> +				this_isa |= (1UL << (isa[i] - 'a'));
> +			if ('A' <= isa[i] && isa[i] <= 'Z')
> +				this_isa |= (1UL << (isa[i] - 'A'));

Are these uppercase variants still needed?


- Paul
Atish Patra July 31, 2019, 6:27 a.m. UTC | #2 
On 7/30/19, 9:23 PM, "Paul Walmsley" <paul.walmsley@sifive.com> wrote:

    On Tue, 30 Jul 2019, Atish Patra wrote:
    
    > From: Anup Patel <anup.patel@wdc.com>
    > 
    > This patch adds riscv_isa integer to represent ISA features common
    > across all CPUs. The riscv_isa is not same as elf_hwcap because
    > elf_hwcap will only have ISA features relevant for user-space apps
    > whereas riscv_isa will have ISA features relevant to both kernel
    > and user-space apps.
    > 
    > One of the use case is KVM hypervisor where riscv_isa will be used
    > to do following operations:
    > 
    > 1. Check whether hypervisor extension is available
    > 2. Find ISA features that need to be virtualized (e.g. floating
    >    point support, vector extension, etc.)
    > 
    > Signed-off-by: Anup Patel <anup.patel@wdc.com>
    > Signed-off-by: Atish Patra <atish.patra@wdc.com>
    > ---
    >  arch/riscv/include/asm/hwcap.h | 25 +++++++++++++++++++++
    >  arch/riscv/kernel/cpufeature.c | 41 +++++++++++++++++++++++++++++++---
    >  2 files changed, 63 insertions(+), 3 deletions(-)
    > 
    > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
    > index 7ecb7c6a57b1..e069f60ad5d2 100644
    > --- a/arch/riscv/include/asm/hwcap.h
    > +++ b/arch/riscv/include/asm/hwcap.h
    > @@ -22,5 +22,30 @@ enum {
    >  };
    >  
    >  extern unsigned long elf_hwcap;
    > +
    > +#define RISCV_ISA_EXT_A		(1UL << ('A' - 'A'))
    
    Are these uppercase variants still needed if we define the ISA string to 
    be all lowercase, per our recent discussion?
    
Argh. Sorry. We have been carrying this patch so long that I completely forgot about the
case sensitive usage here.

    > +#define RISCV_ISA_EXT_a		RISCV_ISA_EXT_A
    > +#define RISCV_ISA_EXT_C		(1UL << ('C' - 'A'))
    > +#define RISCV_ISA_EXT_c		RISCV_ISA_EXT_C
    > +#define RISCV_ISA_EXT_D		(1UL << ('D' - 'A'))
    > +#define RISCV_ISA_EXT_d		RISCV_ISA_EXT_D
    > +#define RISCV_ISA_EXT_F		(1UL << ('F' - 'A'))
    > +#define RISCV_ISA_EXT_f		RISCV_ISA_EXT_F
    > +#define RISCV_ISA_EXT_H		(1UL << ('H' - 'A'))
    > +#define RISCV_ISA_EXT_h		RISCV_ISA_EXT_H
    > +#define RISCV_ISA_EXT_I		(1UL << ('I' - 'A'))
    > +#define RISCV_ISA_EXT_i		RISCV_ISA_EXT_I
    > +#define RISCV_ISA_EXT_M		(1UL << ('M' - 'A'))
    > +#define RISCV_ISA_EXT_m		RISCV_ISA_EXT_M
    > +#define RISCV_ISA_EXT_S		(1UL << ('S' - 'A'))
    > +#define RISCV_ISA_EXT_s		RISCV_ISA_EXT_S
    > +#define RISCV_ISA_EXT_U		(1UL << ('U' - 'A'))
    > +#define RISCV_ISA_EXT_u		RISCV_ISA_EXT_U
    > +
    > +extern unsigned long riscv_isa;
    > +
    > +#define riscv_isa_extension_available(ext_char)	\
    > +		(riscv_isa & RISCV_ISA_EXT_##ext_char)
    > +
    >  #endif
    >  #endif
    > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
    > index b1ade9a49347..177529d48d87 100644
    > --- a/arch/riscv/kernel/cpufeature.c
    > +++ b/arch/riscv/kernel/cpufeature.c
    
    [ ... ]
    
    > @@ -43,8 +49,22 @@ void riscv_fill_hwcap(void)
    >  			continue;
    >  		}
    >  
    > -		for (i = 0; i < strlen(isa); ++i)
    > +		i = 0;
    > +		isa_len = strlen(isa);
    > +#if defined(CONFIG_32BIT)
    > +		if (strncasecmp(isa, "rv32", 4) != 0)
    
    strcmp()?
    
    > +			i += 4;
    > +#elif defined(CONFIG_64BIT)
    > +		if (strncasecmp(isa, "rv64", 4) != 0)
    
    And again here?
    
    > +			i += 4;
    > +#endif
    > +		for (; i < isa_len; ++i) {
    >  			this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
    > +			if ('a' <= isa[i] && isa[i] <= 'z')
    > +				this_isa |= (1UL << (isa[i] - 'a'));
    > +			if ('A' <= isa[i] && isa[i] <= 'Z')
    > +				this_isa |= (1UL << (isa[i] - 'A'));
    
    Are these uppercase variants still needed?
    
Nope. Same as above comment. Apologies for forgetting about these usages.
I will send a v3 removing them.
 
Regards,
Atish   
    - Paul
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 7ecb7c6a57b1..e069f60ad5d2 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -22,5 +22,30 @@  enum {
 };
 
 extern unsigned long elf_hwcap;
+
+#define RISCV_ISA_EXT_A		(1UL << ('A' - 'A'))
+#define RISCV_ISA_EXT_a		RISCV_ISA_EXT_A
+#define RISCV_ISA_EXT_C		(1UL << ('C' - 'A'))
+#define RISCV_ISA_EXT_c		RISCV_ISA_EXT_C
+#define RISCV_ISA_EXT_D		(1UL << ('D' - 'A'))
+#define RISCV_ISA_EXT_d		RISCV_ISA_EXT_D
+#define RISCV_ISA_EXT_F		(1UL << ('F' - 'A'))
+#define RISCV_ISA_EXT_f		RISCV_ISA_EXT_F
+#define RISCV_ISA_EXT_H		(1UL << ('H' - 'A'))
+#define RISCV_ISA_EXT_h		RISCV_ISA_EXT_H
+#define RISCV_ISA_EXT_I		(1UL << ('I' - 'A'))
+#define RISCV_ISA_EXT_i		RISCV_ISA_EXT_I
+#define RISCV_ISA_EXT_M		(1UL << ('M' - 'A'))
+#define RISCV_ISA_EXT_m		RISCV_ISA_EXT_M
+#define RISCV_ISA_EXT_S		(1UL << ('S' - 'A'))
+#define RISCV_ISA_EXT_s		RISCV_ISA_EXT_S
+#define RISCV_ISA_EXT_U		(1UL << ('U' - 'A'))
+#define RISCV_ISA_EXT_u		RISCV_ISA_EXT_U
+
+extern unsigned long riscv_isa;
+
+#define riscv_isa_extension_available(ext_char)	\
+		(riscv_isa & RISCV_ISA_EXT_##ext_char)
+
 #endif
 #endif
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b1ade9a49347..177529d48d87 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -12,6 +12,9 @@ 
 #include <asm/smp.h>
 
 unsigned long elf_hwcap __read_mostly;
+unsigned long riscv_isa __read_mostly;
+EXPORT_SYMBOL_GPL(riscv_isa);
+
 #ifdef CONFIG_FPU
 bool has_fpu __read_mostly;
 #endif
@@ -20,7 +23,8 @@  void riscv_fill_hwcap(void)
 {
 	struct device_node *node;
 	const char *isa;
-	size_t i;
+	char print_str[BITS_PER_LONG+1];
+	size_t i, j, isa_len;
 	static unsigned long isa2hwcap[256] = {0};
 
 	isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
@@ -31,9 +35,11 @@  void riscv_fill_hwcap(void)
 	isa2hwcap['c'] = isa2hwcap['C'] = COMPAT_HWCAP_ISA_C;
 
 	elf_hwcap = 0;
+	riscv_isa = 0;
 
 	for_each_of_cpu_node(node) {
 		unsigned long this_hwcap = 0;
+		unsigned long this_isa = 0;
 
 		if (riscv_of_processor_hartid(node) < 0)
 			continue;
@@ -43,8 +49,22 @@  void riscv_fill_hwcap(void)
 			continue;
 		}
 
-		for (i = 0; i < strlen(isa); ++i)
+		i = 0;
+		isa_len = strlen(isa);
+#if defined(CONFIG_32BIT)
+		if (strncasecmp(isa, "rv32", 4) != 0)
+			i += 4;
+#elif defined(CONFIG_64BIT)
+		if (strncasecmp(isa, "rv64", 4) != 0)
+			i += 4;
+#endif
+		for (; i < isa_len; ++i) {
 			this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
+			if ('a' <= isa[i] && isa[i] <= 'z')
+				this_isa |= (1UL << (isa[i] - 'a'));
+			if ('A' <= isa[i] && isa[i] <= 'Z')
+				this_isa |= (1UL << (isa[i] - 'A'));
+		}
 
 		/*
 		 * All "okay" hart should have same isa. Set HWCAP based on
@@ -55,6 +75,11 @@  void riscv_fill_hwcap(void)
 			elf_hwcap &= this_hwcap;
 		else
 			elf_hwcap = this_hwcap;
+
+		if (riscv_isa)
+			riscv_isa &= this_isa;
+		else
+			riscv_isa = this_isa;
 	}
 
 	/* We don't support systems with F but without D, so mask those out
@@ -64,7 +89,17 @@  void riscv_fill_hwcap(void)
 		elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
 	}
 
-	pr_info("elf_hwcap is 0x%lx\n", elf_hwcap);
+	memset(print_str, 0, sizeof(print_str));
+	for (i = 0, j = 0; i < BITS_PER_LONG; i++)
+		if (riscv_isa & (1UL << i))
+			print_str[j++] = (char)('A' + i);
+	pr_info("riscv: ISA extensions %s\n", print_str);
+
+	memset(print_str, 0, sizeof(print_str));
+	for (i = 0, j = 0; i < BITS_PER_LONG; i++)
+		if (elf_hwcap & (1UL << i))
+			print_str[j++] = (char)('A' + i);
+	pr_info("riscv: ELF capabilities %s\n", print_str);
 
 #ifdef CONFIG_FPU
 	if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))