[PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Atish Patra]

This series implements a generic framework to parse multi-letter ISA
extensions. It introduces a new DT node that can be under /cpus or
individual cpu depends on the platforms with homogeneous or heterogeneous 
harts. This version of the series only allows adds support for homogeneous
harts as there are no platforms with heterogeneous harts yet. However,
the DT binding allows both. 

The patch also indicates the user space about 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[1] adds the support for sscofpmf extension.

[1] https://github.com/atishp04/linux/commit/a23157264118d6fd905fd08d8717c7df03078bb1

Atish Patra (2):
RISC-V: Provide a framework for parsing multi-letter ISA extensions
dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

.../devicetree/bindings/riscv/cpus.yaml       |  9 +++
arch/riscv/include/asm/hwcap.h                | 31 ++++++++++
arch/riscv/kernel/cpu.c                       | 16 +++++
arch/riscv/kernel/cpufeature.c                | 58 ++++++++++++++++++-
4 files changed, 113 insertions(+), 1 deletion(-)

--
2.33.1

[PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Atish Patra]

This series implements a generic framework to parse multi-letter ISA
extensions. It introduces a new DT node that can be under /cpus or
individual cpu depends on the platforms with homogeneous or heterogeneous 
harts. This version of the series only allows adds support for homogeneous
harts as there are no platforms with heterogeneous harts yet. However,
the DT binding allows both. 

The patch also indicates the user space about 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[1] adds the support for sscofpmf extension.

[1] https://github.com/atishp04/linux/commit/a23157264118d6fd905fd08d8717c7df03078bb1

Atish Patra (2):
RISC-V: Provide a framework for parsing multi-letter ISA extensions
dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

.../devicetree/bindings/riscv/cpus.yaml       |  9 +++
arch/riscv/include/asm/hwcap.h                | 31 ++++++++++
arch/riscv/kernel/cpu.c                       | 16 +++++
arch/riscv/kernel/cpufeature.c                | 58 ++++++++++++++++++-
4 files changed, 113 insertions(+), 1 deletion(-)

--
2.33.1


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

[PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Atish Patra]

Recently, there were 15 specifications/40 ISA extensions were ratified.
Except hypervisor ('H') extension, all of them are multi-letter extensions.
Going forward, there will be more number of multi-letter extensions as
well. Parsing all of these extensions from ISA string is not scalable.
Thus, this patch provides a DT based framework to for easy parsing and
querying of any ISA extensions. It facilitates custom user visible strings
for the ISA extensions via /proc/cpuinfo as well.

Currently, there are no platforms with heterogeneous Linux capable harts.
That's why, this patch supports only a single DT node which can only work
for systems with homogeneous harts. To support heterogeneous systems, this
cpu node must be a subnode for each cpu.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/include/asm/hwcap.h | 31 ++++++++++++++++++
 arch/riscv/kernel/cpu.c        | 16 ++++++++++
 arch/riscv/kernel/cpufeature.c | 58 +++++++++++++++++++++++++++++++++-
 3 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 5ce50468aff1..368ab0f330c8 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -34,7 +34,36 @@ 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,
+};
+
+struct riscv_isa_ext_data {
+	struct list_head node;
+	/* Name of the extension property in DT */
+	char dtprop[RISCV_ISA_EXT_NAME_LEN_MAX];
+	/* 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;
+};
+
+extern struct list_head riscv_isa_ext_list;
 
 unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
 
@@ -44,6 +73,8 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
 #define riscv_isa_extension_available(isa_bitmap, ext)	\
 	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
 
+int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata);
+
 #endif
 
 #endif /* _ASM_RISCV_HWCAP_H */
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index f13b2c9ea912..dad95bdd3cca 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -71,6 +71,21 @@ static void print_isa(struct seq_file *f, const char *isa)
 	seq_puts(f, "\n");
 }
 
+static void print_isa_ext(struct seq_file *f)
+{
+	struct riscv_isa_ext_data *edata;
+	int count = 0;
+
+	seq_puts(f, "isa-ext\t\t: ");
+	list_for_each_entry(edata, &riscv_isa_ext_list, node) {
+		if (count)
+			seq_puts(f, ",");
+		seq_write(f, edata->uprop, strnlen(edata->uprop, RISCV_ISA_EXT_NAME_LEN_MAX));
+		count++;
+	}
+	seq_puts(f, "\n");
+}
+
 static void print_mmu(struct seq_file *f, const char *mmu_type)
 {
 #if defined(CONFIG_32BIT)
@@ -113,6 +128,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);
 	if (!of_property_read_string(node, "mmu-type", &mmu))
 		print_mmu(m, mmu);
 	if (!of_property_read_string(node, "compatible", &compat)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index d959d207a40d..c70eeec17f5b 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -7,6 +7,8 @@
  */
 
 #include <linux/bitmap.h>
+#include <linux/list.h>
+#include <linux/slab.h>
 #include <linux/of.h>
 #include <asm/processor.h>
 #include <asm/hwcap.h>
@@ -18,6 +20,8 @@ unsigned long elf_hwcap __read_mostly;
 /* Host ISA bitmap */
 static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
 
+LIST_HEAD(riscv_isa_ext_list);
+
 #ifdef CONFIG_FPU
 __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
 #endif
@@ -59,12 +63,60 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit)
 }
 EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
 
+#define __RISCV_ISA_EXT_DATA(DTPROP, UPROP, EXTID) \
+	{							\
+		.dtprop = #DTPROP,				\
+		.uprop = #UPROP,				\
+		.isa_ext_id = EXTID,				\
+	}
+
+static struct riscv_isa_ext_data isa_ext_arr[] = {
+	__RISCV_ISA_EXT_DATA("", "", RISCV_ISA_EXT_MAX),
+};
+
+int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
+{
+	struct device_node *node, *enode;
+	int eid;
+
+	if (!edata || !edata->dtprop)
+		return -EINVAL;
+
+	node = of_find_node_by_path("/cpus");
+	if (!node) {
+		pr_err("No CPU information found in DT\n");
+		return -ENOENT;
+	}
+
+	enode = of_get_child_by_name(node, "riscv,isa-ext");
+	if (!enode) {
+		pr_err("No riscv-isa-ext found in DT\n");
+		return -ENOENT;
+	}
+
+	eid = edata->isa_ext_id;
+	if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
+		return -EINVAL;
+
+	if (!of_property_read_bool(enode, edata->dtprop)) {
+		pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
+		return -ENODEV;
+	}
+
+	/* Enable the extension id in the riscv_isa for easier probing */
+	riscv_isa[0] |= 1 << eid;
+	list_add(&edata->node, &riscv_isa_ext_list);
+	pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
+
+	return 0;
+}
+
 void __init riscv_fill_hwcap(void)
 {
 	struct device_node *node;
 	const char *isa;
 	char print_str[BITS_PER_LONG + 1];
-	size_t i, j, isa_len;
+	size_t i, j, isa_len, ext_arr_sz;
 	static unsigned long isa2hwcap[256] = {0};
 
 	isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
@@ -148,4 +200,8 @@ void __init riscv_fill_hwcap(void)
 	if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))
 		static_branch_enable(&cpu_hwcap_fpu);
 #endif
+
+	ext_arr_sz = ARRAY_SIZE(isa_ext_arr);
+	for (i = 0; i < ext_arr_sz - 1; i++)
+		riscv_isa_ext_list_add(&isa_ext_arr[i]);
 }
-- 
2.33.1

[PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Atish Patra]

Recently, there were 15 specifications/40 ISA extensions were ratified.
Except hypervisor ('H') extension, all of them are multi-letter extensions.
Going forward, there will be more number of multi-letter extensions as
well. Parsing all of these extensions from ISA string is not scalable.
Thus, this patch provides a DT based framework to for easy parsing and
querying of any ISA extensions. It facilitates custom user visible strings
for the ISA extensions via /proc/cpuinfo as well.

Currently, there are no platforms with heterogeneous Linux capable harts.
That's why, this patch supports only a single DT node which can only work
for systems with homogeneous harts. To support heterogeneous systems, this
cpu node must be a subnode for each cpu.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/include/asm/hwcap.h | 31 ++++++++++++++++++
 arch/riscv/kernel/cpu.c        | 16 ++++++++++
 arch/riscv/kernel/cpufeature.c | 58 +++++++++++++++++++++++++++++++++-
 3 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 5ce50468aff1..368ab0f330c8 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -34,7 +34,36 @@ 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,
