[PATCH v4 0/6] Provide a fraemework for RISC-V ISA extensions

[PATCH v4 0/6] Provide a fraemework for RISC-V ISA extensions

[Atish Patra]

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/e9e240c9a854dceb434ceb53bdbe82a657bee5f2 


Changes from v3->v4:
1. Changed temporary variable for current hart isa to a bitmap
2. Added reviewed-by tags.
3. Improved comments

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 | 125 +++++++++++++++++++++++++++------
3 files changed, 171 insertions(+), 23 deletions(-)

--
2.30.2

[PATCH v4 0/6] Provide a fraemework for RISC-V ISA extensions

[Atish Patra]

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/e9e240c9a854dceb434ceb53bdbe82a657bee5f2 


Changes from v3->v4:
1. Changed temporary variable for current hart isa to a bitmap
2. Added reviewed-by tags.
3. Improved comments

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 | 125 +++++++++++++++++++++++++++------
3 files changed, 171 insertions(+), 23 deletions(-)

--
2.30.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v4 1/6] RISC-V: Correctly print supported extensions

[Atish Patra]

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.

Reviewed-by: Anup Patel <anup@brainfault.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 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

[PATCH v4 1/6] RISC-V: Correctly print supported extensions

[Atish Patra]

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.

Reviewed-by: Anup Patel <anup@brainfault.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v4 2/6] RISC-V: Minimal parser for "riscv, isa" strings

[Atish Patra]

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 | 67 ++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index dd3d57eb4eea..65664422b04d 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,67 @@ 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;
+				break;
+			default:
+				if (unlikely(!islower(*ext))) {
+					ext_err = true;
+					break;
+				}
+				/* Find next extension */
+				if (!isdigit(*isa))
+					break;
+				/* Skip the minor version */
+				while (isdigit(*++isa))
+					;
+				if (*isa != 'p')
+					break;
+				if (!isdigit(*++isa)) {
+					--isa;
+					break;
+				}
+				/* Skip the major version */
+				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

[PATCH v4 2/6] RISC-V: Minimal parser for "riscv, isa" strings

[Atish Patra]

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 | 67 ++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index dd3d57eb4eea..65664422b04d 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,67 @@ 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;
+				break;
+			default:
+				if (unlikely(!islower(*ext))) {
+					ext_err = true;
+					break;
+				}
+				/* Find next extension */
+				if (!isdigit(*isa))
+					break;
+				/* Skip the minor version */
+				while (isdigit(*++isa))
+					;
+				if (*isa != 'p')
+					break;
+				if (!isdigit(*++isa)) {
+					--isa;
+					break;
+				}
+				/* Skip the major version */
+				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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v4 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa"

[Atish Patra]

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.

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

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 65664422b04d..cd9eb34f8d11 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -119,8 +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;
+				/* 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))) {
@@ -146,14 +166,13 @@ void __init riscv_fill_hwcap(void)
 			}
 			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

[PATCH v4 3/6] RISC-V: Extract multi-letter extension names from "riscv, isa"

[Atish Patra]

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.

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

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 65664422b04d..cd9eb34f8d11 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -119,8 +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;
+				/* 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))) {
@@ -146,14 +166,13 @@ void __init riscv_fill_hwcap(void)
 			}
 			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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Atish Patra]

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 | 24 ++++++++++++++++++------
 2 files changed, 36 insertions(+), 6 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..59c70c104256 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;
+		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
 
 		if (riscv_of_processor_hartid(node) < 0)
 			continue;
@@ -100,6 +100,7 @@ void __init riscv_fill_hwcap(void)
 		if (!strncmp(isa, "rv64", 4))
 			isa += 4;
 #endif
+		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
 		for (; *isa; ++isa) {
 			const char *ext = isa++;
 			const char *ext_end = isa;
@@ -167,12 +168,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)) {    \
+					set_bit(bit, this_isa);			\
+					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'));
+				set_bit(*ext - 'a', this_isa);
 			}
+#undef SET_ISA_EXT_MAP
 		}
 
 		/*
@@ -185,10 +196,11 @@ void __init riscv_fill_hwcap(void)
 		else
 			elf_hwcap = this_hwcap;
 
-		if (riscv_isa[0])
-			riscv_isa[0] &= this_isa;
+		if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
+			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
 		else
-			riscv_isa[0] = this_isa;
+			bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
+
 	}
 
 	/* We don't support systems with F but without D, so mask those out
@@ -202,7 +214,7 @@ void __init riscv_fill_hwcap(void)
 	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);
+	pr_info("riscv: base ISA extensions %s\n", print_str);
 
 	memset(print_str, 0, sizeof(print_str));
 	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
-- 
2.30.2

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

[Atish Patra]

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 | 24 ++++++++++++++++++------
 2 files changed, 36 insertions(+), 6 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..59c70c104256 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;
+		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
 
 		if (riscv_of_processor_hartid(node) < 0)
 			continue;
@@ -100,6 +100,7 @@ void __init riscv_fill_hwcap(void)
 		if (!strncmp(isa, "rv64", 4))
 			isa += 4;
 #endif
+		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
 		for (; *isa; ++isa) {
 			const char *ext = isa++;
 			const char *ext_end = isa;
@@ -167,12 +168,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)) {    \
+					set_bit(bit, this_isa);			\
+					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'));
+				set_bit(*ext - 'a', this_isa);
 			}
+#undef SET_ISA_EXT_MAP
 		}
 
 		/*
@@ -185,10 +196,11 @@ void __init riscv_fill_hwcap(void)
 		else
 			elf_hwcap = this_hwcap;
 
-		if (riscv_isa[0])
-			riscv_isa[0] &= this_isa;
+		if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
+			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
 		else
-			riscv_isa[0] = this_isa;
+			bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
+
 	}
 
 	/* We don't support systems with F but without D, so mask those out
@@ -202,7 +214,7 @@ void __init riscv_fill_hwcap(void)
 	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);
+	pr_info("riscv: base ISA extensions %s\n", print_str);
 
 	memset(print_str, 0, sizeof(print_str));
 	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
-- 
2.30.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v4 5/6] RISC-V: Do no continue isa string parsing without correct XLEN

[Atish Patra]

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 59c70c104256..cb9c9e0aab31 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;
 		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
+		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;
 		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
 		for (; *isa; ++isa) {
 			const char *ext = isa++;
-- 
2.30.2

[PATCH v4 5/6] RISC-V: Do no continue isa string parsing without correct XLEN

[Atish Patra]

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 59c70c104256..cb9c9e0aab31 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;
 		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
+		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;
 		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
 		for (; *isa; ++isa) {
 			const char *ext = isa++;
-- 
2.30.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v4 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions

[Atish Patra]

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

[PATCH v4 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions

[Atish Patra]

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 0/2] RISC-V: some improvements for Atish's framework (for v5)

[Tsukasa OI]

This is patches for Atish's ISA framework patchset v4 (intended to be
squashed into patch 3 and 6 of it).

Changes:
1.  It now skips 'S' and 'U' single-letter "extensions" on QEMU
    (with M/S/U modes) without handling as real extensions.
2.  Split base ISA using minimal parser (not by just splitting with '_',
    find and stop before the first multi-letter extensions, stripping
    version numbers)




Tsukasa OI (2):
  RISC-V: Better 'S' workaround
  RISC-V: Extract base ISA from device tree

 arch/riscv/kernel/cpu.c        | 83 ++++++++++++++++++++++++++++++----
 arch/riscv/kernel/cpufeature.c | 10 ++--
 2 files changed, 81 insertions(+), 12 deletions(-)


base-commit: e9e240c9a854dceb434ceb53bdbe82a657bee5f2
-- 
2.32.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/2] RISC-V: Better 'S' workaround

[Tsukasa OI]

This commit moves 'S' workaround and skips 'S' and 'U' "extensions"
(invalid as single-letter extensions) from riscv_isa (base ISA
extensions).

This commit is intended to be squashed into Atish's isa_framework_v4
PATCH 3/6.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
 arch/riscv/kernel/cpufeature.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index c3ef55203117..b00ce81627a4 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -113,15 +113,19 @@ void __init riscv_fill_hwcap(void)
 
 			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')
+				if (ext[-1] != '_' && ext[1] == 'u') {
+					++isa;
+					ext_err = true;
 					break;
+				}
+				fallthrough;
+			case 'x':
+			case 'z':
 				ext_long = true;
 				/* Multi-letter extension must be delimited */
 				for (; *isa && *isa != '_'; ++isa)
