linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions
@ 2022-02-15  9:02 Atish Patra
  2022-02-15  9:02 ` [PATCH v3 1/6] RISC-V: Correctly print supported extensions Atish Patra
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Atish Patra @ 2022-02-15  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Atish Patra, Anup Patel, Damien Le Moal,
	devicetree, Jisheng Zhang, Krzysztof Kozlowski, linux-riscv,
	Palmer Dabbelt, Paul Walmsley, Rob Herring

This series implements a generic framework to parse multi-letter ISA
extensions. This series is based on Tsukasa's v3 isa extension improvement
series[1]. I have fixed few bugs and improved comments from that series
(PATCH1-3). I have not used PATCH 4 from that series as we are not using
ISA extension versioning as of now. We can add that later if required.

PATCH 4 allows the probing of multi-letter extensions via a macro.
It continues to use the common isa extensions between all the harts.
Thus hetergenous hart systems will only see the common ISA extensions.

PATCH 6 improves the /proc/cpuinfo interface for the available ISA extensions
via /proc/cpuinfo.

Here is the example output of /proc/cpuinfo:
(with debug patches in Qemu and Linux kernel)

/ # cat /proc/cpuinfo
processor	: 0
hart		: 0
isa		: rv64imafdcsu
isa-ext		: sstc,sscofpmf
mmu		: sv48

processor	: 1
hart		: 1
isa		: rv64imafdcsu
isa-ext		: sstc,sscofpmf
mmu		: sv48

processor	: 2
hart		: 2
isa		: rv64imafdcsu
isa-ext		: sstc,sscofpmf
mmu		: sv48

processor	: 3
hart		: 3
isa		: rv64imafdcsu
isa-ext		: sstc,sscofpmf
mmu		: sv48

Anybody adding support for any new multi-letter extensions should add an
entry to the riscv_isa_ext_id and the isa extension array. 
E.g. The patch[2] adds the support for various ISA extensions.

[1] https://lore.kernel.org/all/0f568515-a05e-8204-aae3-035975af3ee8@irq.a4lg.com/T/
[2] https://github.com/atishp04/linux/commit/dc6f9200033bb5a72d8fd1a179bb272c6ade17e6 


Changes from v2->v3:
1. Updated comments to mark clearly a fix required for Qemu only.
2. Fixed a bug where the 1st multi-letter extension can be present without _
3. Added Tested by tags. 

Changes from v1->v2:
1. Instead of adding a separate DT property use the riscv,isa property.
2. Based on Tsukasa's v3 isa extension improvement series.

Atish Patra (3):
RISC-V: Implement multi-letter ISA extension probing framework
RISC-V: Do no continue isa string parsing without correct XLEN
RISC-V: Improve /proc/cpuinfo output for ISA extensions

Tsukasa OI (3):
RISC-V: Correctly print supported extensions
RISC-V: Minimal parser for "riscv, isa" strings
RISC-V: Extract multi-letter extension names from "riscv, isa"

arch/riscv/include/asm/hwcap.h |  25 +++++++
arch/riscv/kernel/cpu.c        |  44 ++++++++++-
arch/riscv/kernel/cpufeature.c | 130 ++++++++++++++++++++++++++++-----
3 files changed, 178 insertions(+), 21 deletions(-)

--
2.30.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/6] RISC-V: Correctly print supported extensions
  2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
@ 2022-02-15  9:02 ` Atish Patra
  2022-02-15  9:41   ` Anup Patel
  2022-02-15  9:02 ` [PATCH v3 2/6] RISC-V: Minimal parser for "riscv, isa" strings Atish Patra
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Atish Patra @ 2022-02-15  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tsukasa OI, Atish Patra, Heiko Stuebner, Albert Ou, Atish Patra,
	Anup Patel, Damien Le Moal, devicetree, Jisheng Zhang,
	Krzysztof Kozlowski, linux-riscv, Palmer Dabbelt, Paul Walmsley,
	Rob Herring

From: Tsukasa OI <research_trasio@irq.a4lg.com>

This commit replaces BITS_PER_LONG with number of alphabet letters.

Current ISA pretty-printing code expects extension 'a' (bit 0) through
'z' (bit 25).  Although bit 26 and higher is not currently used (thus never
cause an issue in practice), it will be an annoying problem if we start to
use those in the future.

This commit disables printing high bits for now.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/riscv/kernel/cpufeature.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index d959d207a40d..dd3d57eb4eea 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -13,6 +13,8 @@
 #include <asm/smp.h>
 #include <asm/switch_to.h>
 
+#define NUM_ALPHA_EXTS ('z' - 'a' + 1)
+
 unsigned long elf_hwcap __read_mostly;
 
 /* Host ISA bitmap */
@@ -63,7 +65,7 @@ void __init riscv_fill_hwcap(void)
 {
 	struct device_node *node;
 	const char *isa;
-	char print_str[BITS_PER_LONG + 1];
+	char print_str[NUM_ALPHA_EXTS + 1];
 	size_t i, j, isa_len;
 	static unsigned long isa2hwcap[256] = {0};
 
@@ -133,13 +135,13 @@ void __init riscv_fill_hwcap(void)
 	}
 
 	memset(print_str, 0, sizeof(print_str));