+};
+
+struct riscv_isa_ext_data {
+	struct list_head node;
+	/* Name of the extension property in DT */
+	char dtprop[RISCV_ISA_EXT_NAME_LEN_MAX];
+	/* 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;
+};
+
+extern struct list_head riscv_isa_ext_list;
 
 unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
 
@@ -44,6 +73,8 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
 #define riscv_isa_extension_available(isa_bitmap, ext)	\
 	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
 
+int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata);
+
 #endif
 
 #endif /* _ASM_RISCV_HWCAP_H */
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index f13b2c9ea912..dad95bdd3cca 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -71,6 +71,21 @@ static void print_isa(struct seq_file *f, const char *isa)
 	seq_puts(f, "\n");
 }
 
+static void print_isa_ext(struct seq_file *f)
+{
+	struct riscv_isa_ext_data *edata;
+	int count = 0;
+
+	seq_puts(f, "isa-ext\t\t: ");
+	list_for_each_entry(edata, &riscv_isa_ext_list, node) {
+		if (count)
+			seq_puts(f, ",");
+		seq_write(f, edata->uprop, strnlen(edata->uprop, RISCV_ISA_EXT_NAME_LEN_MAX));
+		count++;
+	}
+	seq_puts(f, "\n");
+}
+
 static void print_mmu(struct seq_file *f, const char *mmu_type)
 {
 #if defined(CONFIG_32BIT)
@@ -113,6 +128,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);
 	if (!of_property_read_string(node, "mmu-type", &mmu))
 		print_mmu(m, mmu);
 	if (!of_property_read_string(node, "compatible", &compat)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index d959d207a40d..c70eeec17f5b 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -7,6 +7,8 @@
  */
 
 #include <linux/bitmap.h>
+#include <linux/list.h>
+#include <linux/slab.h>
 #include <linux/of.h>
 #include <asm/processor.h>
 #include <asm/hwcap.h>
@@ -18,6 +20,8 @@ unsigned long elf_hwcap __read_mostly;
 /* Host ISA bitmap */
 static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
 
+LIST_HEAD(riscv_isa_ext_list);
+
 #ifdef CONFIG_FPU
 __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
 #endif
@@ -59,12 +63,60 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit)
 }
 EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
 
+#define __RISCV_ISA_EXT_DATA(DTPROP, UPROP, EXTID) \
+	{							\
+		.dtprop = #DTPROP,				\
+		.uprop = #UPROP,				\
+		.isa_ext_id = EXTID,				\
+	}
+
+static struct riscv_isa_ext_data isa_ext_arr[] = {
+	__RISCV_ISA_EXT_DATA("", "", RISCV_ISA_EXT_MAX),
+};
+
+int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
+{
+	struct device_node *node, *enode;
+	int eid;
+
+	if (!edata || !edata->dtprop)
+		return -EINVAL;
+
+	node = of_find_node_by_path("/cpus");
+	if (!node) {
+		pr_err("No CPU information found in DT\n");
+		return -ENOENT;
+	}
+
+	enode = of_get_child_by_name(node, "riscv,isa-ext");
+	if (!enode) {
+		pr_err("No riscv-isa-ext found in DT\n");
+		return -ENOENT;
+	}
+
+	eid = edata->isa_ext_id;
+	if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
+		return -EINVAL;
+
+	if (!of_property_read_bool(enode, edata->dtprop)) {
+		pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
+		return -ENODEV;
+	}
+
+	/* Enable the extension id in the riscv_isa for easier probing */
+	riscv_isa[0] |= 1 << eid;
+	list_add(&edata->node, &riscv_isa_ext_list);
+	pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
+
+	return 0;
+}
+
 void __init riscv_fill_hwcap(void)
 {
 	struct device_node *node;
 	const char *isa;
 	char print_str[BITS_PER_LONG + 1];
-	size_t i, j, isa_len;
+	size_t i, j, isa_len, ext_arr_sz;
 	static unsigned long isa2hwcap[256] = {0};
 
 	isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
@@ -148,4 +200,8 @@ void __init riscv_fill_hwcap(void)
 	if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))
 		static_branch_enable(&cpu_hwcap_fpu);
 #endif
+
+	ext_arr_sz = ARRAY_SIZE(isa_ext_arr);
+	for (i = 0; i < ext_arr_sz - 1; i++)
+		riscv_isa_ext_list_add(&isa_ext_arr[i]);
 }
-- 
2.33.1


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

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Tsukasa OI]

Hello Atish,

On 2021/12/25 6:16, Atish Patra wrote:
> Recently, there were 15 specifications/40 ISA extensions were ratified.
> Except hypervisor ('H') extension, all of them are multi-letter extensions.
> Going forward, there will be more number of multi-letter extensions as
> well. Parsing all of these extensions from ISA string is not scalable.

Accepting multi-letter extensions and providing framework is very nice.

But would you enlighten me how and why "parsing all of these extensions
from ISA string is not scalable"? That's the only thing I'm not convinced.

Parsing ISA string is per-hart but one-time only (in kernel space) and
doesn't take much long.  Yes, the ISA string will get longer and longer
over time (which might affect the boot performance on many-core systems)
but once we get heterogeneous systems (as you mentioned), we will have
per-hart "isa-ext" anyway and we will have pretty much the same situation.
We could take per-group approach instead of per-hart for efficiency but
that means ISA string parsing can be optimized just as "isa-ext".

Current RISC-V systems have not long ISA string and not many cores (making
per-hart ISA string processing not a problem).  Future complex RISC-V
systems may have many cores, heterogeneous capabilities and long ISA
strings that need to be taken care.  However, "isa-ext" will have a
similar scaling issue over time (just as the ISA string).

So, I don't see any merits of "isa-ext" in the long or short term.

Regards,


> Thus, this patch provides a DT based framework to for easy parsing and
> querying of any ISA extensions. It facilitates custom user visible strings
> for the ISA extensions via /proc/cpuinfo as well.
> 
> Currently, there are no platforms with heterogeneous Linux capable harts.
> That's why, this patch supports only a single DT node which can only work
> for systems with homogeneous harts. To support heterogeneous systems, this
> cpu node must be a subnode for each cpu.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  arch/riscv/include/asm/hwcap.h | 31 ++++++++++++++++++
>  arch/riscv/kernel/cpu.c        | 16 ++++++++++
>  arch/riscv/kernel/cpufeature.c | 58 +++++++++++++++++++++++++++++++++-
>  3 files changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5ce50468aff1..368ab0f330c8 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -34,7 +34,36 @@ 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,
> +};
> +
> +struct riscv_isa_ext_data {
> +	struct list_head node;
> +	/* Name of the extension property in DT */
> +	char dtprop[RISCV_ISA_EXT_NAME_LEN_MAX];
> +	/* 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;
> +};
> +
> +extern struct list_head riscv_isa_ext_list;
>  
>  unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
>  
> @@ -44,6 +73,8 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>  #define riscv_isa_extension_available(isa_bitmap, ext)	\
>  	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>  
> +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata);
> +
>  #endif
>  
>  #endif /* _ASM_RISCV_HWCAP_H */
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index f13b2c9ea912..dad95bdd3cca 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -71,6 +71,21 @@ static void print_isa(struct seq_file *f, const char *isa)
>  	seq_puts(f, "\n");
>  }
>  
> +static void print_isa_ext(struct seq_file *f)
> +{
> +	struct riscv_isa_ext_data *edata;
> +	int count = 0;
> +
> +	seq_puts(f, "isa-ext\t\t: ");
> +	list_for_each_entry(edata, &riscv_isa_ext_list, node) {
> +		if (count)
> +			seq_puts(f, ",");
> +		seq_write(f, edata->uprop, strnlen(edata->uprop, RISCV_ISA_EXT_NAME_LEN_MAX));
> +		count++;
> +	}
> +	seq_puts(f, "\n");
> +}
> +
>  static void print_mmu(struct seq_file *f, const char *mmu_type)
>  {
>  #if defined(CONFIG_32BIT)
> @@ -113,6 +128,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);
>  	if (!of_property_read_string(node, "mmu-type", &mmu))
>  		print_mmu(m, mmu);
>  	if (!of_property_read_string(node, "compatible", &compat)
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index d959d207a40d..c70eeec17f5b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -7,6 +7,8 @@
>   */
>  
>  #include <linux/bitmap.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
>  #include <linux/of.h>
>  #include <asm/processor.h>
>  #include <asm/hwcap.h>
> @@ -18,6 +20,8 @@ unsigned long elf_hwcap __read_mostly;
>  /* Host ISA bitmap */
>  static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>  
> +LIST_HEAD(riscv_isa_ext_list);
> +
>  #ifdef CONFIG_FPU
>  __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
>  #endif
> @@ -59,12 +63,60 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit)
>  }
>  EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
>  
> +#define __RISCV_ISA_EXT_DATA(DTPROP, UPROP, EXTID) \
> +	{							\
> +		.dtprop = #DTPROP,				\
> +		.uprop = #UPROP,				\
> +		.isa_ext_id = EXTID,				\
> +	}
> +
> +static struct riscv_isa_ext_data isa_ext_arr[] = {
> +	__RISCV_ISA_EXT_DATA("", "", RISCV_ISA_EXT_MAX),
> +};
> +
> +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
> +{
> +	struct device_node *node, *enode;
> +	int eid;
> +
> +	if (!edata || !edata->dtprop)
> +		return -EINVAL;
> +
> +	node = of_find_node_by_path("/cpus");
> +	if (!node) {
> +		pr_err("No CPU information found in DT\n");
> +		return -ENOENT;
> +	}
> +
> +	enode = of_get_child_by_name(node, "riscv,isa-ext");
> +	if (!enode) {
> +		pr_err("No riscv-isa-ext found in DT\n");
> +		return -ENOENT;
> +	}
> +
> +	eid = edata->isa_ext_id;
> +	if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
> +		return -EINVAL;
> +
> +	if (!of_property_read_bool(enode, edata->dtprop)) {
> +		pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
> +		return -ENODEV;
> +	}
> +
> +	/* Enable the extension id in the riscv_isa for easier probing */
> +	riscv_isa[0] |= 1 << eid;
> +	list_add(&edata->node, &riscv_isa_ext_list);
> +	pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
> +
> +	return 0;
> +}
> +
>  void __init riscv_fill_hwcap(void)
>  {
>  	struct device_node *node;
>  	const char *isa;
>  	char print_str[BITS_PER_LONG + 1];
> -	size_t i, j, isa_len;
> +	size_t i, j, isa_len, ext_arr_sz;
>  	static unsigned long isa2hwcap[256] = {0};
>  
>  	isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
> @@ -148,4 +200,8 @@ void __init riscv_fill_hwcap(void)
>  	if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))
>  		static_branch_enable(&cpu_hwcap_fpu);
>  #endif
> +
> +	ext_arr_sz = ARRAY_SIZE(isa_ext_arr);
> +	for (i = 0; i < ext_arr_sz - 1; i++)
> +		riscv_isa_ext_list_add(&isa_ext_arr[i]);
>  }

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

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Tsukasa OI]