-- 
2.32.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] RISC-V: Extract base ISA from device tree

[Tsukasa OI]

This commit replaces print_isa function to extract base ISA from device
tree ("riscv,isa").  It uses a subset of Tsukasa's riscv_fill_hwcap ISA
parser.

Design choices (1):

It constructs base ISA string from the original string (does not cut
the original string like Atish's).  This is to strip version numbers
(too minor to represent as major ISA) but this behavior may be
debatable.

Design choices (2):

It skips when single-letter 'S' or 'U' is encountered.  It improves
familiarity with regular ISA string.

This commit is intended to be squashed into Atish's isa_framework_v4
PATCH 6/6.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
 arch/riscv/kernel/cpu.c | 83 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 6f9660c0a973..3f9607a66d95 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -101,17 +101,82 @@ static void print_isa_ext(struct seq_file *f)
 
 static void print_isa(struct seq_file *f, const char *isa)
 {
-	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);
+	unsigned char parse_break = 0;
+	char base_isa[RISCV_ISA_EXT_BASE + 5];
+	char *base_isa_end = base_isa;
+	const char *isa_end = isa;
+#if IS_ENABLED(CONFIG_32BIT)
+	if (!strncmp(isa, "rv32", 4))
+		isa_end += 4;
+#elif IS_ENABLED(CONFIG_64BIT)
+	if (!strncmp(isa, "rv64", 4))
+		isa_end += 4;
+#endif
+	if (isa != isa_end) {
+		strncpy(base_isa, isa, isa_end - isa);
+		base_isa_end += isa_end - isa;
+		for (; *isa_end; ++isa_end) {
+			switch (*isa_end++) {
+			case 'x':
+			case 'z':
+				parse_break = 1;
+				break;
+			case 's':
+				if (*isa_end != 'u') {
+					parse_break = 1;
+					break;
+				}
+				/**
+				 * Workaround for invalid single-letter 's'
+				 * (QEMU).  Should break for valid ISA string.
+				 */
+				fallthrough;
+			default:
+				if (unlikely(!islower(isa_end[-1])
+				    || base_isa_end == base_isa + sizeof(base_isa))) {
+					parse_break = 2;
+					break;
+				}
+				switch (isa_end[-1]) {
+				case 's':
+				case 'u':
+					break;
+				default:
+					*base_isa_end++ = isa_end[-1];
+				}
+				/* Skip version number */
+				if (!isdigit(*isa_end))
+					break;
+				while (isdigit(*++isa_end))
+					;
+				if (*isa_end != 'p')
+					break;
+				if (!isdigit(*++isa_end)) {
+					--isa_end;
+					break;
+				}
+				while (isdigit(*++isa_end))
+					;
+				break;
+			}
+			if (*isa_end != '_')
+				--isa_end;
+			if (parse_break)
+				break;
+		}
+	}
 
-	/* Print only the base ISA as it is */
+	/**
+	 * Print only the base ISA
+	 * (original ISA string as is if an error is encountered)
+	 */
 	seq_puts(f, "isa\t\t: ");
-	seq_write(f, isa, base_isa_len);
+	if (parse_break < 2) {
+		seq_write(f, base_isa, base_isa_end - base_isa);
+	} else {
+		isa_end += strlen(isa_end);
+		seq_write(f, isa, isa_end - isa);
+	}
 	seq_puts(f, "\n");
 }
 
-- 
2.32.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] RISC-V: Extract base ISA from device tree

[Atish Patra]