-	for (i = 0, j = 0; i < BITS_PER_LONG; i++)
+	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
 		if (riscv_isa[0] & BIT_MASK(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++)
+	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
 		if (elf_hwcap & BIT_MASK(i))
 			print_str[j++] = (char)('a' + i);
 	pr_info("riscv: ELF capabilities %s\n", print_str);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/6] RISC-V: Minimal parser for "riscv, isa" strings
  2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
  2022-02-15  9:02 ` [PATCH v3 1/6] RISC-V: Correctly print supported extensions Atish Patra
@ 2022-02-15  9:02 ` Atish Patra
  2022-02-15  9:02 ` [PATCH v3 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa" Atish Patra
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-02-15  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tsukasa OI, Atish Patra, Heiko Stuebner, Albert Ou, Atish Patra,
	Anup Patel, Damien Le Moal, devicetree, Jisheng Zhang,
	Krzysztof Kozlowski, linux-riscv, Palmer Dabbelt, Paul Walmsley,
	Rob Herring

From: Tsukasa OI <research_trasio@irq.a4lg.com>

Current hart ISA ("riscv,isa") parser don't correctly parse:

1. Multi-letter extensions
2. Version numbers

All ISA extensions ratified recently has multi-letter extensions
(except 'H'). The current "riscv,isa" parser that is easily confused
by multi-letter extensions and "p" in version numbers can be a huge
problem for adding new extensions through the device tree.

Leaving it would create incompatible hacks and would make "riscv,isa"
value unreliable.

This commit implements minimal parser for "riscv,isa" strings.  With this,
we can safely ignore multi-letter extensions and version numbers.

[Improved commit text and fixed a bug around 's' in base extension]
Signed-off-by: Atish Patra <atishp@rivosinc.com>
[Fixed workaround for QEMU]
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/riscv/kernel/cpufeature.c | 66 ++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index dd3d57eb4eea..9d5448542226 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/bitmap.h>
+#include <linux/ctype.h>
 #include <linux/of.h>
 #include <asm/processor.h>
 #include <asm/hwcap.h>
@@ -66,7 +67,7 @@ void __init riscv_fill_hwcap(void)
 	struct device_node *node;
 	const char *isa;
 	char print_str[NUM_ALPHA_EXTS + 1];
-	size_t i, j, isa_len;
+	int i, j;
 	static unsigned long isa2hwcap[256] = {0};
 
 	isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
@@ -92,23 +93,66 @@ void __init riscv_fill_hwcap(void)
 			continue;
 		}
 
-		i = 0;
-		isa_len = strlen(isa);
 #if IS_ENABLED(CONFIG_32BIT)
 		if (!strncmp(isa, "rv32", 4))
-			i += 4;
+			isa += 4;
 #elif IS_ENABLED(CONFIG_64BIT)
 		if (!strncmp(isa, "rv64", 4))
-			i += 4;
+			isa += 4;
 #endif
-		for (; i < isa_len; ++i) {
-			this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
+		for (; *isa; ++isa) {
+			const char *ext = isa++;
+			const char *ext_end = isa;
+			bool ext_long = false, ext_err = false;
+
+			switch (*ext) {
+			case 's':
+			case 'x':
+			case 'z':
+				/**
+				 * Workaround for invalid single-letter 's' (QEMU).
+				 * It works until multi-letter extension starting
+				 * with "Su" appears.
+				 */
+				if (*ext == 's' && ext[-1] != '_' && ext[1] == 'u')
+					break;
+				ext_long = true;
+				/* Multi-letter extension must be delimited */
+				for (; *isa && *isa != '_'; ++isa)
+					if (!islower(*isa) && !isdigit(*isa))
+						ext_err = true;
+				/* ... but must be ignored. */
+				break;
+			default:
+				if (unlikely(!islower(*ext))) {
+					ext_err = true;
+					break;
+				}
+				/* Find next extension */
+				if (!isdigit(*isa))
+					break;
+				while (isdigit(*++isa))
+					;
+				if (*isa != 'p')
+					break;
+				if (!isdigit(*++isa)) {
+					--isa;
+					break;
+				}
+				while (isdigit(*++isa))
+					;
+				break;
+			}
+			if (*isa != '_')
+				--isa;
 			/*
-			 * TODO: X, Y and Z extension parsing for Host ISA
-			 * bitmap will be added in-future.
+			 * TODO: Full version-aware handling including
+			 * multi-letter extensions will be added in-future.
 			 */
-			if ('a' <= isa[i] && isa[i] < 'x')
-				this_isa |= (1UL << (isa[i] - 'a'));
+			if (ext_err || ext_long)
+				continue;
+			this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
+			this_isa |= (1UL << (*ext - 'a'));
 		}
 
 		/*
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa"
  2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
  2022-02-15  9:02 ` [PATCH v3 1/6] RISC-V: Correctly print supported extensions Atish Patra
  2022-02-15  9:02 ` [PATCH v3 2/6] RISC-V: Minimal parser for "riscv, isa" strings Atish Patra
@ 2022-02-15  9:02 ` Atish Patra
  2022-02-15 10:14   ` Anup Patel
  2022-02-15  9:02 ` [PATCH v3 4/6] RISC-V: Implement multi-letter ISA extension probing framework Atish Patra
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Atish Patra @ 2022-02-15  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tsukasa OI, Atish Patra, Heiko Stuebner, Albert Ou, Atish Patra,
	Anup Patel, Damien Le Moal, devicetree, Jisheng Zhang,
	Krzysztof Kozlowski, linux-riscv, Palmer Dabbelt, Paul Walmsley,
	Rob Herring