Hello Atish,

On 2021/12/25 6:16, Atish Patra wrote:
> Recently, there were 15 specifications/40 ISA extensions were ratified.
> Except hypervisor ('H') extension, all of them are multi-letter extensions.
> Going forward, there will be more number of multi-letter extensions as
> well. Parsing all of these extensions from ISA string is not scalable.

Accepting multi-letter extensions and providing framework is very nice.

But would you enlighten me how and why "parsing all of these extensions
from ISA string is not scalable"? That's the only thing I'm not convinced.

Parsing ISA string is per-hart but one-time only (in kernel space) and
doesn't take much long.  Yes, the ISA string will get longer and longer
over time (which might affect the boot performance on many-core systems)
but once we get heterogeneous systems (as you mentioned), we will have
per-hart "isa-ext" anyway and we will have pretty much the same situation.
We could take per-group approach instead of per-hart for efficiency but
that means ISA string parsing can be optimized just as "isa-ext".

Current RISC-V systems have not long ISA string and not many cores (making
per-hart ISA string processing not a problem).  Future complex RISC-V
systems may have many cores, heterogeneous capabilities and long ISA
strings that need to be taken care.  However, "isa-ext" will have a
similar scaling issue over time (just as the ISA string).

So, I don't see any merits of "isa-ext" in the long or short term.

Regards,


> Thus, this patch provides a DT based framework to for easy parsing and
> querying of any ISA extensions. It facilitates custom user visible strings
> for the ISA extensions via /proc/cpuinfo as well.
> 
> Currently, there are no platforms with heterogeneous Linux capable harts.
> That's why, this patch supports only a single DT node which can only work
> for systems with homogeneous harts. To support heterogeneous systems, this
> cpu node must be a subnode for each cpu.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  arch/riscv/include/asm/hwcap.h | 31 ++++++++++++++++++
>  arch/riscv/kernel/cpu.c        | 16 ++++++++++
>  arch/riscv/kernel/cpufeature.c | 58 +++++++++++++++++++++++++++++++++-
>  3 files changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5ce50468aff1..368ab0f330c8 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -34,7 +34,36 @@ 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,
> +};
> +
> +struct riscv_isa_ext_data {
> +	struct list_head node;
> +	/* Name of the extension property in DT */
> +	char dtprop[RISCV_ISA_EXT_NAME_LEN_MAX];
> +	/* 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;
> +};
> +
> +extern struct list_head riscv_isa_ext_list;
>  
>  unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
>  
> @@ -44,6 +73,8 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>  #define riscv_isa_extension_available(isa_bitmap, ext)	\
>  	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>  
> +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata);
> +
>  #endif
>  
>  #endif /* _ASM_RISCV_HWCAP_H */
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index f13b2c9ea912..dad95bdd3cca 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -71,6 +71,21 @@ static void print_isa(struct seq_file *f, const char *isa)
>  	seq_puts(f, "\n");
>  }
>  
> +static void print_isa_ext(struct seq_file *f)
> +{
> +	struct riscv_isa_ext_data *edata;
> +	int count = 0;
> +
> +	seq_puts(f, "isa-ext\t\t: ");
> +	list_for_each_entry(edata, &riscv_isa_ext_list, node) {
> +		if (count)
> +			seq_puts(f, ",");
> +		seq_write(f, edata->uprop, strnlen(edata->uprop, RISCV_ISA_EXT_NAME_LEN_MAX));
> +		count++;
> +	}
> +	seq_puts(f, "\n");
> +}
> +
>  static void print_mmu(struct seq_file *f, const char *mmu_type)
>  {
>  #if defined(CONFIG_32BIT)
> @@ -113,6 +128,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);
>  	if (!of_property_read_string(node, "mmu-type", &mmu))
>  		print_mmu(m, mmu);
>  	if (!of_property_read_string(node, "compatible", &compat)
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index d959d207a40d..c70eeec17f5b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -7,6 +7,8 @@
>   */
>  
>  #include <linux/bitmap.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
>  #include <linux/of.h>
>  #include <asm/processor.h>
>  #include <asm/hwcap.h>
> @@ -18,6 +20,8 @@ unsigned long elf_hwcap __read_mostly;
>  /* Host ISA bitmap */
>  static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>  
> +LIST_HEAD(riscv_isa_ext_list);
> +
>  #ifdef CONFIG_FPU
>  __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
>  #endif
> @@ -59,12 +63,60 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit)
>  }
>  EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
>  
> +#define __RISCV_ISA_EXT_DATA(DTPROP, UPROP, EXTID) \
> +	{							\
> +		.dtprop = #DTPROP,				\
> +		.uprop = #UPROP,				\
> +		.isa_ext_id = EXTID,				\
> +	}
> +
> +static struct riscv_isa_ext_data isa_ext_arr[] = {
> +	__RISCV_ISA_EXT_DATA("", "", RISCV_ISA_EXT_MAX),
> +};
> +
> +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
> +{
> +	struct device_node *node, *enode;
> +	int eid;
> +
> +	if (!edata || !edata->dtprop)
> +		return -EINVAL;
> +
> +	node = of_find_node_by_path("/cpus");
> +	if (!node) {
> +		pr_err("No CPU information found in DT\n");
> +		return -ENOENT;
> +	}
> +
> +	enode = of_get_child_by_name(node, "riscv,isa-ext");
> +	if (!enode) {
> +		pr_err("No riscv-isa-ext found in DT\n");
> +		return -ENOENT;
> +	}
> +
> +	eid = edata->isa_ext_id;
> +	if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
> +		return -EINVAL;
> +
> +	if (!of_property_read_bool(enode, edata->dtprop)) {
> +		pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
> +		return -ENODEV;
> +	}
> +
> +	/* Enable the extension id in the riscv_isa for easier probing */
> +	riscv_isa[0] |= 1 << eid;
> +	list_add(&edata->node, &riscv_isa_ext_list);
> +	pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
> +
> +	return 0;
> +}
> +
>  void __init riscv_fill_hwcap(void)
>  {
>  	struct device_node *node;
>  	const char *isa;
>  	char print_str[BITS_PER_LONG + 1];
> -	size_t i, j, isa_len;
> +	size_t i, j, isa_len, ext_arr_sz;
>  	static unsigned long isa2hwcap[256] = {0};
>  
>  	isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
> @@ -148,4 +200,8 @@ void __init riscv_fill_hwcap(void)
>  	if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))
>  		static_branch_enable(&cpu_hwcap_fpu);
>  #endif
> +
> +	ext_arr_sz = ARRAY_SIZE(isa_ext_arr);
> +	for (i = 0; i < ext_arr_sz - 1; i++)
> +		riscv_isa_ext_list_add(&isa_ext_arr[i]);
>  }

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Atish Patra]