On Tue, Feb 15, 2022 at 9:04 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> This commit replaces print_isa function to extract base ISA from device
> tree ("riscv,isa").  It uses a subset of Tsukasa's riscv_fill_hwcap ISA
> parser.
>
> Design choices (1):
>
> It constructs base ISA string from the original string (does not cut
> the original string like Atish's).  This is to strip version numbers
> (too minor to represent as major ISA) but this behavior may be
> debatable.
>

A simpler solution would be to just invoke
__riscv_isa_extension_available for all the base
extensions (imafdc) and print the base isa string.
We don't need to parse the ISA string again here.

> Design choices (2):
>
> It skips when single-letter 'S' or 'U' is encountered.  It improves
> familiarity with regular ISA string.
>

I don't think this is necessary because only Qemu appends 'S' & 'U' in
the isa string.
Your patch in Qemu will fix that for future releases of Qemu.
All other commonly used platforms (hardware/spike) already use the
correct ISA string.

> This commit is intended to be squashed into Atish's isa_framework_v4
> PATCH 6/6.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
>  arch/riscv/kernel/cpu.c | 83 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 6f9660c0a973..3f9607a66d95 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -101,17 +101,82 @@ static void print_isa_ext(struct seq_file *f)
>
>  static void print_isa(struct seq_file *f, const char *isa)
>  {
> -       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);
> +       unsigned char parse_break = 0;
> +       char base_isa[RISCV_ISA_EXT_BASE + 5];
> +       char *base_isa_end = base_isa;
> +       const char *isa_end = isa;
> +#if IS_ENABLED(CONFIG_32BIT)
> +       if (!strncmp(isa, "rv32", 4))
> +               isa_end += 4;
> +#elif IS_ENABLED(CONFIG_64BIT)
> +       if (!strncmp(isa, "rv64", 4))
> +               isa_end += 4;
> +#endif
> +       if (isa != isa_end) {
> +               strncpy(base_isa, isa, isa_end - isa);
> +               base_isa_end += isa_end - isa;
> +               for (; *isa_end; ++isa_end) {
> +                       switch (*isa_end++) {
> +                       case 'x':
> +                       case 'z':
> +                               parse_break = 1;
> +                               break;
> +                       case 's':
> +                               if (*isa_end != 'u') {
> +                                       parse_break = 1;
> +                                       break;
> +                               }
> +                               /**
> +                                * Workaround for invalid single-letter 's'
> +                                * (QEMU).  Should break for valid ISA string.
> +                                */
> +                               fallthrough;
> +                       default:
> +                               if (unlikely(!islower(isa_end[-1])
> +                                   || base_isa_end == base_isa + sizeof(base_isa))) {
> +                                       parse_break = 2;
> +                                       break;
> +                               }
> +                               switch (isa_end[-1]) {
> +                               case 's':
> +                               case 'u':
> +                                       break;
> +                               default:
> +                                       *base_isa_end++ = isa_end[-1];
> +                               }
> +                               /* Skip version number */
> +                               if (!isdigit(*isa_end))
> +                                       break;
> +                               while (isdigit(*++isa_end))
> +                                       ;
> +                               if (*isa_end != 'p')
> +                                       break;
> +                               if (!isdigit(*++isa_end)) {
> +                                       --isa_end;
> +                                       break;
> +                               }
> +                               while (isdigit(*++isa_end))
> +                                       ;
> +                               break;
> +                       }
> +                       if (*isa_end != '_')
> +                               --isa_end;
> +                       if (parse_break)
> +                               break;
> +               }
> +       }
>
> -       /* Print only the base ISA as it is */
> +       /**
> +        * Print only the base ISA
> +        * (original ISA string as is if an error is encountered)
> +        */
>         seq_puts(f, "isa\t\t: ");
> -       seq_write(f, isa, base_isa_len);
> +       if (parse_break < 2) {
> +               seq_write(f, base_isa, base_isa_end - base_isa);
> +       } else {
> +               isa_end += strlen(isa_end);
> +               seq_write(f, isa, isa_end - isa);
> +       }
>         seq_puts(f, "\n");
>  }
>
> --
> 2.32.0
>


-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] RISC-V: Extract base ISA from device tree

[Tsukasa OI]