From: Tsukasa OI <research_trasio@irq.a4lg.com>

Currently, there is no usage for version numbers in extensions as
any ratified non base ISA extension will always at v1.0.

Extract the extension names in place for future parsing.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
[Improved commit text and comments]
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/riscv/kernel/cpufeature.c | 38 ++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 9d5448542226..cd9eb34f8d11 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -119,9 +119,28 @@ void __init riscv_fill_hwcap(void)
 				ext_long = true;
 				/* Multi-letter extension must be delimited */
 				for (; *isa && *isa != '_'; ++isa)
-					if (!islower(*isa) && !isdigit(*isa))
+					if (unlikely(!islower(*isa)
+						     && !isdigit(*isa)))
 						ext_err = true;
-				/* ... but must be ignored. */
+				/* Parse backwards */
+				ext_end = isa;
+				if (unlikely(ext_err))
+					break;
+				if (!isdigit(ext_end[-1]))
+					break;
+				/* Skip the minor version */
+				while (isdigit(*--ext_end))
+					;
+				if (ext_end[0] != 'p'
+				    || !isdigit(ext_end[-1])) {
+					/* Advance it to offset the pre-decrement */
+					++ext_end;
+					break;
+				}
+				/* Skip the major version */
+				while (isdigit(*--ext_end))
+					;
+				++ext_end;
 				break;
 			default:
 				if (unlikely(!islower(*ext))) {
@@ -131,6 +150,7 @@ void __init riscv_fill_hwcap(void)
 				/* Find next extension */
 				if (!isdigit(*isa))
 					break;
+				/* Skip the minor version */
 				while (isdigit(*++isa))
 					;
 				if (*isa != 'p')
@@ -139,20 +159,20 @@ void __init riscv_fill_hwcap(void)
 					--isa;
 					break;
 				}
+				/* Skip the major version */
 				while (isdigit(*++isa))
 					;
 				break;
 			}
 			if (*isa != '_')
 				--isa;
-			/*
-			 * TODO: Full version-aware handling including
-			 * multi-letter extensions will be added in-future.
-			 */
-			if (ext_err || ext_long)
+
+			if (unlikely(ext_err))
 				continue;