On Fri, Dec 24, 2021 at 7:13 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Hello Atish,
>
> On 2021/12/25 6:16, Atish Patra wrote:
> > Recently, there were 15 specifications/40 ISA extensions were ratified.
> > Except hypervisor ('H') extension, all of them are multi-letter extensions.
> > Going forward, there will be more number of multi-letter extensions as
> > well. Parsing all of these extensions from ISA string is not scalable.
>
> Accepting multi-letter extensions and providing framework is very nice.
>
> But would you enlighten me how and why "parsing all of these extensions
> from ISA string is not scalable"? That's the only thing I'm not convinced.
>

I thought we already discussed the merits in this thread.
https://lore.kernel.org/lkml/9fa0492d-b448-045b-ba8f-e3d68b46d673@irq.a4lg.com/

Anyways, here is summary of disadvantages of "riscv,isa" string
parsing that you proposed

1. The isa string in DT will be really really long. It is time
consuming to parse it and becomes
somewhat unreadable after a point.
2. Currently, /proc/cpuinfo dumps the entire riscv,isa string. That
works for base/single letter extensions.
However, dumping the entire string will not work in userspace. Imagine
the 10~20 extensions string in your terminal.
3. If you want to group them into different extensions, you have to
parse them again and create separate extensions
which I am doing in this patch.
4. In future, there will be subnodes in the isa-ext which will only
describe the ISA-extensions corresponding to Sv* or Sh* or Z*
groups. I did not add those in this version because the unpriv
specification doesn't define them clearly and may change in the future
(as suggested by Greg [1].)

[1] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088

It is very easy to create such groupings in the Device tree and
display it to the userspace. However, the isa string has to perform
additional string parsing even for that.

> Parsing ISA string is per-hart but one-time only (in kernel space) and
> doesn't take much long.  Yes, the ISA string will get longer and longer
> over time (which might affect the boot performance on many-core systems)
> but once we get heterogeneous systems (as you mentioned), we will have
> per-hart "isa-ext" anyway and we will have pretty much the same situation.
> We could take per-group approach instead of per-hart for efficiency but
> that means ISA string parsing can be optimized just as "isa-ext".
>
> Current RISC-V systems have not long ISA string and not many cores (making
> per-hart ISA string processing not a problem).  Future complex RISC-V
> systems may have many cores, heterogeneous capabilities and long ISA
> strings that need to be taken care.  However, "isa-ext" will have a
> similar scaling issue over time (just as the ISA string).
>

No. The scaling issue in isa-ext will not be as near as the riscv,isa
string as the early boot code will
load a value from the array and check a DT property.

> So, I don't see any merits of "isa-ext" in the long or short term.
>

On the other hand, I don't understand what is the disadvantage of the
isa-ext approach. Even if you don't
agree on the performance and simplicity front, it certainly becomes
more human readable.


> Regards,
>
>
> > Thus, this patch provides a DT based framework to for easy parsing and
> > querying of any ISA extensions. It facilitates custom user visible strings
> > for the ISA extensions via /proc/cpuinfo as well.
> >
> > Currently, there are no platforms with heterogeneous Linux capable harts.
> > That's why, this patch supports only a single DT node which can only work
> > for systems with homogeneous harts. To support heterogeneous systems, this
> > cpu node must be a subnode for each cpu.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/hwcap.h | 31 ++++++++++++++++++
> >  arch/riscv/kernel/cpu.c        | 16 ++++++++++
> >  arch/riscv/kernel/cpufeature.c | 58 +++++++++++++++++++++++++++++++++-
> >  3 files changed, 104 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 5ce50468aff1..368ab0f330c8 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -34,7 +34,36 @@ 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,
> > +};
> > +
> > +struct riscv_isa_ext_data {
> > +     struct list_head node;
> > +     /* Name of the extension property in DT */
> > +     char dtprop[RISCV_ISA_EXT_NAME_LEN_MAX];
> > +     /* 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;
> > +};
> > +
> > +extern struct list_head riscv_isa_ext_list;
> >
> >  unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
> >
> > @@ -44,6 +73,8 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
> >  #define riscv_isa_extension_available(isa_bitmap, ext)       \
> >       __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
> >
> > +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata);
> > +
> >  #endif
> >
> >  #endif /* _ASM_RISCV_HWCAP_H */
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index f13b2c9ea912..dad95bdd3cca 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -71,6 +71,21 @@ static void print_isa(struct seq_file *f, const char *isa)
> >       seq_puts(f, "\n");
> >  }
> >
> > +static void print_isa_ext(struct seq_file *f)
> > +{
> > +     struct riscv_isa_ext_data *edata;
> > +     int count = 0;
> > +
> > +     seq_puts(f, "isa-ext\t\t: ");
> > +     list_for_each_entry(edata, &riscv_isa_ext_list, node) {
> > +             if (count)
> > +                     seq_puts(f, ",");
> > +             seq_write(f, edata->uprop, strnlen(edata->uprop, RISCV_ISA_EXT_NAME_LEN_MAX));
> > +             count++;
> > +     }
> > +     seq_puts(f, "\n");
> > +}
> > +
> >  static void print_mmu(struct seq_file *f, const char *mmu_type)
> >  {
> >  #if defined(CONFIG_32BIT)
> > @@ -113,6 +128,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);
> >       if (!of_property_read_string(node, "mmu-type", &mmu))
> >               print_mmu(m, mmu);
> >       if (!of_property_read_string(node, "compatible", &compat)
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index d959d207a40d..c70eeec17f5b 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -7,6 +7,8 @@
> >   */
> >
> >  #include <linux/bitmap.h>
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> >  #include <linux/of.h>
> >  #include <asm/processor.h>
> >  #include <asm/hwcap.h>
> > @@ -18,6 +20,8 @@ unsigned long elf_hwcap __read_mostly;
> >  /* Host ISA bitmap */
> >  static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> >
> > +LIST_HEAD(riscv_isa_ext_list);
> > +
> >  #ifdef CONFIG_FPU
> >  __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
> >  #endif
> > @@ -59,12 +63,60 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit)
> >  }
> >  EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
> >
> > +#define __RISCV_ISA_EXT_DATA(DTPROP, UPROP, EXTID) \
> > +     {                                                       \
> > +             .dtprop = #DTPROP,                              \
> > +             .uprop = #UPROP,                                \
> > +             .isa_ext_id = EXTID,                            \
> > +     }
> > +
> > +static struct riscv_isa_ext_data isa_ext_arr[] = {
> > +     __RISCV_ISA_EXT_DATA("", "", RISCV_ISA_EXT_MAX),
> > +};
> > +
> > +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
> > +{
> > +     struct device_node *node, *enode;
> > +     int eid;
> > +
> > +     if (!edata || !edata->dtprop)
> > +             return -EINVAL;
> > +
> > +     node = of_find_node_by_path("/cpus");
> > +     if (!node) {
> > +             pr_err("No CPU information found in DT\n");
> > +             return -ENOENT;
> > +     }
> > +
> > +     enode = of_get_child_by_name(node, "riscv,isa-ext");
> > +     if (!enode) {
> > +             pr_err("No riscv-isa-ext found in DT\n");
> > +             return -ENOENT;
> > +     }
> > +
> > +     eid = edata->isa_ext_id;
> > +     if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
> > +             return -EINVAL;
> > +
> > +     if (!of_property_read_bool(enode, edata->dtprop)) {
> > +             pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* Enable the extension id in the riscv_isa for easier probing */
> > +     riscv_isa[0] |= 1 << eid;
> > +     list_add(&edata->node, &riscv_isa_ext_list);
> > +     pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
> > +
> > +     return 0;
> > +}
> > +
> >  void __init riscv_fill_hwcap(void)
> >  {
> >       struct device_node *node;
> >       const char *isa;
> >       char print_str[BITS_PER_LONG + 1];
> > -     size_t i, j, isa_len;
> > +     size_t i, j, isa_len, ext_arr_sz;
> >       static unsigned long isa2hwcap[256] = {0};
> >
> >       isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
> > @@ -148,4 +200,8 @@ void __init riscv_fill_hwcap(void)
> >       if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))
> >               static_branch_enable(&cpu_hwcap_fpu);
> >  #endif
> > +
> > +     ext_arr_sz = ARRAY_SIZE(isa_ext_arr);
> > +     for (i = 0; i < ext_arr_sz - 1; i++)
> > +             riscv_isa_ext_list_add(&isa_ext_arr[i]);
> >  }
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Atish Patra]