On 2022/02/16 15:01, Atish Patra wrote:
> On Tue, Feb 15, 2022 at 9:04 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>>
>> This commit replaces print_isa function to extract base ISA from device
>> tree ("riscv,isa").  It uses a subset of Tsukasa's riscv_fill_hwcap ISA
>> parser.
>>
>> Design choices (1):
>>
>> It constructs base ISA string from the original string (does not cut
>> the original string like Atish's).  This is to strip version numbers
>> (too minor to represent as major ISA) but this behavior may be
>> debatable.
>>
> 
> A simpler solution would be to just invoke
> __riscv_isa_extension_available for all the base
> extensions (imafdc) and print the base isa string.
> We don't need to parse the ISA string again here.

I agree that my patchset is ugly and simpler solution would be good.
(Having two parser for same string is definitely not the best idea.)
There are some testcases (some are not accepted by Spike's --isa option
and requires custom Device tree) for testing:

*   rv64imac
*   rv64imacsu      (invalid ISA string; test QEMU workaround)
*   rv64imafdch
*   rv64imafdcsuh   (invalid ISA string; test QEMU workaround)
*   rv64imafdc_svinval_svnapot
*   rv64imafdcsvinval_svnapot
*   rv64i2p1m2a2fdcsvinval
*   rv64i2p1_m_afdcsvinval

> 
>> Design choices (2):
>>
>> It skips when single-letter 'S' or 'U' is encountered.  It improves
>> familiarity with regular ISA string.
>>
> 
> I don't think this is necessary because only Qemu appends 'S' & 'U' in
> the isa string.
> Your patch in Qemu will fix that for future releases of Qemu.
> All other commonly used platforms (hardware/spike) already use the
> correct ISA string.

Yes, only QEMU.  It's not a serious problem like on cpufeature.c but would
be nice to remove them (when exposing to the usermode) to prevent confusion
on usermode side.

If you invoke __riscv_isa_extension_available to generate base ISA string,
would you exclude 'S' and 'U'?

> 
>> This commit is intended to be squashed into Atish's isa_framework_v4
>> PATCH 6/6.
>>
>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>> ---
>>  arch/riscv/kernel/cpu.c | 83 ++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 74 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>> index 6f9660c0a973..3f9607a66d95 100644
>> --- a/arch/riscv/kernel/cpu.c
>> +++ b/arch/riscv/kernel/cpu.c
>> @@ -101,17 +101,82 @@ static void print_isa_ext(struct seq_file *f)
>>
>>  static void print_isa(struct seq_file *f, const char *isa)
>>  {
>> -       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);
>> +       unsigned char parse_break = 0;
>> +       char base_isa[RISCV_ISA_EXT_BASE + 5];
>> +       char *base_isa_end = base_isa;
>> +       const char *isa_end = isa;
>> +#if IS_ENABLED(CONFIG_32BIT)
>> +       if (!strncmp(isa, "rv32", 4))
>> +               isa_end += 4;
>> +#elif IS_ENABLED(CONFIG_64BIT)
>> +       if (!strncmp(isa, "rv64", 4))
>> +               isa_end += 4;
>> +#endif
>> +       if (isa != isa_end) {
>> +               strncpy(base_isa, isa, isa_end - isa);
>> +               base_isa_end += isa_end - isa;
>> +               for (; *isa_end; ++isa_end) {
>> +                       switch (*isa_end++) {
>> +                       case 'x':
>> +                       case 'z':
>> +                               parse_break = 1;
>> +                               break;
>> +                       case 's':
>> +                               if (*isa_end != 'u') {
>> +                                       parse_break = 1;
>> +                                       break;
>> +                               }
>> +                               /**
>> +                                * Workaround for invalid single-letter 's'
>> +                                * (QEMU).  Should break for valid ISA string.
>> +                                */
>> +                               fallthrough;
>> +                       default:
>> +                               if (unlikely(!islower(isa_end[-1])
>> +                                   || base_isa_end == base_isa + sizeof(base_isa))) {
>> +                                       parse_break = 2;
>> +                                       break;
>> +                               }
>> +                               switch (isa_end[-1]) {
>> +                               case 's':
>> +                               case 'u':
>> +                                       break;
>> +                               default:
>> +                                       *base_isa_end++ = isa_end[-1];
>> +                               }
>> +                               /* Skip version number */
>> +                               if (!isdigit(*isa_end))
>> +                                       break;
>> +                               while (isdigit(*++isa_end))
>> +                                       ;
>> +                               if (*isa_end != 'p')
>> +                                       break;
>> +                               if (!isdigit(*++isa_end)) {
>> +                                       --isa_end;
>> +                                       break;
>> +                               }
>> +                               while (isdigit(*++isa_end))
>> +                                       ;
>> +                               break;
>> +                       }
>> +                       if (*isa_end != '_')
>> +                               --isa_end;
>> +                       if (parse_break)
>> +                               break;
>> +               }
>> +       }
>>
>> -       /* Print only the base ISA as it is */
>> +       /**
>> +        * Print only the base ISA
>> +        * (original ISA string as is if an error is encountered)
>> +        */
>>         seq_puts(f, "isa\t\t: ");
>> -       seq_write(f, isa, base_isa_len);
>> +       if (parse_break < 2) {
>> +               seq_write(f, base_isa, base_isa_end - base_isa);
>> +       } else {
>> +               isa_end += strlen(isa_end);
>> +               seq_write(f, isa, isa_end - isa);
>> +       }
>>         seq_puts(f, "\n");
>>  }
>>
>> --
>> 2.32.0
>>
> 
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] RISC-V: Extract base ISA from device tree

[Atish Patra]

On Tue, Feb 15, 2022 at 10:59 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> On 2022/02/16 15:01, Atish Patra wrote:
> > On Tue, Feb 15, 2022 at 9:04 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
> >>
> >> This commit replaces print_isa function to extract base ISA from device
> >> tree ("riscv,isa").  It uses a subset of Tsukasa's riscv_fill_hwcap ISA
> >> parser.
> >>
> >> Design choices (1):
> >>
> >> It constructs base ISA string from the original string (does not cut
> >> the original string like Atish's).  This is to strip version numbers
> >> (too minor to represent as major ISA) but this behavior may be
> >> debatable.
> >>
> >
> > A simpler solution would be to just invoke
> > __riscv_isa_extension_available for all the base
> > extensions (imafdc) and print the base isa string.
> > We don't need to parse the ISA string again here.
>
> I agree that my patchset is ugly and simpler solution would be good.
> (Having two parser for same string is definitely not the best idea.)
> There are some testcases (some are not accepted by Spike's --isa option
> and requires custom Device tree) for testing:
>
> *   rv64imac
> *   rv64imacsu      (invalid ISA string; test QEMU workaround)
> *   rv64imafdch
> *   rv64imafdcsuh   (invalid ISA string; test QEMU workaround)
> *   rv64imafdc_svinval_svnapot
> *   rv64imafdcsvinval_svnapot
> *   rv64i2p1m2a2fdcsvinval
> *   rv64i2p1_m_afdcsvinval
>

Thanks. This is a good set of test cases that covers most of the cases.

> >
> >> Design choices (2):
> >>
> >> It skips when single-letter 'S' or 'U' is encountered.  It improves
> >> familiarity with regular ISA string.
> >>
> >
> > I don't think this is necessary because only Qemu appends 'S' & 'U' in
> > the isa string.
> > Your patch in Qemu will fix that for future releases of Qemu.
> > All other commonly used platforms (hardware/spike) already use the
> > correct ISA string.
>
> Yes, only QEMU.  It's not a serious problem like on cpufeature.c but would
> be nice to remove them (when exposing to the usermode) to prevent confusion
> on usermode side.
>
> If you invoke __riscv_isa_extension_available to generate base ISA string,
> would you exclude 'S' and 'U'?
>

Yeah. That's easy. The possible valid base extensions are "IMAFHDQCBJPV".
(all the possible defined bits in misa except 'S' & 'U')
So we will invoke __riscv_isa_extension_available for those only.

> >
> >> This commit is intended to be squashed into Atish's isa_framework_v4
> >> PATCH 6/6.
> >>
> >> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> >> ---
> >>  arch/riscv/kernel/cpu.c | 83 ++++++++++++++++++++++++++++++++++++-----
> >>  1 file changed, 74 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> >> index 6f9660c0a973..3f9607a66d95 100644
> >> --- a/arch/riscv/kernel/cpu.c
> >> +++ b/arch/riscv/kernel/cpu.c
> >> @@ -101,17 +101,82 @@ static void print_isa_ext(struct seq_file *f)
> >>
> >>  static void print_isa(struct seq_file *f, const char *isa)
> >>  {
> >> -       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);
> >> +       unsigned char parse_break = 0;
> >> +       char base_isa[RISCV_ISA_EXT_BASE + 5];
> >> +       char *base_isa_end = base_isa;
> >> +       const char *isa_end = isa;
> >> +#if IS_ENABLED(CONFIG_32BIT)
> >> +       if (!strncmp(isa, "rv32", 4))
> >> +               isa_end += 4;
> >> +#elif IS_ENABLED(CONFIG_64BIT)
> >> +       if (!strncmp(isa, "rv64", 4))
> >> +               isa_end += 4;
> >> +#endif
> >> +       if (isa != isa_end) {
> >> +               strncpy(base_isa, isa, isa_end - isa);
> >> +               base_isa_end += isa_end - isa;
> >> +               for (; *isa_end; ++isa_end) {
> >> +                       switch (*isa_end++) {
> >> +                       case 'x':
> >> +                       case 'z':
> >> +                               parse_break = 1;
> >> +                               break;
> >> +                       case 's':
> >> +                               if (*isa_end != 'u') {
> >> +                                       parse_break = 1;
> >> +                                       break;
> >> +                               }
> >> +                               /**
> >> +                                * Workaround for invalid single-letter 's'
> >> +                                * (QEMU).  Should break for valid ISA string.
> >> +                                */
> >> +                               fallthrough;
> >> +                       default:
> >> +                               if (unlikely(!islower(isa_end[-1])
> >> +                                   || base_isa_end == base_isa + sizeof(base_isa))) {
> >> +                                       parse_break = 2;
> >> +                                       break;
> >> +                               }
> >> +                               switch (isa_end[-1]) {
> >> +                               case 's':
> >> +                               case 'u':
> >> +                                       break;
> >> +                               default:
> >> +                                       *base_isa_end++ = isa_end[-1];
> >> +                               }
> >> +                               /* Skip version number */
> >> +                               if (!isdigit(*isa_end))
> >> +                                       break;
> >> +                               while (isdigit(*++isa_end))
> >> +                                       ;
> >> +                               if (*isa_end != 'p')
> >> +                                       break;
> >> +                               if (!isdigit(*++isa_end)) {
> >> +                                       --isa_end;
> >> +                                       break;
> >> +                               }
> >> +                               while (isdigit(*++isa_end))
> >> +                                       ;
> >> +                               break;
> >> +                       }
> >> +                       if (*isa_end != '_')
> >> +                               --isa_end;
> >> +                       if (parse_break)
> >> +                               break;
> >> +               }
> >> +       }
> >>
> >> -       /* Print only the base ISA as it is */
> >> +       /**
> >> +        * Print only the base ISA
> >> +        * (original ISA string as is if an error is encountered)
> >> +        */
> >>         seq_puts(f, "isa\t\t: ");
> >> -       seq_write(f, isa, base_isa_len);
> >> +       if (parse_break < 2) {
> >> +               seq_write(f, base_isa, base_isa_end - base_isa);
> >> +       } else {
> >> +               isa_end += strlen(isa_end);
> >> +               seq_write(f, isa, isa_end - isa);
> >> +       }
> >>         seq_puts(f, "\n");
> >>  }
> >>
> >> --
> >> 2.32.0
> >>
> >
> >



