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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On 24 Dec 2021, at 21:16, Atish Patra <[email protected]> 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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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]);
> }
On Fri, Dec 24, 2021 at 3:25 PM Jessica Clarke <[email protected]> wrote:
>
> On 24 Dec 2021, at 21:16, Atish Patra <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Fri, Dec 24, 2021 at 7:13 PM Tsukasa OI <[email protected]> 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/[email protected]/
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 <[email protected]>
> > ---
> > 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Regards,
Atish
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 <[email protected]>
Your from address does not match SoB. Please use consistent one - they
must match.
Best regards,
Krzysztof
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 <[email protected]>
> ---
> 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.
On Fri, 24 Dec 2021 13:16:30 PST (-0800), [email protected] 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(-)
On Fri, Jan 7, 2022 at 1:58 PM Palmer Dabbelt <[email protected]> wrote:
>
> On Fri, 24 Dec 2021 13:16:30 PST (-0800), [email protected] 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
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 <[email protected]> wrote:
> >
> > On Fri, 24 Dec 2021 13:16:30 PST (-0800), [email protected] 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