On Fri, Dec 24, 2021 at 7:13 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Hello Atish,
>
> On 2021/12/25 6:16, Atish Patra wrote:
> > Recently, there were 15 specifications/40 ISA extensions were ratified.
> > Except hypervisor ('H') extension, all of them are multi-letter extensions.
> > Going forward, there will be more number of multi-letter extensions as
> > well. Parsing all of these extensions from ISA string is not scalable.
>
> Accepting multi-letter extensions and providing framework is very nice.
>
> But would you enlighten me how and why "parsing all of these extensions
> from ISA string is not scalable"? That's the only thing I'm not convinced.
>

I thought we already discussed the merits in this thread.
https://lore.kernel.org/lkml/9fa0492d-b448-045b-ba8f-e3d68b46d673@irq.a4lg.com/

Anyways, here is summary of disadvantages of "riscv,isa" string
parsing that you proposed

1. The isa string in DT will be really really long. It is time
consuming to parse it and becomes
somewhat unreadable after a point.
2. Currently, /proc/cpuinfo dumps the entire riscv,isa string. That
works for base/single letter extensions.
However, dumping the entire string will not work in userspace. Imagine
the 10~20 extensions string in your terminal.
3. If you want to group them into different extensions, you have to
parse them again and create separate extensions
which I am doing in this patch.
4. In future, there will be subnodes in the isa-ext which will only
describe the ISA-extensions corresponding to Sv* or Sh* or Z*
groups. I did not add those in this version because the unpriv
specification doesn't define them clearly and may change in the future
(as suggested by Greg [1].)

[1] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088

It is very easy to create such groupings in the Device tree and
display it to the userspace. However, the isa string has to perform
additional string parsing even for that.

> Parsing ISA string is per-hart but one-time only (in kernel space) and
> doesn't take much long.  Yes, the ISA string will get longer and longer
> over time (which might affect the boot performance on many-core systems)
> but once we get heterogeneous systems (as you mentioned), we will have
> per-hart "isa-ext" anyway and we will have pretty much the same situation.
> We could take per-group approach instead of per-hart for efficiency but
> that means ISA string parsing can be optimized just as "isa-ext".
>
> Current RISC-V systems have not long ISA string and not many cores (making
> per-hart ISA string processing not a problem).  Future complex RISC-V
> systems may have many cores, heterogeneous capabilities and long ISA
> strings that need to be taken care.  However, "isa-ext" will have a
> similar scaling issue over time (just as the ISA string).
>

No. The scaling issue in isa-ext will not be as near as the riscv,isa
string as the early boot code will
load a value from the array and check a DT property.

> So, I don't see any merits of "isa-ext" in the long or short term.
>

On the other hand, I don't understand what is the disadvantage of the
isa-ext approach. Even if you don't
agree on the performance and simplicity front, it certainly becomes
more human readable.


> Regards,
>
>
> > Thus, this patch provides a DT based framework to for easy parsing and
> > querying of any ISA extensions. It facilitates custom user visible strings
> > for the ISA extensions via /proc/cpuinfo as well.
> >
> > Currently, there are no platforms with heterogeneous Linux capable harts.
> > That's why, this patch supports only a single DT node which can only work
> > for systems with homogeneous harts. To support heterogeneous systems, this
> > cpu node must be a subnode for each cpu.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/hwcap.h | 31 ++++++++++++++++++
> >  arch/riscv/kernel/cpu.c        | 16 ++++++++++
> >  arch/riscv/kernel/cpufeature.c | 58 +++++++++++++++++++++++++++++++++-
> >  3 files changed, 104 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 5ce50468aff1..368ab0f330c8 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -34,7 +34,36 @@ 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,
> > +};
> > +
> > +struct riscv_isa_ext_data {
> > +     struct list_head node;
> > +     /* Name of the extension property in DT */
> > +     char dtprop[RISCV_ISA_EXT_NAME_LEN_MAX];
> > +     /* 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;
> > +};
> > +
> > +extern struct list_head riscv_isa_ext_list;
> >
> >  unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
> >
> > @@ -44,6 +73,8 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
> >  #define riscv_isa_extension_available(isa_bitmap, ext)       \
> >       __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
> >
> > +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata);
> > +
> >  #endif
> >
> >  #endif /* _ASM_RISCV_HWCAP_H */
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index f13b2c9ea912..dad95bdd3cca 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -71,6 +71,21 @@ static void print_isa(struct seq_file *f, const char *isa)
> >       seq_puts(f, "\n");
> >  }
> >
> > +static void print_isa_ext(struct seq_file *f)
> > +{
> > +     struct riscv_isa_ext_data *edata;
> > +     int count = 0;
> > +
> > +     seq_puts(f, "isa-ext\t\t: ");
> > +     list_for_each_entry(edata, &riscv_isa_ext_list, node) {
> > +             if (count)
> > +                     seq_puts(f, ",");
> > +             seq_write(f, edata->uprop, strnlen(edata->uprop, RISCV_ISA_EXT_NAME_LEN_MAX));
> > +             count++;
> > +     }
> > +     seq_puts(f, "\n");
> > +}
> > +
> >  static void print_mmu(struct seq_file *f, const char *mmu_type)
> >  {
> >  #if defined(CONFIG_32BIT)
> > @@ -113,6 +128,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);
> >       if (!of_property_read_string(node, "mmu-type", &mmu))
> >               print_mmu(m, mmu);
> >       if (!of_property_read_string(node, "compatible", &compat)
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index d959d207a40d..c70eeec17f5b 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -7,6 +7,8 @@
> >   */
> >
> >  #include <linux/bitmap.h>
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> >  #include <linux/of.h>
> >  #include <asm/processor.h>
> >  #include <asm/hwcap.h>
> > @@ -18,6 +20,8 @@ unsigned long elf_hwcap __read_mostly;
> >  /* Host ISA bitmap */
> >  static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> >
> > +LIST_HEAD(riscv_isa_ext_list);
> > +
> >  #ifdef CONFIG_FPU
> >  __ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
> >  #endif
> > @@ -59,12 +63,60 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit)
> >  }
> >  EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
> >
> > +#define __RISCV_ISA_EXT_DATA(DTPROP, UPROP, EXTID) \
> > +     {                                                       \
> > +             .dtprop = #DTPROP,                              \
> > +             .uprop = #UPROP,                                \
> > +             .isa_ext_id = EXTID,                            \
> > +     }
> > +
> > +static struct riscv_isa_ext_data isa_ext_arr[] = {
> > +     __RISCV_ISA_EXT_DATA("", "", RISCV_ISA_EXT_MAX),
> > +};
> > +
> > +int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
> > +{
> > +     struct device_node *node, *enode;
> > +     int eid;
> > +
> > +     if (!edata || !edata->dtprop)
> > +             return -EINVAL;
> > +
> > +     node = of_find_node_by_path("/cpus");
> > +     if (!node) {
> > +             pr_err("No CPU information found in DT\n");
> > +             return -ENOENT;
> > +     }
> > +
> > +     enode = of_get_child_by_name(node, "riscv,isa-ext");
> > +     if (!enode) {
> > +             pr_err("No riscv-isa-ext found in DT\n");
> > +             return -ENOENT;
> > +     }
> > +
> > +     eid = edata->isa_ext_id;
> > +     if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
> > +             return -EINVAL;
> > +
> > +     if (!of_property_read_bool(enode, edata->dtprop)) {
> > +             pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* Enable the extension id in the riscv_isa for easier probing */
> > +     riscv_isa[0] |= 1 << eid;
> > +     list_add(&edata->node, &riscv_isa_ext_list);
> > +     pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
> > +
> > +     return 0;
> > +}
> > +
> >  void __init riscv_fill_hwcap(void)
> >  {
> >       struct device_node *node;
> >       const char *isa;
> >       char print_str[BITS_PER_LONG + 1];
> > -     size_t i, j, isa_len;
> > +     size_t i, j, isa_len, ext_arr_sz;
> >       static unsigned long isa2hwcap[256] = {0};
> >
> >       isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
> > @@ -148,4 +200,8 @@ void __init riscv_fill_hwcap(void)
> >       if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))
> >               static_branch_enable(&cpu_hwcap_fpu);
> >  #endif
> > +
> > +     ext_arr_sz = ARRAY_SIZE(isa_ext_arr);
> > +     for (i = 0; i < ext_arr_sz - 1; i++)
> > +             riscv_isa_ext_list_add(&isa_ext_arr[i]);
> >  }
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

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

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Krzysztof Kozlowski]

On 24/12/2021 22:16, Atish Patra wrote:
> Recently, there were 15 specifications/40 ISA extensions were ratified.
> Except hypervisor ('H') extension, all of them are multi-letter extensions.
> Going forward, there will be more number of multi-letter extensions as
> well. Parsing all of these extensions from ISA string is not scalable.
> Thus, this patch provides a DT based framework to for easy parsing and
> querying of any ISA extensions. It facilitates custom user visible strings
> for the ISA extensions via /proc/cpuinfo as well.
> 
> Currently, there are no platforms with heterogeneous Linux capable harts.
> That's why, this patch supports only a single DT node which can only work
> for systems with homogeneous harts. To support heterogeneous systems, this
> cpu node must be a subnode for each cpu.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

Your from address does not match SoB. Please use consistent one - they
must match.


Best regards,
Krzysztof

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[Krzysztof Kozlowski]

On 24/12/2021 22:16, Atish Patra wrote:
> Recently, there were 15 specifications/40 ISA extensions were ratified.
> Except hypervisor ('H') extension, all of them are multi-letter extensions.
> Going forward, there will be more number of multi-letter extensions as
> well. Parsing all of these extensions from ISA string is not scalable.
> Thus, this patch provides a DT based framework to for easy parsing and
> querying of any ISA extensions. It facilitates custom user visible strings
> for the ISA extensions via /proc/cpuinfo as well.
> 
> Currently, there are no platforms with heterogeneous Linux capable harts.
> That's why, this patch supports only a single DT node which can only work
> for systems with homogeneous harts. To support heterogeneous systems, this
> cpu node must be a subnode for each cpu.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

Your from address does not match SoB. Please use consistent one - they
must match.


Best regards,
Krzysztof

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

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[kernel test robot]

Hi Atish,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on robh/for-next linus/master v5.16-rc7 next-20211224]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Atish-Patra/Provide-a-fraemework-for-RISC-V-ISA-extensions/20211225-051801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 136057256686de39cc3a07c2e39ef6bc43003ff6
config: riscv-randconfig-r024-20220101 (https://download.01.org/0day-ci/archive/20220101/202201011012.ZeiwqIeg-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c054402170cd8466683a20385befc0523aba3359)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/30cc2f98601a733995aadbd52fbcefbb5c37511d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Atish-Patra/Provide-a-fraemework-for-RISC-V-ISA-extensions/20211225-051801
        git checkout 30cc2f98601a733995aadbd52fbcefbb5c37511d
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/kernel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> arch/riscv/kernel/cpufeature.c:82:24: warning: address of array 'edata->dtprop' will always evaluate to 'true' [-Wpointer-bool-conversion]
           if (!edata || !edata->dtprop)
                         ~~~~~~~~^~~~~~
   1 warning generated.


vim +82 arch/riscv/kernel/cpufeature.c

    76	
    77	int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
    78	{
    79		struct device_node *node, *enode;
    80		int eid;
    81	
  > 82		if (!edata || !edata->dtprop)
    83			return -EINVAL;
    84	
    85		node = of_find_node_by_path("/cpus");
    86		if (!node) {
    87			pr_err("No CPU information found in DT\n");
    88			return -ENOENT;
    89		}
    90	
    91		enode = of_get_child_by_name(node, "riscv,isa-ext");
    92		if (!enode) {
    93			pr_err("No riscv-isa-ext found in DT\n");
    94			return -ENOENT;
    95		}
    96	
    97		eid = edata->isa_ext_id;
    98		if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
    99			return -EINVAL;
   100	
   101		if (!of_property_read_bool(enode, edata->dtprop)) {
   102			pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
   103			return -ENODEV;
   104		}
   105	
   106		/* Enable the extension id in the riscv_isa for easier probing */
   107		riscv_isa[0] |= 1 << eid;
   108		list_add(&edata->node, &riscv_isa_ext_list);
   109		pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
   110	
   111		return 0;
   112	}
   113	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v1 1/2] RISC-V: Provide a framework for parsing multi-letter ISA extensions

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3475 bytes --]

Hi Atish,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on robh/for-next linus/master v5.16-rc7 next-20211224]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Atish-Patra/Provide-a-fraemework-for-RISC-V-ISA-extensions/20211225-051801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 136057256686de39cc3a07c2e39ef6bc43003ff6
config: riscv-randconfig-r024-20220101 (https://download.01.org/0day-ci/archive/20220101/202201011012.ZeiwqIeg-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c054402170cd8466683a20385befc0523aba3359)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/30cc2f98601a733995aadbd52fbcefbb5c37511d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Atish-Patra/Provide-a-fraemework-for-RISC-V-ISA-extensions/20211225-051801
        git checkout 30cc2f98601a733995aadbd52fbcefbb5c37511d
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/kernel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> arch/riscv/kernel/cpufeature.c:82:24: warning: address of array 'edata->dtprop' will always evaluate to 'true' [-Wpointer-bool-conversion]
           if (!edata || !edata->dtprop)
                         ~~~~~~~~^~~~~~
   1 warning generated.


vim +82 arch/riscv/kernel/cpufeature.c

    76	
    77	int riscv_isa_ext_list_add(struct riscv_isa_ext_data *edata)
    78	{
    79		struct device_node *node, *enode;
    80		int eid;
    81	
  > 82		if (!edata || !edata->dtprop)
    83			return -EINVAL;
    84	
    85		node = of_find_node_by_path("/cpus");
    86		if (!node) {
    87			pr_err("No CPU information found in DT\n");
    88			return -ENOENT;
    89		}
    90	
    91		enode = of_get_child_by_name(node, "riscv,isa-ext");
    92		if (!enode) {
    93			pr_err("No riscv-isa-ext found in DT\n");
    94			return -ENOENT;
    95		}
    96	
    97		eid = edata->isa_ext_id;
    98		if (eid < RISCV_ISA_EXT_BASE || eid >= RISCV_ISA_EXT_MAX)
    99			return -EINVAL;
   100	
   101		if (!of_property_read_bool(enode, edata->dtprop)) {
   102			pr_err("The ISA extension %s is not present in DT\n", edata->dtprop);
   103			return -ENODEV;
   104		}
   105	
   106		/* Enable the extension id in the riscv_isa for easier probing */
   107		riscv_isa[0] |= 1 << eid;
   108		list_add(&edata->node, &riscv_isa_ext_list);
   109		pr_info("RISC-V ISA extension '%s' available\n", edata->uprop);
   110	
   111		return 0;
   112	}
   113	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Atish Patra]

RISC-V ISA extensions can be single letter or multi-letter names.
The single letter extensions are mostly base extensions and encoded in
"riscv,isa" DT property. However, parsing the multi-letter extensions
via the isa string is cumbersome and is not scalable.

Add a new DT node for multi-letter extensions.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index aa5fb64d57eb..6c4eecf389a9 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -78,6 +78,15 @@ properties:
       - rv64imac
       - rv64imafdc
 
+  riscv,isa-ext:
+    description:
+      Identifies the specific RISC-V instruction set architecture extensions
+      supported by one or multiple harts. All the multi-letter extensions
+      should be listed here as a boolean property. This subnode can be under
+      /cpus or under individual cpu node. In case of former, it represent
+      the common ISA extensions for all harts. The name of the boolean property
+      must match the actual ISA extension name in all lowercase format.
+    $ref: "/schemas/types.yaml#/definitions/boolean"
   # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
   timebase-frequency: false
 
-- 
2.33.1

[PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Atish Patra]

RISC-V ISA extensions can be single letter or multi-letter names.
The single letter extensions are mostly base extensions and encoded in
"riscv,isa" DT property. However, parsing the multi-letter extensions
via the isa string is cumbersome and is not scalable.

Add a new DT node for multi-letter extensions.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index aa5fb64d57eb..6c4eecf389a9 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -78,6 +78,15 @@ properties:
       - rv64imac
       - rv64imafdc
 
+  riscv,isa-ext:
+    description:
+      Identifies the specific RISC-V instruction set architecture extensions
+      supported by one or multiple harts. All the multi-letter extensions
+      should be listed here as a boolean property. This subnode can be under
+      /cpus or under individual cpu node. In case of former, it represent
+      the common ISA extensions for all harts. The name of the boolean property
+      must match the actual ISA extension name in all lowercase format.
+    $ref: "/schemas/types.yaml#/definitions/boolean"
   # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
   timebase-frequency: false
 
-- 
2.33.1


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

Re: [PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Jessica Clarke]

On 24 Dec 2021, at 21:16, Atish Patra <atishp@atishpatra.org> wrote:
> 
> RISC-V ISA extensions can be single letter or multi-letter names.
> The single letter extensions are mostly base extensions and encoded in
> "riscv,isa" DT property. However, parsing the multi-letter extensions
> via the isa string is cumbersome and is not scalable.
> 
> Add a new DT node for multi-letter extensions.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
> Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index aa5fb64d57eb..6c4eecf389a9 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -78,6 +78,15 @@ properties:
>       - rv64imac
>       - rv64imafdc
> 
> +  riscv,isa-ext:
> +    description:
> +      Identifies the specific RISC-V instruction set architecture extensions
> +      supported by one or multiple harts. All the multi-letter extensions
> +      should be listed here as a boolean property. This subnode can be under
> +      /cpus or under individual cpu node. In case of former, it represent
> +      the common ISA extensions for all harts. The name of the boolean property
> +      must match the actual ISA extension name in all lowercase format.

I don’t see why this needs explicitly calling out, that’s true in
general of cpu node properties (3.8p4 of the Devicetree spec v0.4-rc1 /
v0.3-40-g7e1cc17), not that I like it.

Jess

Re: [PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Jessica Clarke]

On 24 Dec 2021, at 21:16, Atish Patra <atishp@atishpatra.org> wrote:
> 
> RISC-V ISA extensions can be single letter or multi-letter names.
> The single letter extensions are mostly base extensions and encoded in
> "riscv,isa" DT property. However, parsing the multi-letter extensions
> via the isa string is cumbersome and is not scalable.
> 
> Add a new DT node for multi-letter extensions.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
> Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index aa5fb64d57eb..6c4eecf389a9 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -78,6 +78,15 @@ properties:
>       - rv64imac
>       - rv64imafdc
> 
> +  riscv,isa-ext:
> +    description:
> +      Identifies the specific RISC-V instruction set architecture extensions
> +      supported by one or multiple harts. All the multi-letter extensions
> +      should be listed here as a boolean property. This subnode can be under
> +      /cpus or under individual cpu node. In case of former, it represent
> +      the common ISA extensions for all harts. The name of the boolean property
> +      must match the actual ISA extension name in all lowercase format.

I don’t see why this needs explicitly calling out, that’s true in
general of cpu node properties (3.8p4 of the Devicetree spec v0.4-rc1 /
v0.3-40-g7e1cc17), not that I like it.

Jess


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

Re: [PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Atish Patra]

On Fri, Dec 24, 2021 at 3:25 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 24 Dec 2021, at 21:16, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > RISC-V ISA extensions can be single letter or multi-letter names.
> > The single letter extensions are mostly base extensions and encoded in
> > "riscv,isa" DT property. However, parsing the multi-letter extensions
> > via the isa string is cumbersome and is not scalable.
> >
> > Add a new DT node for multi-letter extensions.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> > Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index aa5fb64d57eb..6c4eecf389a9 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -78,6 +78,15 @@ properties:
> >       - rv64imac
> >       - rv64imafdc
> >
> > +  riscv,isa-ext:
> > +    description:
> > +      Identifies the specific RISC-V instruction set architecture extensions
> > +      supported by one or multiple harts. All the multi-letter extensions
> > +      should be listed here as a boolean property. This subnode can be under
> > +      /cpus or under individual cpu node. In case of former, it represent
> > +      the common ISA extensions for all harts. The name of the boolean property
> > +      must match the actual ISA extension name in all lowercase format.
>
> I don’t see why this needs explicitly calling out, that’s true in
> general of cpu node properties (3.8p4 of the Devicetree spec v0.4-rc1 /
> v0.3-40-g7e1cc17), not that I like it.
>

Thanks for pointing it out. I did not realize the DT specification
already has a paragraph
about this. I will update the description in the next version.

> Jess
>


-- 
Regards,
Atish

Re: [PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Atish Patra]

On Fri, Dec 24, 2021 at 3:25 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 24 Dec 2021, at 21:16, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > RISC-V ISA extensions can be single letter or multi-letter names.
> > The single letter extensions are mostly base extensions and encoded in
> > "riscv,isa" DT property. However, parsing the multi-letter extensions
> > via the isa string is cumbersome and is not scalable.
> >
> > Add a new DT node for multi-letter extensions.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> > Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index aa5fb64d57eb..6c4eecf389a9 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -78,6 +78,15 @@ properties:
> >       - rv64imac
> >       - rv64imafdc
> >
> > +  riscv,isa-ext:
> > +    description:
> > +      Identifies the specific RISC-V instruction set architecture extensions
> > +      supported by one or multiple harts. All the multi-letter extensions
> > +      should be listed here as a boolean property. This subnode can be under
> > +      /cpus or under individual cpu node. In case of former, it represent
> > +      the common ISA extensions for all harts. The name of the boolean property
> > +      must match the actual ISA extension name in all lowercase format.
>
> I don’t see why this needs explicitly calling out, that’s true in
> general of cpu node properties (3.8p4 of the Devicetree spec v0.4-rc1 /
> v0.3-40-g7e1cc17), not that I like it.
>

Thanks for pointing it out. I did not realize the DT specification
already has a paragraph
about this. I will update the description in the next version.

> Jess
>


-- 
Regards,
Atish

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

Re: [PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Rob Herring]

On Fri, 24 Dec 2021 13:16:32 -0800, Atish Patra wrote:
> RISC-V ISA extensions can be single letter or multi-letter names.
> The single letter extensions are mostly base extensions and encoded in
> "riscv,isa" DT property. However, parsing the multi-letter extensions
> via the isa string is cumbersome and is not scalable.
> 
> Add a new DT node for multi-letter extensions.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/riscv/cpus.yaml: Unresolvable JSON pointer: 'definitions/boolean'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1573119

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v1 2/2] dt-bindings: riscv: Add DT binding for RISC-V ISA extensions

[Rob Herring]

On Fri, 24 Dec 2021 13:16:32 -0800, Atish Patra wrote:
> RISC-V ISA extensions can be single letter or multi-letter names.
> The single letter extensions are mostly base extensions and encoded in
> "riscv,isa" DT property. However, parsing the multi-letter extensions
> via the isa string is cumbersome and is not scalable.
> 
> Add a new DT node for multi-letter extensions.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/riscv/cpus.yaml: Unresolvable JSON pointer: 'definitions/boolean'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1573119

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

Re: [PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Palmer Dabbelt]

On Fri, 24 Dec 2021 13:16:30 PST (-0800), atishp@atishpatra.org wrote:
> This series implements a generic framework to parse multi-letter ISA
> extensions. It introduces a new DT node that can be under /cpus or
> individual cpu depends on the platforms with homogeneous or heterogeneous
> harts. This version of the series only allows adds support for homogeneous
> harts as there are no platforms with heterogeneous harts yet. However,
> the DT binding allows both.
>
> The patch also indicates the user space about 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

IMO this is the wrong way to go.  I get that the ISA string is very 
complicated to parse, but we've tried to come up with other 
representations of this we've ended up having that interface break when 
the ISA string rules eventually change.  We should just stick to the ISA 
string for these interfaces, as that's at least something everyone can 
agree on because they're defined by the spec.

That said, we should add the spec versions into this interface.  At 
least the user spec version is relevant here, but given that we're 
passing through some other priv-level details we might as well pass that 
though too.

> 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[1] adds the support for sscofpmf extension.
>
> [1] https://github.com/atishp04/linux/commit/a23157264118d6fd905fd08d8717c7df03078bb1
>
> Atish Patra (2):
> RISC-V: Provide a framework for parsing multi-letter ISA extensions
> dt-bindings: riscv: Add DT binding for RISC-V ISA extensions
>
> .../devicetree/bindings/riscv/cpus.yaml       |  9 +++
> arch/riscv/include/asm/hwcap.h                | 31 ++++++++++
> arch/riscv/kernel/cpu.c                       | 16 +++++
> arch/riscv/kernel/cpufeature.c                | 58 ++++++++++++++++++-
> 4 files changed, 113 insertions(+), 1 deletion(-)

Re: [PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Palmer Dabbelt]

On Fri, 24 Dec 2021 13:16:30 PST (-0800), atishp@atishpatra.org wrote:
> This series implements a generic framework to parse multi-letter ISA
> extensions. It introduces a new DT node that can be under /cpus or
> individual cpu depends on the platforms with homogeneous or heterogeneous
> harts. This version of the series only allows adds support for homogeneous
> harts as there are no platforms with heterogeneous harts yet. However,
> the DT binding allows both.
>
> The patch also indicates the user space about 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

IMO this is the wrong way to go.  I get that the ISA string is very 
complicated to parse, but we've tried to come up with other 
representations of this we've ended up having that interface break when 
the ISA string rules eventually change.  We should just stick to the ISA 
string for these interfaces, as that's at least something everyone can 
agree on because they're defined by the spec.