-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v4 4/6] RISC-V: Implement multi-letter ISA extension probing framework

[Heiko Stübner]

Am Mittwoch, 16. Februar 2022, 01:29:09 CET schrieb Atish Patra:
> 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>

With my in-flight svpbmt series:
Tested-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  arch/riscv/include/asm/hwcap.h | 18 ++++++++++++++++++
>  arch/riscv/kernel/cpufeature.c | 24 ++++++++++++++++++------
>  2 files changed, 36 insertions(+), 6 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..59c70c104256 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;
> +		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
>  
>  		if (riscv_of_processor_hartid(node) < 0)
>  			continue;
> @@ -100,6 +100,7 @@ void __init riscv_fill_hwcap(void)
>  		if (!strncmp(isa, "rv64", 4))
>  			isa += 4;
>  #endif
> +		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
>  		for (; *isa; ++isa) {
>  			const char *ext = isa++;
>  			const char *ext_end = isa;
> @@ -167,12 +168,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)) {    \
> +					set_bit(bit, this_isa);			\
> +					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'));
> +				set_bit(*ext - 'a', this_isa);
>  			}
> +#undef SET_ISA_EXT_MAP
>  		}
>  
>  		/*
> @@ -185,10 +196,11 @@ void __init riscv_fill_hwcap(void)
>  		else
>  			elf_hwcap = this_hwcap;
>  
> -		if (riscv_isa[0])
> -			riscv_isa[0] &= this_isa;
> +		if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
> +			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
>  		else
> -			riscv_isa[0] = this_isa;
> +			bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> +
>  	}
>  
>  	/* We don't support systems with F but without D, so mask those out
> @@ -202,7 +214,7 @@ void __init riscv_fill_hwcap(void)
>  	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);
> +	pr_info("riscv: base ISA extensions %s\n", print_str);
>  
>  	memset(print_str, 0, sizeof(print_str));
>  	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
> 

Re: [PATCH v4 4/6] RISC-V: Implement multi-letter ISA extension probing framework

[Heiko Stübner]

Am Mittwoch, 16. Februar 2022, 01:29:09 CET schrieb Atish Patra:
> 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>

With my in-flight svpbmt series:
Tested-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  arch/riscv/include/asm/hwcap.h | 18 ++++++++++++++++++
>  arch/riscv/kernel/cpufeature.c | 24 ++++++++++++++++++------
>  2 files changed, 36 insertions(+), 6 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..59c70c104256 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;
> +		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
>  
>  		if (riscv_of_processor_hartid(node) < 0)
>  			continue;
> @@ -100,6 +100,7 @@ void __init riscv_fill_hwcap(void)
>  		if (!strncmp(isa, "rv64", 4))
>  			isa += 4;
>  #endif
> +		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
>  		for (; *isa; ++isa) {
>  			const char *ext = isa++;
>  			const char *ext_end = isa;
> @@ -167,12 +168,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)) {    \
> +					set_bit(bit, this_isa);			\
> +					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'));
> +				set_bit(*ext - 'a', this_isa);
>  			}
> +#undef SET_ISA_EXT_MAP
>  		}
>  
>  		/*
> @@ -185,10 +196,11 @@ void __init riscv_fill_hwcap(void)
>  		else
>  			elf_hwcap = this_hwcap;
>  
> -		if (riscv_isa[0])
> -			riscv_isa[0] &= this_isa;
> +		if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
> +			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
>  		else
> -			riscv_isa[0] = this_isa;
> +			bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> +
>  	}
>  
>  	/* We don't support systems with F but without D, so mask those out
> @@ -202,7 +214,7 @@ void __init riscv_fill_hwcap(void)
>  	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);
> +	pr_info("riscv: base ISA extensions %s\n", print_str);
>  
>  	memset(print_str, 0, sizeof(print_str));
>  	for (i = 0, j = 0; i < NUM_ALPHA_EXTS; i++)
> 





_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v4 5/6] RISC-V: Do no continue isa string parsing without correct XLEN

[Heiko Stübner]

Am Mittwoch, 16. Februar 2022, 01:29:10 CET schrieb Atish Patra:
> 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 59c70c104256..cb9c9e0aab31 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;
>  		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> +		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;

hmm, should (and can) this create some warning about the ignored
malformed ISA string?

Otherwise
Tested-by: Heiko Stuebner <heiko@sntech.de>

>  		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
>  		for (; *isa; ++isa) {
>  			const char *ext = isa++;
> 

Re: [PATCH v4 5/6] RISC-V: Do no continue isa string parsing without correct XLEN

[Heiko Stübner]

Am Mittwoch, 16. Februar 2022, 01:29:10 CET schrieb Atish Patra:
> 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 59c70c104256..cb9c9e0aab31 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;
>  		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> +		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;

hmm, should (and can) this create some warning about the ignored
malformed ISA string?

Otherwise
Tested-by: Heiko Stuebner <heiko@sntech.de>

>  		bitmap_zero(this_isa, RISCV_ISA_EXT_MAX);
>  		for (; *isa; ++isa) {
>  			const char *ext = isa++;
> 





_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v4 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions

[Heiko Stübner]

Am Mittwoch, 16. Februar 2022, 01:29:11 CET schrieb Atish Patra:
> 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>

Tested-by: Heiko Stuebner <heiko@sntech.de>

Re: [PATCH v4 6/6] RISC-V: Improve /proc/cpuinfo output for ISA extensions

[Heiko Stübner]

Am Mittwoch, 16. Februar 2022, 01:29:11 CET schrieb Atish Patra:
> 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>

Tested-by: Heiko Stuebner <heiko@sntech.de>



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] RISC-V: Extract base ISA from device tree

[Tsukasa OI]

On 2022/02/16 16:43, Atish Patra wrote:
> On Tue, Feb 15, 2022 at 10:59 PM Tsukasa OI
> <research_trasio@irq.a4lg.com> wrote:
>>
>> On 2022/02/16 15:01, Atish Patra wrote:
>>> On Tue, Feb 15, 2022 at 9:04 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>>>>
>>>> This commit replaces print_isa function to extract base ISA from device
>>>> tree ("riscv,isa").  It uses a subset of Tsukasa's riscv_fill_hwcap ISA
>>>> parser.
>>>>
>>>> Design choices (1):
>>>>
>>>> It constructs base ISA string from the original string (does not cut
>>>> the original string like Atish's).  This is to strip version numbers
>>>> (too minor to represent as major ISA) but this behavior may be
>>>> debatable.
>>>>
>>>
>>> A simpler solution would be to just invoke
>>> __riscv_isa_extension_available for all the base
>>> extensions (imafdc) and print the base isa string.
>>> We don't need to parse the ISA string again here.
>>
>> I agree that my patchset is ugly and simpler solution would be good.
>> (Having two parser for same string is definitely not the best idea.)
>> There are some testcases (some are not accepted by Spike's --isa option
>> and requires custom Device tree) for testing:
>>
>> *   rv64imac
>> *   rv64imacsu      (invalid ISA string; test QEMU workaround)
>> *   rv64imafdch
>> *   rv64imafdcsuh   (invalid ISA string; test QEMU workaround)
>> *   rv64imafdc_svinval_svnapot
>> *   rv64imafdcsvinval_svnapot
>> *   rv64i2p1m2a2fdcsvinval
>> *   rv64i2p1_m_afdcsvinval
>>
> 
> Thanks. This is a good set of test cases that covers most of the cases.
> 
>>>
>>>> Design choices (2):
>>>>
>>>> It skips when single-letter 'S' or 'U' is encountered.  It improves
>>>> familiarity with regular ISA string.
>>>>
>>>
>>> I don't think this is necessary because only Qemu appends 'S' & 'U' in
>>> the isa string.
>>> Your patch in Qemu will fix that for future releases of Qemu.
>>> All other commonly used platforms (hardware/spike) already use the
>>> correct ISA string.
>>
>> Yes, only QEMU.  It's not a serious problem like on cpufeature.c but would
>> be nice to remove them (when exposing to the usermode) to prevent confusion
>> on usermode side.
>>
>> If you invoke __riscv_isa_extension_available to generate base ISA string,
>> would you exclude 'S' and 'U'?
>>
> 
> Yeah. That's easy. The possible valid base extensions are "IMAFHDQCBJPV".
> (all the possible defined bits in misa except 'S' & 'U')
> So we will invoke __riscv_isa_extension_available for those only.

Order "IEMAFDQLCBKJTPVH" would be sufficient.
Note that some of them may be incompatible with Linux or currently has
no plan to be added as a single-letter extension.

"E"**: RV32E
     (incompatible with Linux?)
"L"**: Decimal floating point
     (placeholder is removed from the ISA specification)
"B"**: Bit manipulation
     (no single-letter extension plan but multiple subextensions)
"K"**: Cryptography
     (no single-letter extension plan but multiple subextensions)
"J"**: Extensions for dynamic languages
     (no single-letter extension but planned multiple subextensions)
"T"**: Transactional memory
     (placeholder is removed from the ISA specification)
"P"  : narrow SIMD using GPR
     (this order is ratified but "P" is not)
"H"  : Hypervisor (different position compared to your post)
     (this order is not ratified but likely placed here
      for compatibility with QEMU and Spike)

Tsukasa

> 
>>>
>>>> This commit is intended to be squashed into Atish's isa_framework_v4
>>>> PATCH 6/6.
>>>>
>>>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>>>> ---
>>>>  arch/riscv/kernel/cpu.c | 83 ++++++++++++++++++++++++++++++++++++-----
>>>>  1 file changed, 74 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>>>> index 6f9660c0a973..3f9607a66d95 100644
>>>> --- a/arch/riscv/kernel/cpu.c
>>>> +++ b/arch/riscv/kernel/cpu.c
>>>> @@ -101,17 +101,82 @@ static void print_isa_ext(struct seq_file *f)
>>>>
>>>>  static void print_isa(struct seq_file *f, const char *isa)
>>>>  {
>>>> -       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);
>>>> +       unsigned char parse_break = 0;
>>>> +       char base_isa[RISCV_ISA_EXT_BASE + 5];
>>>> +       char *base_isa_end = base_isa;
>>>> +       const char *isa_end = isa;
>>>> +#if IS_ENABLED(CONFIG_32BIT)
>>>> +       if (!strncmp(isa, "rv32", 4))
>>>> +               isa_end += 4;
>>>> +#elif IS_ENABLED(CONFIG_64BIT)
>>>> +       if (!strncmp(isa, "rv64", 4))
>>>> +               isa_end += 4;
>>>> +#endif
>>>> +       if (isa != isa_end) {
>>>> +               strncpy(base_isa, isa, isa_end - isa);
>>>> +               base_isa_end += isa_end - isa;
>>>> +               for (; *isa_end; ++isa_end) {
>>>> +                       switch (*isa_end++) {
>>>> +                       case 'x':
>>>> +                       case 'z':
>>>> +                               parse_break = 1;
>>>> +                               break;
>>>> +                       case 's':
>>>> +                               if (*isa_end != 'u') {
>>>> +                                       parse_break = 1;
>>>> +                                       break;
>>>> +                               }
>>>> +                               /**
>>>> +                                * Workaround for invalid single-letter 's'
>>>> +                                * (QEMU).  Should break for valid ISA string.
>>>> +                                */
>>>> +                               fallthrough;
>>>> +                       default:
>>>> +                               if (unlikely(!islower(isa_end[-1])
>>>> +                                   || base_isa_end == base_isa + sizeof(base_isa))) {
>>>> +                                       parse_break = 2;
>>>> +                                       break;
>>>> +                               }
>>>> +                               switch (isa_end[-1]) {
>>>> +                               case 's':
>>>> +                               case 'u':
>>>> +                                       break;
>>>> +                               default:
>>>> +                                       *base_isa_end++ = isa_end[-1];
>>>> +                               }
>>>> +                               /* Skip version number */
>>>> +                               if (!isdigit(*isa_end))
>>>> +                                       break;
>>>> +                               while (isdigit(*++isa_end))
>>>> +                                       ;
>>>> +                               if (*isa_end != 'p')
>>>> +                                       break;
>>>> +                               if (!isdigit(*++isa_end)) {
>>>> +                                       --isa_end;
>>>> +                                       break;
>>>> +                               }
>>>> +                               while (isdigit(*++isa_end))
>>>> +                                       ;
>>>> +                               break;
>>>> +                       }
>>>> +                       if (*isa_end != '_')
>>>> +                               --isa_end;
>>>> +                       if (parse_break)
>>>> +                               break;
>>>> +               }
>>>> +       }
>>>>
>>>> -       /* Print only the base ISA as it is */
>>>> +       /**
>>>> +        * Print only the base ISA
>>>> +        * (original ISA string as is if an error is encountered)
>>>> +        */
>>>>         seq_puts(f, "isa\t\t: ");
>>>> -       seq_write(f, isa, base_isa_len);
>>>> +       if (parse_break < 2) {
>>>> +               seq_write(f, base_isa, base_isa_end - base_isa);
>>>> +       } else {
>>>> +               isa_end += strlen(isa_end);
>>>> +               seq_write(f, isa, isa_end - isa);
>>>> +       }
>>>>         seq_puts(f, "\n");
>>>>  }
>>>>
>>>> --
>>>> 2.32.0
>>>>
>>>
>>>
> 
> 
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] RISC-V: Extract base ISA from device tree