-			this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
-			this_isa |= (1UL << (*ext - 'a'));
+			if (!ext_long) {
+				this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
+				this_isa |= (1UL << (*ext - 'a'));
+			}
 		}
 
 		/*
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/6] RISC-V: Implement multi-letter ISA extension probing framework
  2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
                   ` (2 preceding siblings ...)
  2022-02-15  9:02 ` [PATCH v3 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa" Atish Patra
@ 2022-02-15  9:02 ` Atish Patra
  2022-02-15 10:23   ` Anup Patel
  2022-02-15  9:02 ` [PATCH v3 5/6] RISC-V: Do no continue isa string parsing without correct XLEN Atish Patra
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Atish Patra @ 2022-02-15  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Atish Patra, Anup Patel, Damien Le Moal,
	devicetree, Jisheng Zhang, Krzysztof Kozlowski, linux-riscv,
	Palmer Dabbelt, Paul Walmsley, Rob Herring

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;
 
 		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
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 5/6] RISC-V: Do no continue isa string parsing without correct XLEN
  2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
                   ` (3 preceding siblings ...)
  2022-02-15  9:02 ` [PATCH v3 4/6] RISC-V: Implement multi-letter ISA extension probing framework Atish Patra
@ 2022-02-15  9:02 ` Atish Patra
  2022-02-15  9:02 ` [PATCH v3 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions Atish Patra
  2022-02-15 15:56 ` [PATCH v3 0/6] Provide a fraemework for RISC-V " Jisheng Zhang
  6 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-02-15  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Atish Patra, Anup Patel, Damien Le Moal,
	devicetree, Jisheng Zhang, Krzysztof Kozlowski, linux-riscv,
	Palmer Dabbelt, Paul Walmsley, Rob Herring

The isa string should begin with either rv64 or rv32. Otherwise, it is
an incorrect isa string. Currently, the string parsing continues even if
it doesnot begin with current XLEN.

Fix this by checking if it found "rv64" or "rv32" in the beginning.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/kernel/cpufeature.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index af9a57ad3d4e..0c818035ec92 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -84,6 +84,7 @@ void __init riscv_fill_hwcap(void)
 	for_each_of_cpu_node(node) {
 		unsigned long this_hwcap = 0;
 		uint64_t this_isa = 0;
+		const char *temp;
 
 		if (riscv_of_processor_hartid(node) < 0)
 			continue;
@@ -93,6 +94,7 @@ void __init riscv_fill_hwcap(void)
 			continue;
 		}
 
+		temp = isa;
 #if IS_ENABLED(CONFIG_32BIT)
 		if (!strncmp(isa, "rv32", 4))
 			isa += 4;
@@ -100,6 +102,9 @@ void __init riscv_fill_hwcap(void)
 		if (!strncmp(isa, "rv64", 4))
 			isa += 4;
 #endif
+		/* The riscv,isa DT property must start with rv64 or rv32 */
+		if (temp == isa)
+			continue;
 		for (; *isa; ++isa) {
 			const char *ext = isa++;
 			const char *ext_end = isa;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions
  2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
                   ` (4 preceding siblings ...)
  2022-02-15  9:02 ` [PATCH v3 5/6] RISC-V: Do no continue isa string parsing without correct XLEN Atish Patra
@ 2022-02-15  9:02 ` Atish Patra
  2022-02-15 15:56 ` [PATCH v3 0/6] Provide a fraemework for RISC-V " Jisheng Zhang
  6 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-02-15  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Atish Patra, Anup Patel, Damien Le Moal,
	devicetree, Jisheng Zhang, Krzysztof Kozlowski, linux-riscv,
	Palmer Dabbelt, Paul Walmsley, Rob Herring

Currently, the /proc/cpuinfo outputs the entire riscv,isa string which
is not ideal when we have multiple ISA extensions present in the ISA
string. Some of them may not be enabled in kernel as well.

Parse only the enabled ISA extension and print them in a separate row.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/include/asm/hwcap.h |  7 ++++++
 arch/riscv/kernel/cpu.c        | 44 ++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 170bd80da520..691fc9c8099b 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -54,6 +54,13 @@ enum riscv_isa_ext_id {
 	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
 };
 
+struct riscv_isa_ext_data {
+	/* Name of the extension displayed to userspace via /proc/cpuinfo */
+	char uprop[RISCV_ISA_EXT_NAME_LEN_MAX];
+	/* The logical ISA extension ID */
+	unsigned int isa_ext_id;
+};
+
 unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
 
 #define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ad0a7e9f828b..ced7e5be8641 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -6,6 +6,7 @@
 #include <linux/init.h>
 #include <linux/seq_file.h>
 #include <linux/of.h>
+#include <asm/hwcap.h>
 #include <asm/smp.h>
 #include <asm/pgtable.h>
 
@@ -63,12 +64,50 @@ int riscv_of_parent_hartid(struct device_node *node)
 }
 
 #ifdef CONFIG_PROC_FS
+#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
+	{							\
+		.uprop = #UPROP,				\
+		.isa_ext_id = EXTID,				\
+	}
+
+static struct riscv_isa_ext_data isa_ext_arr[] = {
+	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
+};
+
+static void print_isa_ext(struct seq_file *f)
+{
+	struct riscv_isa_ext_data *edata;
+	int i = 0, arr_sz;
+
+	arr_sz = ARRAY_SIZE(isa_ext_arr) - 1;
+
+	/* No extension support available */
+	if (arr_sz <= 0)
+		return;
+
+	seq_puts(f, "isa-ext\t\t: ");
+	for (i = 0; i <= arr_sz; i++) {
+		edata = &isa_ext_arr[i];
+		if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id))
+			continue;
+		seq_printf(f, "%s ", edata->uprop);
+	}
+	seq_puts(f, "\n");
+}
 
 static void print_isa(struct seq_file *f, const char *isa)
 {
-	/* Print the entire ISA as it is */
+	char *ext_start;
+	int isa_len = strlen(isa);
+	int base_isa_len = isa_len;
+
+	ext_start = strnchr(isa, isa_len, '_');
+	if (ext_start)
+		base_isa_len = isa_len - strlen(ext_start);
+
+	/* Print only the base ISA as it is */
 	seq_puts(f, "isa\t\t: ");
-	seq_write(f, isa, strlen(isa));
+	seq_write(f, isa, base_isa_len);
 	seq_puts(f, "\n");
 }
 
@@ -115,6 +154,7 @@ static int c_show(struct seq_file *m, void *v)
 	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
 	if (!of_property_read_string(node, "riscv,isa", &isa))
 		print_isa(m, isa);
+	print_isa_ext(m);
 	print_mmu(m);
 	if (!of_property_read_string(node, "compatible", &compat)
 	    && strcmp(compat, "riscv"))
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/6] RISC-V: Correctly print supported extensions
  2022-02-15  9:02 ` [PATCH v3 1/6] RISC-V: Correctly print supported extensions Atish Patra
@ 2022-02-15  9:41   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2022-02-15  9:41 UTC (permalink / raw)
  To: Atish Patra
  Cc: linux-kernel@vger.kernel.org List, Tsukasa OI, Heiko Stuebner,
	Albert Ou, Atish Patra, Damien Le Moal, DTML, Jisheng Zhang,
	Krzysztof Kozlowski, linux-riscv, Palmer Dabbelt, Paul Walmsley,
	Rob Herring

On Tue, Feb 15, 2022 at 2:32 PM Atish Patra <atishp@rivosinc.com> wrote:
>
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> This commit replaces BITS_PER_LONG with number of alphabet letters.
>
> Current ISA pretty-printing code expects extension 'a' (bit 0) through
> 'z' (bit 25).  Although bit 26 and higher is not currently used (thus never
> cause an issue in practice), it will be an annoying problem if we start to
> use those in the future.
>
> This commit disables printing high bits for now.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/kernel/cpufeature.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index d959d207a40d..dd3d57eb4eea 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -13,6 +13,8 @@
>  #include <asm/smp.h>
>  #include <asm/switch_to.h>
>
> +#define NUM_ALPHA_EXTS ('z' - 'a' + 1)
> +
>  unsigned long elf_hwcap __read_mostly;
>
>  /* Host ISA bitmap */
> @@ -63,7 +65,7 @@ void __init riscv_fill_hwcap(void)
>  {
>         struct device_node *node;
>         const char *isa;
> -       char print_str[BITS_PER_LONG + 1];
> +       char print_str[NUM_ALPHA_EXTS + 1];
>         size_t i, j, isa_len;
>         static unsigned long isa2hwcap[256] = {0};
>
> @@ -133,13 +135,13 @@ void __init riscv_fill_hwcap(void)
>         }
>
>         memset(print_str, 0, sizeof(print_str));
> -       for (i = 0, j = 0; i < BITS_PER_LONG; i++)
> +       for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
>                 if (riscv_isa[0] & BIT_MASK(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++)
> +       for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
>                 if (elf_hwcap & BIT_MASK(i))
>                         print_str[j++] = (char)('a' + i);
>         pr_info("riscv: ELF capabilities %s\n", print_str);
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa"
  2022-02-15  9:02 ` [PATCH v3 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa" Atish Patra
@ 2022-02-15 10:14   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2022-02-15 10:14 UTC (permalink / raw)
  To: Atish Patra
  Cc: linux-kernel@vger.kernel.org List, Tsukasa OI, Heiko Stuebner,
	Albert Ou, Atish Patra, Damien Le Moal, DTML, Jisheng Zhang,
	Krzysztof Kozlowski, linux-riscv, Palmer Dabbelt, Paul Walmsley,
	Rob Herring

On Tue, Feb 15, 2022 at 2:32 PM Atish Patra <atishp@rivosinc.com> wrote:
>
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> Currently, there is no usage for version numbers in extensions as
> any ratified non base ISA extension will always at v1.0.
>
> Extract the extension names in place for future parsing.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> [Improved commit text and comments]
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  arch/riscv/kernel/cpufeature.c | 38 ++++++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 9d5448542226..cd9eb34f8d11 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -119,9 +119,28 @@ void __init riscv_fill_hwcap(void)
>                                 ext_long = true;
>                                 /* Multi-letter extension must be delimited */
>                                 for (; *isa && *isa != '_'; ++isa)
> -                                       if (!islower(*isa) && !isdigit(*isa))
> +                                       if (unlikely(!islower(*isa)
> +                                                    && !isdigit(*isa)))
>                                                 ext_err = true;
> -                               /* ... but must be ignored. */
> +                               /* Parse backwards */
> +                               ext_end = isa;
> +                               if (unlikely(ext_err))
> +                                       break;
> +                               if (!isdigit(ext_end[-1]))
> +                                       break;
> +                               /* Skip the minor version */
> +                               while (isdigit(*--ext_end))
> +                                       ;
> +                               if (ext_end[0] != 'p'
> +                                   || !isdigit(ext_end[-1])) {
> +                                       /* Advance it to offset the pre-decrement */
> +                                       ++ext_end;
> +                                       break;
> +                               }
> +                               /* Skip the major version */
> +                               while (isdigit(*--ext_end))
> +                                       ;
> +                               ++ext_end;
>                                 break;
>                         default:
>                                 if (unlikely(!islower(*ext))) {
> @@ -131,6 +150,7 @@ void __init riscv_fill_hwcap(void)
>                                 /* Find next extension */
>                                 if (!isdigit(*isa))
>                                         break;
> +                               /* Skip the minor version */

This comment should be moved to PATCH2

>                                 while (isdigit(*++isa))
>                                         ;
>                                 if (*isa != 'p')
> @@ -139,20 +159,20 @@ void __init riscv_fill_hwcap(void)
>                                         --isa;
>                                         break;
>                                 }
> +                               /* Skip the major version */

Same applies to this comment as well.

>                                 while (isdigit(*++isa))
>                                         ;
>                                 break;
>                         }
>                         if (*isa != '_')
>                                 --isa;
> -                       /*
> -                        * TODO: Full version-aware handling including
> -                        * multi-letter extensions will be added in-future.
> -                        */
> -                       if (ext_err || ext_long)
> +
> +                       if (unlikely(ext_err))
>                                 continue;
> -                       this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
> -                       this_isa |= (1UL << (*ext - 'a'));
> +                       if (!ext_long) {
> +                               this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
> +                               this_isa |= (1UL << (*ext - 'a'));
> +                       }
>                 }
>
>                 /*
> --
> 2.30.2
>

Otherwise it looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/6] RISC-V: Implement multi-letter ISA extension probing framework
  2022-02-15  9:02 ` [PATCH v3 4/6] RISC-V: Implement multi-letter ISA extension probing framework Atish Patra
@ 2022-02-15 10:23   ` Anup Patel
  2022-02-15 23:11     ` Atish Patra
  0 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2022-02-15 10:23 UTC (permalink / raw)
  To: Atish Patra
  Cc: linux-kernel@vger.kernel.org List, Albert Ou, Atish Patra,
	Damien Le Moal, DTML, Jisheng Zhang, Krzysztof Kozlowski,
	linux-riscv, Palmer Dabbelt, Paul Walmsley, Rob Herring

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions
  2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
                   ` (5 preceding siblings ...)
  2022-02-15  9:02 ` [PATCH v3 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions Atish Patra
@ 2022-02-15 15:56 ` Jisheng Zhang
  2022-02-15 19:06   ` Atish Kumar Patra
  6 siblings, 1 reply; 14+ messages in thread
From: Jisheng Zhang @ 2022-02-15 15:56 UTC (permalink / raw)
  To: Atish Patra
  Cc: linux-kernel, Albert Ou, Atish Patra, Anup Patel, Damien Le Moal,
	devicetree, Krzysztof Kozlowski, linux-riscv, Palmer Dabbelt,
	Paul Walmsley, Rob Herring

On Tue, Feb 15, 2022 at 01:02:05AM -0800, Atish Patra wrote:
> This series implements a generic framework to parse multi-letter ISA
> extensions. This series is based on Tsukasa's v3 isa extension improvement
> series[1]. I have fixed few bugs and improved comments from that series
> (PATCH1-3). I have not used PATCH 4 from that series as we are not using
> ISA extension versioning as of now. We can add that later if required.
> 
> PATCH 4 allows the probing of multi-letter extensions via a macro.
> It continues to use the common isa extensions between all the harts.
> Thus hetergenous hart systems will only see the common ISA extensions.
> 
> PATCH 6 improves the /proc/cpuinfo interface for the available ISA extensions
> via /proc/cpuinfo.
> 
> Here is the example output of /proc/cpuinfo:
> (with debug patches in Qemu and Linux kernel)
> 
> / # cat /proc/cpuinfo
> processor	: 0
> hart		: 0
> isa		: rv64imafdcsu
> isa-ext		: sstc,sscofpmf
> mmu		: sv48
> 
> processor	: 1
> hart		: 1
> isa		: rv64imafdcsu
> isa-ext		: sstc,sscofpmf
> mmu		: sv48
> 
> processor	: 2
> hart		: 2
> isa		: rv64imafdcsu
> isa-ext		: sstc,sscofpmf
> mmu		: sv48
> 
> processor	: 3
> hart		: 3
> isa		: rv64imafdcsu
> isa-ext		: sstc,sscofpmf
> mmu		: sv48
> 
> Anybody adding support for any new multi-letter extensions should add an
> entry to the riscv_isa_ext_id and the isa extension array. 
> E.g. The patch[2] adds the support for various ISA extensions.

Hi Atish,

Thanks for this series. I'm thinking cpu features VS ISA extenstions.
I'm converting the sv48 to static key:
https://lore.kernel.org/linux-riscv/20220125165036.987-1-jszhang@kernel.org/

Previously, I thought the SV48 as a cpu feature, and there will be
more and more cpu features, so I implemented an unified static key
mechanism for CPU features. But after reading this series, I think
I may need to rebase(even reimplement) the above patch to your series.
But I'm a bit confused by CPU features VS ISA extenstions now:

1. Is cpu feature  == ISA extension?

2. Is SV48 considered as ISA extension?
If yes, now SV48 or not is determined during runtime, but current ISA
extensions seem parsed from DT. So how to support those ISA extensions
which can be determined during runtime?

Could you please share your thought?

Thanks

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions
  2022-02-15 15:56 ` [PATCH v3 0/6] Provide a fraemework for RISC-V " Jisheng Zhang
@ 2022-02-15 19:06   ` Atish Kumar Patra
  2022-03-26  7:18     ` Jisheng Zhang
  0 siblings, 1 reply; 14+ messages in thread
From: Atish Kumar Patra @ 2022-02-15 19:06 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: linux-kernel@vger.kernel.org List, Albert Ou, Atish Patra,
	Anup Patel, Damien Le Moal, devicetree, Krzysztof Kozlowski,
	linux-riscv, Palmer Dabbelt, Paul Walmsley, Rob Herring

On Tue, Feb 15, 2022 at 8:04 AM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> On Tue, Feb 15, 2022 at 01:02:05AM -0800, Atish Patra wrote:
> > This series implements a generic framework to parse multi-letter ISA
> > extensions. This series is based on Tsukasa's v3 isa extension improvement
> > series[1]. I have fixed few bugs and improved comments from that series
> > (PATCH1-3). I have not used PATCH 4 from that series as we are not using
> > ISA extension versioning as of now. We can add that later if required.
> >
> > PATCH 4 allows the probing of multi-letter extensions via a macro.
> > It continues to use the common isa extensions between all the harts.
> > Thus hetergenous hart systems will only see the common ISA extensions.
> >
> > PATCH 6 improves the /proc/cpuinfo interface for the available ISA extensions
> > via /proc/cpuinfo.
> >
> > Here is the example output of /proc/cpuinfo:
> > (with debug patches in Qemu and Linux kernel)
> >
> > / # cat /proc/cpuinfo
> > processor     : 0
> > hart          : 0
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > processor     : 1
> > hart          : 1
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > processor     : 2
> > hart          : 2
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > processor     : 3
> > hart          : 3
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > Anybody adding support for any new multi-letter extensions should add an
> > entry to the riscv_isa_ext_id and the isa extension array.
> > E.g. The patch[2] adds the support for various ISA extensions.
>
> Hi Atish,
>
> Thanks for this series. I'm thinking cpu features VS ISA extenstions.
> I'm converting the sv48 to static key:
> https://lore.kernel.org/linux-riscv/20220125165036.987-1-jszhang@kernel.org/
>
> Previously, I thought the SV48 as a cpu feature, and there will be
> more and more cpu features, so I implemented an unified static key
> mechanism for CPU features. But after reading this series, I think
> I may need to rebase(even reimplement) the above patch to your series.
> But I'm a bit confused by CPU features VS ISA extenstions now:
>
> 1. Is cpu feature  == ISA extension?
>
> 2. Is SV48 considered as ISA extension?
> If yes, now SV48 or not is determined during runtime, but current ISA
> extensions seem parsed from DT. So how to support those ISA extensions
> which can be determined during runtime?
>
> Could you please share your thought?
>

Here are my two cents:

I think the cpu feature is a superset of the ISA extension.
cpu feature != ISA extension.

While all ISA extensions are cpu features, all CPU features may not be
an ISA extension.
e.g. sv48 is not a ISA extension but F/D are (used to set the
cpu_hwcap_fpu static key)

Moreover, not all cpu feature/ISA extension requires a static key.
e.g SSTC extension will require a static key because the check has to
happen in the hot path.
However, sscofpmf extension don't need a static key as the check
happens only one time during boot.

We should keep these two separate but a common static framework would
be very useful.

Here is the flow that I have in my mind.
1. All ISA extensions will be parsed through riscv,isa DT property
2. Any supported/enabled extension will be set in riscv_isa bitmap
3. Any extension requiring a static key will invoke the cpus_set_cap.

cpus_set_cap will be invoked from a different code path that uses a
static key for a specific ISA
extension or a CPU feature.

The only problem I see here is that we have to set a bit in both
cpu_hwcaps & riscv_isa bitmap.
We also have to define the value of that bit for any extension
requiring a static key twice as well.

I think that should be okay. But I would like to hear what everybody
else thinks as well.

> Thanks

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/6] RISC-V: Implement multi-letter ISA extension probing framework
  2022-02-15 10:23   ` Anup Patel
@ 2022-02-15 23:11     ` Atish Patra
  0 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-02-15 23:11 UTC (permalink / raw)
  To: Anup Patel
  Cc: Atish Patra, linux-kernel@vger.kernel.org List, Albert Ou,
	Damien Le Moal, DTML, Jisheng Zhang, Krzysztof Kozlowski,
	linux-riscv, Palmer Dabbelt, Paul Walmsley, Rob Herring

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



-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions
  2022-02-15 19:06   ` Atish Kumar Patra
@ 2022-03-26  7:18     ` Jisheng Zhang
  0 siblings, 0 replies; 14+ messages in thread
From: Jisheng Zhang @ 2022-03-26  7:18 UTC (permalink / raw)
  To: Atish Kumar Patra
  Cc: linux-kernel@vger.kernel.org List, Albert Ou, Atish Patra,
	Anup Patel, Damien Le Moal, devicetree, Krzysztof Kozlowski,
	linux-riscv, Palmer Dabbelt, Paul Walmsley, Rob Herring

On Tue, Feb 15, 2022 at 11:06:24AM -0800, Atish Kumar Patra wrote:
> On Tue, Feb 15, 2022 at 8:04 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> >
> > On Tue, Feb 15, 2022 at 01:02:05AM -0800, Atish Patra wrote:
> > > This series implements a generic framework to parse multi-letter ISA
> > > extensions. This series is based on Tsukasa's v3 isa extension improvement
> > > series[1]. I have fixed few bugs and improved comments from that series
> > > (PATCH1-3). I have not used PATCH 4 from that series as we are not using
> > > ISA extension versioning as of now. We can add that later if required.
> > >
> > > PATCH 4 allows the probing of multi-letter extensions via a macro.
> > > It continues to use the common isa extensions between all the harts.
> > > Thus hetergenous hart systems will only see the common ISA extensions.
> > >
> > > PATCH 6 improves the /proc/cpuinfo interface for the available ISA extensions
> > > via /proc/cpuinfo.
> > >
> > > Here is the example output of /proc/cpuinfo:
> > > (with debug patches in Qemu and Linux kernel)
> > >
> > > / # cat /proc/cpuinfo
> > > processor     : 0
> > > hart          : 0
> > > isa           : rv64imafdcsu
> > > isa-ext               : sstc,sscofpmf
> > > mmu           : sv48
> > >
> > > processor     : 1
> > > hart          : 1
> > > isa           : rv64imafdcsu
> > > isa-ext               : sstc,sscofpmf
> > > mmu           : sv48
> > >
> > > processor     : 2
> > > hart          : 2
> > > isa           : rv64imafdcsu
> > > isa-ext               : sstc,sscofpmf
> > > mmu           : sv48
> > >
> > > processor     : 3
> > > hart          : 3
> > > isa           : rv64imafdcsu
> > > isa-ext               : sstc,sscofpmf
> > > mmu           : sv48
> > >
> > > Anybody adding support for any new multi-letter extensions should add an
> > > entry to the riscv_isa_ext_id and the isa extension array.
> > > E.g. The patch[2] adds the support for various ISA extensions.
> >
> > Hi Atish,
> >
> > Thanks for this series. I'm thinking cpu features VS ISA extenstions.
> > I'm converting the sv48 to static key:
> > https://lore.kernel.org/linux-riscv/20220125165036.987-1-jszhang@kernel.org/
> >
> > Previously, I thought the SV48 as a cpu feature, and there will be
> > more and more cpu features, so I implemented an unified static key
> > mechanism for CPU features. But after reading this series, I think
> > I may need to rebase(even reimplement) the above patch to your series.
> > But I'm a bit confused by CPU features VS ISA extenstions now:
> >
> > 1. Is cpu feature  == ISA extension?
> >
> > 2. Is SV48 considered as ISA extension?
> > If yes, now SV48 or not is determined during runtime, but current ISA
> > extensions seem parsed from DT. So how to support those ISA extensions
> > which can be determined during runtime?
> >
> > Could you please share your thought?
> >
> 
> Here are my two cents:
> 
> I think the cpu feature is a superset of the ISA extension.
> cpu feature != ISA extension.
> 
> While all ISA extensions are cpu features, all CPU features may not be
> an ISA extension.
> e.g. sv48 is not a ISA extension but F/D are (used to set the
> cpu_hwcap_fpu static key)
> 
> Moreover, not all cpu feature/ISA extension requires a static key.
> e.g SSTC extension will require a static key because the check has to
> happen in the hot path.
> However, sscofpmf extension don't need a static key as the check
> happens only one time during boot.
> 
> We should keep these two separate but a common static framework would
> be very useful.
> 
> Here is the flow that I have in my mind.
> 1. All ISA extensions will be parsed through riscv,isa DT property
> 2. Any supported/enabled extension will be set in riscv_isa bitmap
> 3. Any extension requiring a static key will invoke the cpus_set_cap.
> 
> cpus_set_cap will be invoked from a different code path that uses a
> static key for a specific ISA
> extension or a CPU feature.
> 
> The only problem I see here is that we have to set a bit in both
> cpu_hwcaps & riscv_isa bitmap.
> We also have to define the value of that bit for any extension
> requiring a static key twice as well.
> 
> I think that should be okay. But I would like to hear what everybody
> else thinks as well.
> 

Thank Atish's input. I notice that SV57 support is merged, I'll
send a new version to apply static mechanism to both SV48 and SV57
once rc1 is released.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-03-26  7:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-15  9:02 [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions Atish Patra
2022-02-15  9:02 ` [PATCH v3 1/6] RISC-V: Correctly print supported extensions Atish Patra
2022-02-15  9:41   ` Anup Patel
2022-02-15  9:02 ` [PATCH v3 2/6] RISC-V: Minimal parser for "riscv, isa" strings Atish Patra
2022-02-15  9:02 ` [PATCH v3 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa" Atish Patra
2022-02-15 10:14   ` Anup Patel
2022-02-15  9:02 ` [PATCH v3 4/6] RISC-V: Implement multi-letter ISA extension probing framework Atish Patra
2022-02-15 10:23   ` Anup Patel
2022-02-15 23:11     ` Atish Patra
2022-02-15  9:02 ` [PATCH v3 5/6] RISC-V: Do no continue isa string parsing without correct XLEN Atish Patra
2022-02-15  9:02 ` [PATCH v3 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions Atish Patra
2022-02-15 15:56 ` [PATCH v3 0/6] Provide a fraemework for RISC-V " Jisheng Zhang
2022-02-15 19:06   ` Atish Kumar Patra
2022-03-26  7:18     ` Jisheng Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).