That said, we should add the spec versions into this interface.  At 
least the user spec version is relevant here, but given that we're 
passing through some other priv-level details we might as well pass that 
though too.

> 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[1] adds the support for sscofpmf extension.
>
> [1] https://github.com/atishp04/linux/commit/a23157264118d6fd905fd08d8717c7df03078bb1
>
> Atish Patra (2):
> RISC-V: Provide a framework for parsing multi-letter ISA extensions
> dt-bindings: riscv: Add DT binding for RISC-V ISA extensions
>
> .../devicetree/bindings/riscv/cpus.yaml       |  9 +++
> arch/riscv/include/asm/hwcap.h                | 31 ++++++++++
> arch/riscv/kernel/cpu.c                       | 16 +++++
> arch/riscv/kernel/cpufeature.c                | 58 ++++++++++++++++++-
> 4 files changed, 113 insertions(+), 1 deletion(-)

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

Re: [PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Atish Patra]

On Fri, Jan 7, 2022 at 1:58 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Fri, 24 Dec 2021 13:16:30 PST (-0800), atishp@atishpatra.org wrote:
> > This series implements a generic framework to parse multi-letter ISA
> > extensions. It introduces a new DT node that can be under /cpus or
> > individual cpu depends on the platforms with homogeneous or heterogeneous
> > harts. This version of the series only allows adds support for homogeneous
> > harts as there are no platforms with heterogeneous harts yet. However,
> > the DT binding allows both.
> >
> > The patch also indicates the user space about 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
>
> IMO this is the wrong way to go.  I get that the ISA string is very
> complicated to parse, but we've tried to come up with other
> representations of this we've ended up having that interface break when
> the ISA string rules eventually change.  We should just stick to the ISA
> string for these interfaces, as that's at least something everyone can
> agree on because they're defined by the spec.
>

Fair enough.

> That said, we should add the spec versions into this interface.  At
> least the user spec version is relevant here, but given that we're
> passing through some other priv-level details we might as well pass that
> though too.
>

Tsukasa already has a version that extends the isa string parsing for
multi-letter extensions
and versions parsing. We just need to add the ISA bitmap support on top of it.

I will coordinate with Tsukasa to have a complete framework based on
string parsing.
It would be good to have this series asap as all other series  (perf,
svpbmt) will rely on it.

> > 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[1] adds the support for sscofpmf extension.
> >
> > [1] https://github.com/atishp04/linux/commit/a23157264118d6fd905fd08d8717c7df03078bb1
> >
> > Atish Patra (2):
> > RISC-V: Provide a framework for parsing multi-letter ISA extensions
> > dt-bindings: riscv: Add DT binding for RISC-V ISA extensions
> >
> > .../devicetree/bindings/riscv/cpus.yaml       |  9 +++
> > arch/riscv/include/asm/hwcap.h                | 31 ++++++++++
> > arch/riscv/kernel/cpu.c                       | 16 +++++
> > arch/riscv/kernel/cpufeature.c                | 58 ++++++++++++++++++-
> > 4 files changed, 113 insertions(+), 1 deletion(-)



--
Regards,
Atish

Re: [PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Atish Patra]

On Fri, Jan 7, 2022 at 1:58 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Fri, 24 Dec 2021 13:16:30 PST (-0800), atishp@atishpatra.org wrote:
> > This series implements a generic framework to parse multi-letter ISA
> > extensions. It introduces a new DT node that can be under /cpus or
> > individual cpu depends on the platforms with homogeneous or heterogeneous
> > harts. This version of the series only allows adds support for homogeneous
> > harts as there are no platforms with heterogeneous harts yet. However,
> > the DT binding allows both.
> >
> > The patch also indicates the user space about 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
>
> IMO this is the wrong way to go.  I get that the ISA string is very
> complicated to parse, but we've tried to come up with other
> representations of this we've ended up having that interface break when
> the ISA string rules eventually change.  We should just stick to the ISA
> string for these interfaces, as that's at least something everyone can
> agree on because they're defined by the spec.
>

Fair enough.

> That said, we should add the spec versions into this interface.  At
> least the user spec version is relevant here, but given that we're
> passing through some other priv-level details we might as well pass that
> though too.
>

Tsukasa already has a version that extends the isa string parsing for
multi-letter extensions
and versions parsing. We just need to add the ISA bitmap support on top of it.

I will coordinate with Tsukasa to have a complete framework based on
string parsing.
It would be good to have this series asap as all other series  (perf,
svpbmt) will rely on it.

> > 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[1] adds the support for sscofpmf extension.
> >
> > [1] https://github.com/atishp04/linux/commit/a23157264118d6fd905fd08d8717c7df03078bb1
> >
> > Atish Patra (2):
> > RISC-V: Provide a framework for parsing multi-letter ISA extensions
> > dt-bindings: riscv: Add DT binding for RISC-V ISA extensions
> >
> > .../devicetree/bindings/riscv/cpus.yaml       |  9 +++
> > arch/riscv/include/asm/hwcap.h                | 31 ++++++++++
> > arch/riscv/kernel/cpu.c                       | 16 +++++
> > arch/riscv/kernel/cpufeature.c                | 58 ++++++++++++++++++-
> > 4 files changed, 113 insertions(+), 1 deletion(-)



--
Regards,
Atish

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

Re: [PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Heiko Stübner]

Hi Atish,

Am Samstag, 8. Januar 2022, 03:24:12 CET schrieb Atish Patra:
> On Fri, Jan 7, 2022 at 1:58 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Fri, 24 Dec 2021 13:16:30 PST (-0800), atishp@atishpatra.org wrote:
> > > This series implements a generic framework to parse multi-letter ISA
> > > extensions. It introduces a new DT node that can be under /cpus or
> > > individual cpu depends on the platforms with homogeneous or heterogeneous
> > > harts. This version of the series only allows adds support for homogeneous
> > > harts as there are no platforms with heterogeneous harts yet. However,
> > > the DT binding allows both.
> > >
> > > The patch also indicates the user space about 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
> >
> > IMO this is the wrong way to go.  I get that the ISA string is very
> > complicated to parse, but we've tried to come up with other
> > representations of this we've ended up having that interface break when
> > the ISA string rules eventually change.  We should just stick to the ISA
> > string for these interfaces, as that's at least something everyone can
> > agree on because they're defined by the spec.
> >
> 
> Fair enough.
> 
> > That said, we should add the spec versions into this interface.  At
> > least the user spec version is relevant here, but given that we're
> > passing through some other priv-level details we might as well pass that
> > though too.
> >
> 
> Tsukasa already has a version that extends the isa string parsing for
> multi-letter extensions
> and versions parsing. We just need to add the ISA bitmap support on top of it.
> 
> I will coordinate with Tsukasa to have a complete framework based on
> string parsing.
> It would be good to have this series asap as all other series  (perf,
> svpbmt) will rely on it.

out of curiosity, did this go anywhere yet in terms of the coordinated
approach you wrote about?

Thanks
Heiko

Re: [PATCH v1 0/2] Provide a fraemework for RISC-V ISA extensions

[Heiko Stübner]

Hi Atish,

Am Samstag, 8. Januar 2022, 03:24:12 CET schrieb Atish Patra:
> On Fri, Jan 7, 2022 at 1:58 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Fri, 24 Dec 2021 13:16:30 PST (-0800), atishp@atishpatra.org wrote:
> > > This series implements a generic framework to parse multi-letter ISA
> > > extensions. It introduces a new DT node that can be under /cpus or
> > > individual cpu depends on the platforms with homogeneous or heterogeneous
> > > harts. This version of the series only allows adds support for homogeneous
> > > harts as there are no platforms with heterogeneous harts yet. However,
> > > the DT binding allows both.
> > >
> > > The patch also indicates the user space about 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
> >
> > IMO this is the wrong way to go.  I get that the ISA string is very
> > complicated to parse, but we've tried to come up with other
> > representations of this we've ended up having that interface break when
> > the ISA string rules eventually change.  We should just stick to the ISA
> > string for these interfaces, as that's at least something everyone can
> > agree on because they're defined by the spec.
> >
> 
> Fair enough.
> 
> > That said, we should add the spec versions into this interface.  At
> > least the user spec version is relevant here, but given that we're
> > passing through some other priv-level details we might as well pass that
> > though too.
> >
> 
> Tsukasa already has a version that extends the isa string parsing for
> multi-letter extensions
> and versions parsing. We just need to add the ISA bitmap support on top of it.
> 
> I will coordinate with Tsukasa to have a complete framework based on
> string parsing.
> It would be good to have this series asap as all other series  (perf,
> svpbmt) will rely on it.

out of curiosity, did this go anywhere yet in terms of the coordinated
approach you wrote about?

Thanks
Heiko



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