[Atish Patra]

On Mon, Feb 21, 2022 at 5:42 AM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> On 2022/02/16 16:43, Atish Patra wrote:
> > On Tue, Feb 15, 2022 at 10:59 PM Tsukasa OI
> > <research_trasio@irq.a4lg.com> wrote:
> >>
> >> On 2022/02/16 15:01, Atish Patra wrote:
> >>> On Tue, Feb 15, 2022 at 9:04 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
> >>>>
> >>>> This commit replaces print_isa function to extract base ISA from device
> >>>> tree ("riscv,isa").  It uses a subset of Tsukasa's riscv_fill_hwcap ISA
> >>>> parser.
> >>>>
> >>>> Design choices (1):
> >>>>
> >>>> It constructs base ISA string from the original string (does not cut
> >>>> the original string like Atish's).  This is to strip version numbers
> >>>> (too minor to represent as major ISA) but this behavior may be
> >>>> debatable.
> >>>>
> >>>
> >>> A simpler solution would be to just invoke
> >>> __riscv_isa_extension_available for all the base
> >>> extensions (imafdc) and print the base isa string.
> >>> We don't need to parse the ISA string again here.
> >>
> >> I agree that my patchset is ugly and simpler solution would be good.
> >> (Having two parser for same string is definitely not the best idea.)
> >> There are some testcases (some are not accepted by Spike's --isa option
> >> and requires custom Device tree) for testing:
> >>
> >> *   rv64imac
> >> *   rv64imacsu      (invalid ISA string; test QEMU workaround)
> >> *   rv64imafdch
> >> *   rv64imafdcsuh   (invalid ISA string; test QEMU workaround)
> >> *   rv64imafdc_svinval_svnapot
> >> *   rv64imafdcsvinval_svnapot
> >> *   rv64i2p1m2a2fdcsvinval
> >> *   rv64i2p1_m_afdcsvinval
> >>
> >
> > Thanks. This is a good set of test cases that covers most of the cases.
> >
> >>>
> >>>> Design choices (2):
> >>>>
> >>>> It skips when single-letter 'S' or 'U' is encountered.  It improves
> >>>> familiarity with regular ISA string.
> >>>>
> >>>
> >>> I don't think this is necessary because only Qemu appends 'S' & 'U' in
> >>> the isa string.
> >>> Your patch in Qemu will fix that for future releases of Qemu.
> >>> All other commonly used platforms (hardware/spike) already use the
> >>> correct ISA string.
> >>
> >> Yes, only QEMU.  It's not a serious problem like on cpufeature.c but would
> >> be nice to remove them (when exposing to the usermode) to prevent confusion
> >> on usermode side.
> >>
> >> If you invoke __riscv_isa_extension_available to generate base ISA string,
> >> would you exclude 'S' and 'U'?
> >>
> >
> > Yeah. That's easy. The possible valid base extensions are "IMAFHDQCBJPV".
> > (all the possible defined bits in misa except 'S' & 'U')
> > So we will invoke __riscv_isa_extension_available for those only.
>
> Order "IEMAFDQLCBKJTPVH" would be sufficient.
> Note that some of them may be incompatible with Linux or currently has
> no plan to be added as a single-letter extension.
>
> "E"**: RV32E
>      (incompatible with Linux?)

Yes. No need to check for this.

> "L"**: Decimal floating point
>      (placeholder is removed from the ISA specification)
> "B"**: Bit manipulation
>      (no single-letter extension plan but multiple subextensions)
> "K"**: Cryptography
>      (no single-letter extension plan but multiple subextensions)
> "J"**: Extensions for dynamic languages
>      (no single-letter extension but planned multiple subextensions)
> "T"**: Transactional memory
>      (placeholder is removed from the ISA specification)

That's why I did not include L & T as the ISA spec doesn't talk about
it anymore.
I am in favor of not including right now and add it later if any
future ratified spec includes it.

> "P"  : narrow SIMD using GPR
>      (this order is ratified but "P" is not)
> "H"  : Hypervisor (different position compared to your post)
>      (this order is not ratified but likely placed here
>       for compatibility with QEMU and Spike)
>

I thought it would be after "imafdqc". I will change it as per your suggestion.
We can always fix if the ratified spec will specify a different order.

This is what base ISA extension (single letter) will look like (for now)

"imafdqcbkjpvh"

I will send the patch shortly.

> Tsukasa
>
> >
> >>>
> >>>> This commit is intended to be squashed into Atish's isa_framework_v4
> >>>> PATCH 6/6.
> >>>>
> >>>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> >>>> ---
> >>>>  arch/riscv/kernel/cpu.c | 83 ++++++++++++++++++++++++++++++++++++-----
> >>>>  1 file changed, 74 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> >>>> index 6f9660c0a973..3f9607a66d95 100644
> >>>> --- a/arch/riscv/kernel/cpu.c
> >>>> +++ b/arch/riscv/kernel/cpu.c
> >>>> @@ -101,17 +101,82 @@ static void print_isa_ext(struct seq_file *f)
> >>>>
> >>>>  static void print_isa(struct seq_file *f, const char *isa)
> >>>>  {
> >>>> -       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);
> >>>> +       unsigned char parse_break = 0;
> >>>> +       char base_isa[RISCV_ISA_EXT_BASE + 5];
> >>>> +       char *base_isa_end = base_isa;
> >>>> +       const char *isa_end = isa;
> >>>> +#if IS_ENABLED(CONFIG_32BIT)
> >>>> +       if (!strncmp(isa, "rv32", 4))
> >>>> +               isa_end += 4;
> >>>> +#elif IS_ENABLED(CONFIG_64BIT)
> >>>> +       if (!strncmp(isa, "rv64", 4))
> >>>> +               isa_end += 4;
> >>>> +#endif
> >>>> +       if (isa != isa_end) {
> >>>> +               strncpy(base_isa, isa, isa_end - isa);
> >>>> +               base_isa_end += isa_end - isa;
> >>>> +               for (; *isa_end; ++isa_end) {
> >>>> +                       switch (*isa_end++) {
> >>>> +                       case 'x':
> >>>> +                       case 'z':
> >>>> +                               parse_break = 1;
> >>>> +                               break;
> >>>> +                       case 's':
> >>>> +                               if (*isa_end != 'u') {
> >>>> +                                       parse_break = 1;
> >>>> +                                       break;
> >>>> +                               }
> >>>> +                               /**
> >>>> +                                * Workaround for invalid single-letter 's'
> >>>> +                                * (QEMU).  Should break for valid ISA string.
> >>>> +                                */
> >>>> +                               fallthrough;
> >>>> +                       default:
> >>>> +                               if (unlikely(!islower(isa_end[-1])
> >>>> +                                   || base_isa_end == base_isa + sizeof(base_isa))) {
> >>>> +                                       parse_break = 2;
> >>>> +                                       break;
> >>>> +                               }
> >>>> +                               switch (isa_end[-1]) {
> >>>> +                               case 's':
> >>>> +                               case 'u':
> >>>> +                                       break;
> >>>> +                               default:
> >>>> +                                       *base_isa_end++ = isa_end[-1];
> >>>> +                               }
> >>>> +                               /* Skip version number */
> >>>> +                               if (!isdigit(*isa_end))
> >>>> +                                       break;
> >>>> +                               while (isdigit(*++isa_end))
> >>>> +                                       ;
> >>>> +                               if (*isa_end != 'p')
> >>>> +                                       break;
> >>>> +                               if (!isdigit(*++isa_end)) {
> >>>> +                                       --isa_end;
> >>>> +                                       break;
> >>>> +                               }
> >>>> +                               while (isdigit(*++isa_end))
> >>>> +                                       ;
> >>>> +                               break;
> >>>> +                       }
> >>>> +                       if (*isa_end != '_')
> >>>> +                               --isa_end;
> >>>> +                       if (parse_break)
> >>>> +                               break;
> >>>> +               }
> >>>> +       }
> >>>>
> >>>> -       /* Print only the base ISA as it is */
> >>>> +       /**
> >>>> +        * Print only the base ISA
> >>>> +        * (original ISA string as is if an error is encountered)
> >>>> +        */
> >>>>         seq_puts(f, "isa\t\t: ");
> >>>> -       seq_write(f, isa, base_isa_len);
> >>>> +       if (parse_break < 2) {
> >>>> +               seq_write(f, base_isa, base_isa_end - base_isa);
> >>>> +       } else {
> >>>> +               isa_end += strlen(isa_end);
> >>>> +               seq_write(f, isa, isa_end - isa);
> >>>> +       }
> >>>>         seq_puts(f, "\n");
> >>>>  }
> >>>>
> >>>> --
> >>>> 2.32.0
> >>>>
> >>>
> >>>
> >
> >
> >



-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv