[PATCH 0/3] RISC-V: Add Bitmanip/Scalar Crypto HWCAP

[PATCH 0/3] RISC-V: Add Bitmanip/Scalar Crypto HWCAP

[Hongren (Zenithal) Zheng]

This patchset proposes a currently viable and forward
compatible way to expose the bitmanip/scalar crypto
capability of the platform to the userspace.

Currently viable refers to the property that hardware
platforms can easily modify the riscv,isa field in DT to
tell the kernel it has the capability. Note that QEMU
has already done so in its device tree.

Forward compatible refers to the property that userspace
can still detect the capability of the environment by
using HWCAP regardless of how the mechanism changes
below kernel in the future. I do know that it has not
been settled how to discover a capability, but I think
kernel has to offer some API after all, and HWCAP
is the preferred way among other mechanisms for now.

More discussion on userspace discovering
can be found on my PR to openssl
https://github.com/openssl/openssl/pull/18197

Hongren (Zenithal) Zheng (3):
  RISC-V: add Bitmanip/Scalar Crypto parsing from DT
  RISC-V: uapi: add HWCAP for Bitmanip/Scalar Crypto
  RISC-V: HWCAP: parse Bitmanip/Scalar Crypto HWCAP from DT

 arch/riscv/include/asm/elf.h        |  2 +
 arch/riscv/include/asm/hwcap.h      | 16 ++++++
 arch/riscv/include/uapi/asm/hwcap.h | 22 ++++++++
 arch/riscv/kernel/cpu.c             | 14 +++++
 arch/riscv/kernel/cpufeature.c      | 79 +++++++++++++++++++++++++----
 5 files changed, 123 insertions(+), 10 deletions(-)

-- 
2.35.1

[PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Hongren (Zenithal) Zheng]

This commit parses Zb/Zk related string from DT and
output them in cpuinfo

One thing worth noting is that if DT provides zk,
all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.

Note that zk is a valid extension name and the current
DT binding spec allows this.

There currently lacks a mechanism to merge them when
producing cpuinfo. Namely if you provide a riscv,isa
"rv64imafdc_zk_zks", the cpuinfo output would be
"rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
_zksh_zkt"

Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 arch/riscv/include/asm/hwcap.h | 14 ++++++++++++++
 arch/riscv/kernel/cpu.c        | 14 ++++++++++++++
 arch/riscv/kernel/cpufeature.c | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 0734e42f74f2..199eda39e0b8 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -52,6 +52,20 @@ extern unsigned long elf_hwcap;
  */
 enum riscv_isa_ext_id {
 	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
+	RISCV_ISA_EXT_ZBA,
+	RISCV_ISA_EXT_ZBB,
+	RISCV_ISA_EXT_ZBC,
+	RISCV_ISA_EXT_ZBS,
+	RISCV_ISA_EXT_ZBKB,
+	RISCV_ISA_EXT_ZBKC,
+	RISCV_ISA_EXT_ZBKX,
+	RISCV_ISA_EXT_ZKNE,
+	RISCV_ISA_EXT_ZKND,
+	RISCV_ISA_EXT_ZKNH,
+	RISCV_ISA_EXT_ZKSED,
+	RISCV_ISA_EXT_ZKSH,
+	RISCV_ISA_EXT_ZKR,
+	RISCV_ISA_EXT_ZKT,
 	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
 };
 
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ccb617791e56..7251336969c1 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -87,6 +87,20 @@ int riscv_of_parent_hartid(struct device_node *node)
  *    extensions by an underscore.
  */
 static struct riscv_isa_ext_data isa_ext_arr[] = {
+	__RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
+	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
+	__RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
+	__RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
+	__RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
+	__RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
+	__RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
+	__RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
+	__RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
+	__RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
+	__RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
+	__RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
+	__RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
+	__RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
 	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
 };
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1b2d42d7f589..10f9daf3734e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -192,6 +192,39 @@ void __init riscv_fill_hwcap(void)
 				set_bit(*ext - 'a', this_isa);
 			} else {
 				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
+				SET_ISA_EXT_MAP("zba"     , RISCV_ISA_EXT_ZBA     );
+				SET_ISA_EXT_MAP("zbb"     , RISCV_ISA_EXT_ZBB     );
+				SET_ISA_EXT_MAP("zbc"     , RISCV_ISA_EXT_ZBC     );
+				SET_ISA_EXT_MAP("zbs"     , RISCV_ISA_EXT_ZBS     );
+				SET_ISA_EXT_MAP("zbkb"    , RISCV_ISA_EXT_ZBKB    );
+				SET_ISA_EXT_MAP("zbkc"    , RISCV_ISA_EXT_ZBKC    );
+				SET_ISA_EXT_MAP("zbks"    , RISCV_ISA_EXT_ZBKX    );
+				SET_ISA_EXT_MAP("zknd"    , RISCV_ISA_EXT_ZKND    );
+				SET_ISA_EXT_MAP("zkne"    , RISCV_ISA_EXT_ZKNE    );
+				SET_ISA_EXT_MAP("zknh"    , RISCV_ISA_EXT_ZKNH    );
+				SET_ISA_EXT_MAP("zksed"   , RISCV_ISA_EXT_ZKSED   );
+				SET_ISA_EXT_MAP("zksh"    , RISCV_ISA_EXT_ZKSH    );
+				SET_ISA_EXT_MAP("zkr"     , RISCV_ISA_EXT_ZKR     );
+				SET_ISA_EXT_MAP("zkt"     , RISCV_ISA_EXT_ZKT     );
+				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKB    );
+				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKC    );
+				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKX    );
+				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKND    );
+				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKNE    );
+				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKNH    );
+				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKB    );
+				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKC    );
+				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKX    );
+				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZKSED   );
+				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZKSH    );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKB    );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKC    );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKX    );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKND    );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKNE    );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKNH    );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKR     );
+				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKT     );
 			}
 #undef SET_ISA_EXT_MAP
 		}
-- 
2.35.1

[PATCH 2/3] RISC-V: uapi: add HWCAP for Bitmanip/Scalar Crypto

[Hongren (Zenithal) Zheng]

userspace currently lacks a way to detect whether the
platform has Bitmanip/Scalar Crypto capability,
this commit provides a way such that the userspace
can detect it.

RISC-V currently still has no mature mechanism,
but no matter how things in the spec changes,
(no matter how "M" mode things change), the kernel
still needs to offer some API to the userspace.

More discussion can be found at
https://github.com/openssl/openssl/pull/18197
Userspace currently has to use env var to detect them.

This commit along does not assume any specific mechanism
below kernel.

Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 arch/riscv/include/uapi/asm/hwcap.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/riscv/include/uapi/asm/hwcap.h b/arch/riscv/include/uapi/asm/hwcap.h
index 46dc3f5ee99f..bfed3e5c338c 100644
--- a/arch/riscv/include/uapi/asm/hwcap.h
+++ b/arch/riscv/include/uapi/asm/hwcap.h
@@ -22,4 +22,26 @@
 #define COMPAT_HWCAP_ISA_D	(1 << ('D' - 'A'))
 #define COMPAT_HWCAP_ISA_C	(1 << ('C' - 'A'))
 
+/*
+ * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
+ *
+ * As only 32 bits of elf_hwcap (in kernel) could be used
+ * and RISC-V has reserved 26 bits of it, other caps like
+ * bitmanip and crypto can not be placed in AT_HWCAP
+ */
+#define COMPAT_HWCAP2_ISA_ZBA   (1 <<  0)
+#define COMPAT_HWCAP2_ISA_ZBB   (1 <<  1)
+#define COMPAT_HWCAP2_ISA_ZBC   (1 <<  2)
+#define COMPAT_HWCAP2_ISA_ZBS   (1 <<  3)
+#define COMPAT_HWCAP2_ISA_ZBKB  (1 <<  4)
+#define COMPAT_HWCAP2_ISA_ZBKC  (1 <<  5)
+#define COMPAT_HWCAP2_ISA_ZBKX  (1 <<  6)
+#define COMPAT_HWCAP2_ISA_ZKND  (1 <<  7)
+#define COMPAT_HWCAP2_ISA_ZKNE  (1 <<  8)
+#define COMPAT_HWCAP2_ISA_ZKNH  (1 <<  9)
+#define COMPAT_HWCAP2_ISA_ZKSED (1 << 10)
+#define COMPAT_HWCAP2_ISA_ZKSH  (1 << 11)
+#define COMPAT_HWCAP2_ISA_ZKR   (1 << 12)
+#define COMPAT_HWCAP2_ISA_ZKT   (1 << 13)
+
 #endif /* _UAPI_ASM_RISCV_HWCAP_H */
-- 
2.35.1

[PATCH 3/3] RISC-V: HWCAP: parse Bitmanip/Scalar Crypto HWCAP from DT

[Hongren (Zenithal) Zheng]

One viable way to detect Zb/Zk HWCAP is from the DT binding.
No matter how the "M" mode things change, this way can
always be an auxiliary way to detect it.

Note that QEMU currently has "zba/zbb/zbc/zbs" in their device tree
riscv,isa

This also fixes the isa2hwcap way as using unsigned char
for long extension is not viable. Note that the tolower function
ensures functionality. For other no-hwcap extension (e.g. h, s, u),
or ("|") with 0 has no effect on hwcap.

Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 arch/riscv/include/asm/elf.h   |  2 ++
 arch/riscv/include/asm/hwcap.h |  2 ++
 arch/riscv/kernel/cpufeature.c | 46 ++++++++++++++++++++++++++--------
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index f53c40026c7a..c6a4d8d2a241 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -51,7 +51,9 @@
  * but it's not easy, and we've already done it here.
  */
 #define ELF_HWCAP	(elf_hwcap)
+#define ELF_HWCAP2	(elf_hwcap2)
 extern unsigned long elf_hwcap;
+extern unsigned long elf_hwcap2;
 
 /*
  * This yields a string that ld.so will use to load implementation
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 199eda39e0b8..357b0481f1d0 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -17,12 +17,14 @@
  * instruction set this cpu supports.
  */
 #define ELF_HWCAP		(elf_hwcap)
+#define ELF_HWCAP2		(elf_hwcap2)
 
 enum {
 	CAP_HWCAP = 1,
 };
 
 extern unsigned long elf_hwcap;
+extern unsigned long elf_hwcap2;
 
 #define RISCV_ISA_EXT_a		('a' - 'a')
 #define RISCV_ISA_EXT_c		('c' - 'a')
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 10f9daf3734e..f3a033bb51f5 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -17,6 +17,7 @@
 #define NUM_ALPHA_EXTS ('z' - 'a' + 1)
 
 unsigned long elf_hwcap __read_mostly;
+unsigned long elf_hwcap2 __read_mostly;
 
 /* Host ISA bitmap */
 static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
@@ -68,21 +69,39 @@ void __init riscv_fill_hwcap(void)
 	const char *isa;
 	char print_str[NUM_ALPHA_EXTS + 1];
 	int i, j;
-	static unsigned long isa2hwcap[256] = {0};
+	static unsigned long isa2hwcap[RISCV_ISA_EXT_MAX] = {0};
 
-	isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
-	isa2hwcap['m'] = isa2hwcap['M'] = COMPAT_HWCAP_ISA_M;
-	isa2hwcap['a'] = isa2hwcap['A'] = COMPAT_HWCAP_ISA_A;
-	isa2hwcap['f'] = isa2hwcap['F'] = COMPAT_HWCAP_ISA_F;
-	isa2hwcap['d'] = isa2hwcap['D'] = COMPAT_HWCAP_ISA_D;
-	isa2hwcap['c'] = isa2hwcap['C'] = COMPAT_HWCAP_ISA_C;
+	/* HWCAP */
+	isa2hwcap[RISCV_ISA_EXT_i] = COMPAT_HWCAP_ISA_I;
+	isa2hwcap[RISCV_ISA_EXT_m] = COMPAT_HWCAP_ISA_M;
+	isa2hwcap[RISCV_ISA_EXT_a] = COMPAT_HWCAP_ISA_A;
+	isa2hwcap[RISCV_ISA_EXT_f] = COMPAT_HWCAP_ISA_F;
+	isa2hwcap[RISCV_ISA_EXT_d] = COMPAT_HWCAP_ISA_D;
+	isa2hwcap[RISCV_ISA_EXT_c] = COMPAT_HWCAP_ISA_C;
+	/* HWCAP2 */
+	isa2hwcap[RISCV_ISA_EXT_ZBA  ] = COMPAT_HWCAP2_ISA_ZBA;
+	isa2hwcap[RISCV_ISA_EXT_ZBB  ] = COMPAT_HWCAP2_ISA_ZBB;
+	isa2hwcap[RISCV_ISA_EXT_ZBC  ] = COMPAT_HWCAP2_ISA_ZBC;
+	isa2hwcap[RISCV_ISA_EXT_ZBS  ] = COMPAT_HWCAP2_ISA_ZBS;
+	isa2hwcap[RISCV_ISA_EXT_ZBKB ] = COMPAT_HWCAP2_ISA_ZBKB;
+	isa2hwcap[RISCV_ISA_EXT_ZBKC ] = COMPAT_HWCAP2_ISA_ZBKC;
+	isa2hwcap[RISCV_ISA_EXT_ZBKX ] = COMPAT_HWCAP2_ISA_ZBKX;
+	isa2hwcap[RISCV_ISA_EXT_ZKNE ] = COMPAT_HWCAP2_ISA_ZKND;
+	isa2hwcap[RISCV_ISA_EXT_ZKND ] = COMPAT_HWCAP2_ISA_ZKNE;
+	isa2hwcap[RISCV_ISA_EXT_ZKNH ] = COMPAT_HWCAP2_ISA_ZKNH;
+	isa2hwcap[RISCV_ISA_EXT_ZKSED] = COMPAT_HWCAP2_ISA_ZKSED;
+	isa2hwcap[RISCV_ISA_EXT_ZKSH ] = COMPAT_HWCAP2_ISA_ZKSH;
+	isa2hwcap[RISCV_ISA_EXT_ZKR  ] = COMPAT_HWCAP2_ISA_ZKR;
+	isa2hwcap[RISCV_ISA_EXT_ZKT  ] = COMPAT_HWCAP2_ISA_ZKT;
 
 	elf_hwcap = 0;
+	elf_hwcap2 = 0;
 
 	bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
 
 	for_each_of_cpu_node(node) {
 		unsigned long this_hwcap = 0;
+		unsigned long this_hwcap2 = 0;
 		DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
 		const char *temp;
 
@@ -181,15 +200,17 @@ void __init riscv_fill_hwcap(void)
 #define SET_ISA_EXT_MAP(name, bit)						\
 			do {							\
 				if ((ext_end - ext == sizeof(name) - 1) &&	\
-				     !memcmp(ext, name, sizeof(name) - 1))	\
+				     !memcmp(ext, name, sizeof(name) - 1)) {	\
+					this_hwcap2 |= isa2hwcap[bit];		\
 					set_bit(bit, this_isa);			\
+				}						\
 			} while (false)						\
 
 			if (unlikely(ext_err))
 				continue;
 			if (!ext_long) {
-				this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
-				set_bit(*ext - 'a', this_isa);
+				this_hwcap |= isa2hwcap[tolower(*ext) - 'a'];
+				set_bit(tolower(*ext) - 'a', this_isa);
 			} else {
 				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
 				SET_ISA_EXT_MAP("zba"     , RISCV_ISA_EXT_ZBA     );
@@ -239,6 +260,11 @@ void __init riscv_fill_hwcap(void)
 		else
 			elf_hwcap = this_hwcap;
 
+		if (elf_hwcap2)
+			elf_hwcap2 &= this_hwcap2;
+		else
+			elf_hwcap2 = this_hwcap2;
+
 		if (bitmap_weight(riscv_isa, RISCV_ISA_EXT_MAX))
 			bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
 		else
-- 
2.35.1

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Heiko Stuebner]

Am Samstag, 30. April 2022, 15:50:22 CEST schrieb Hongren (Zenithal) Zheng:
> This commit parses Zb/Zk related string from DT and
> output them in cpuinfo
> 
> One thing worth noting is that if DT provides zk,
> all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> 
> Note that zk is a valid extension name and the current
> DT binding spec allows this.
> 
> There currently lacks a mechanism to merge them when
> producing cpuinfo. Namely if you provide a riscv,isa
> "rv64imafdc_zk_zks", the cpuinfo output would be
> "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> _zksh_zkt"
> 
> Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> ---
>  arch/riscv/include/asm/hwcap.h | 14 ++++++++++++++
>  arch/riscv/kernel/cpu.c        | 14 ++++++++++++++
>  arch/riscv/kernel/cpufeature.c | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 0734e42f74f2..199eda39e0b8 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -52,6 +52,20 @@ extern unsigned long elf_hwcap;
>   */
>  enum riscv_isa_ext_id {
>  	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> +	RISCV_ISA_EXT_ZBA,
> +	RISCV_ISA_EXT_ZBB,
> +	RISCV_ISA_EXT_ZBC,
> +	RISCV_ISA_EXT_ZBS,
> +	RISCV_ISA_EXT_ZBKB,
> +	RISCV_ISA_EXT_ZBKC,
> +	RISCV_ISA_EXT_ZBKX,
> +	RISCV_ISA_EXT_ZKNE,
> +	RISCV_ISA_EXT_ZKND,
> +	RISCV_ISA_EXT_ZKNH,
> +	RISCV_ISA_EXT_ZKSED,
> +	RISCV_ISA_EXT_ZKSH,
> +	RISCV_ISA_EXT_ZKR,
> +	RISCV_ISA_EXT_ZKT,
>  	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
>  };
>  
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index ccb617791e56..7251336969c1 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -87,6 +87,20 @@ int riscv_of_parent_hartid(struct device_node *node)
>   *    extensions by an underscore.
>   */
>  static struct riscv_isa_ext_data isa_ext_arr[] = {
> +	__RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> +	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> +	__RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> +	__RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> +	__RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> +	__RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> +	__RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> +	__RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> +	__RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> +	__RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> +	__RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> +	__RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> +	__RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> +	__RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
>  	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),

I guess a bit of sorting rule might be helpful, here it's the additions
above sscofpmf while in the enum it's the other way around.

As the list will get a long longer over time, I guess consistency
might improve readability.

>  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
>  };
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1b2d42d7f589..10f9daf3734e 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -192,6 +192,39 @@ void __init riscv_fill_hwcap(void)
>  				set_bit(*ext - 'a', this_isa);
>  			} else {
>  				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> +				SET_ISA_EXT_MAP("zba"     , RISCV_ISA_EXT_ZBA     );

not sure if the additional whitespaces are really necessary? [especially the ones at the end]
What did checkpatch have to say about them?

> +				SET_ISA_EXT_MAP("zbb"     , RISCV_ISA_EXT_ZBB     );
> +				SET_ISA_EXT_MAP("zbc"     , RISCV_ISA_EXT_ZBC     );
> +				SET_ISA_EXT_MAP("zbs"     , RISCV_ISA_EXT_ZBS     );
> +				SET_ISA_EXT_MAP("zbkb"    , RISCV_ISA_EXT_ZBKB    );
> +				SET_ISA_EXT_MAP("zbkc"    , RISCV_ISA_EXT_ZBKC    );
> +				SET_ISA_EXT_MAP("zbks"    , RISCV_ISA_EXT_ZBKX    );
> +				SET_ISA_EXT_MAP("zknd"    , RISCV_ISA_EXT_ZKND    );
> +				SET_ISA_EXT_MAP("zkne"    , RISCV_ISA_EXT_ZKNE    );
> +				SET_ISA_EXT_MAP("zknh"    , RISCV_ISA_EXT_ZKNH    );
> +				SET_ISA_EXT_MAP("zksed"   , RISCV_ISA_EXT_ZKSED   );
> +				SET_ISA_EXT_MAP("zksh"    , RISCV_ISA_EXT_ZKSH    );
> +				SET_ISA_EXT_MAP("zkr"     , RISCV_ISA_EXT_ZKR     );
> +				SET_ISA_EXT_MAP("zkt"     , RISCV_ISA_EXT_ZKT     );
> +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKB    );
> +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKC    );
> +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKX    );
> +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKND    );
> +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKNE    );
> +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKNH    );
> +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKB    );
> +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKC    );
> +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKX    );
> +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZKSED   );
> +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZKSH    );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKB    );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKC    );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKX    );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKND    );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKNE    );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKNH    );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKR     );
> +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKT     );
>  			}
>  #undef SET_ISA_EXT_MAP
>  		}
> 

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Hongren (Zenithal) Zheng]

On Wed, May 04, 2022 at 01:21:23AM +0200, Heiko Stuebner wrote:
> Am Samstag, 30. April 2022, 15:50:22 CEST schrieb Hongren (Zenithal) Zheng:
> > This commit parses Zb/Zk related string from DT and
> > output them in cpuinfo
> > 
> > One thing worth noting is that if DT provides zk,
> > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> > 
> > Note that zk is a valid extension name and the current
> > DT binding spec allows this.
> > 
> > There currently lacks a mechanism to merge them when
> > producing cpuinfo. Namely if you provide a riscv,isa
> > "rv64imafdc_zk_zks", the cpuinfo output would be
> > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> > _zksh_zkt"
> > 
> > Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> > Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> > ---
> >  arch/riscv/include/asm/hwcap.h | 14 ++++++++++++++
> >  arch/riscv/kernel/cpu.c        | 14 ++++++++++++++
> >  arch/riscv/kernel/cpufeature.c | 33 +++++++++++++++++++++++++++++++++
> >  3 files changed, 61 insertions(+)
> > 
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 0734e42f74f2..199eda39e0b8 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -52,6 +52,20 @@ extern unsigned long elf_hwcap;
> >   */
> >  enum riscv_isa_ext_id {
> >  	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> > +	RISCV_ISA_EXT_ZBA,
> > +	RISCV_ISA_EXT_ZBB,
> > +	RISCV_ISA_EXT_ZBC,
> > +	RISCV_ISA_EXT_ZBS,
> > +	RISCV_ISA_EXT_ZBKB,
> > +	RISCV_ISA_EXT_ZBKC,
> > +	RISCV_ISA_EXT_ZBKX,
> > +	RISCV_ISA_EXT_ZKNE,
> > +	RISCV_ISA_EXT_ZKND,
> > +	RISCV_ISA_EXT_ZKNH,
> > +	RISCV_ISA_EXT_ZKSED,
> > +	RISCV_ISA_EXT_ZKSH,
> > +	RISCV_ISA_EXT_ZKR,
> > +	RISCV_ISA_EXT_ZKT,
> >  	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> >  };
> >  
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index ccb617791e56..7251336969c1 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -87,6 +87,20 @@ int riscv_of_parent_hartid(struct device_node *node)
> >   *    extensions by an underscore.
> >   */
> >  static struct riscv_isa_ext_data isa_ext_arr[] = {
> > +	__RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> > +	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > +	__RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > +	__RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> > +	__RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> > +	__RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> > +	__RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > +	__RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> > +	__RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> > +	__RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> > +	__RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> > +	__RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> > +	__RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> > +	__RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
> >  	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> 
> I guess a bit of sorting rule might be helpful, here it's the additions
> above sscofpmf while in the enum it's the other way around.

Here the order is defined by the rule that
"Standard supervisor-level extensions (starts with 'S')
should be listed after standard unprivileged extensions."
You can see the comment above this snippet.
You can also see the for loop in print_isa_ext.

Although I do not know should I place 'Zbs' before 'Zbk*'
or after them as 'Zbk*' belong to scalar crypto but
they are prefixed with "b". It seems it is undecided now.

In the enum it is sorted according to the logical id.
As "This enum represent the logical ID" in
arch/riscv/include/asm/hwcap.h said.

> As the list will get a long longer over time, I guess consistency
> might improve readability.

Agreed. I think changing the internal logical id is
acceptable for me and it brings no API change.

I need feedback on whether I could change the logical id
of the existing sscofpmf extension.

> 
> >  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> >  };
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 1b2d42d7f589..10f9daf3734e 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -192,6 +192,39 @@ void __init riscv_fill_hwcap(void)
> >  				set_bit(*ext - 'a', this_isa);
> >  			} else {
> >  				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> > +				SET_ISA_EXT_MAP("zba"     , RISCV_ISA_EXT_ZBA     );
> 
> not sure if the additional whitespaces are really necessary? [especially the ones at the end]
> What did checkpatch have to say about them?

checkpatch did complain about them. Also checkpatch complaint
about the style issue for PATCH 3/3.
It's my fault not to check style before sending.
Will fix the style issue in the next version.
I think I'll send the next version after getting more feedbacks.

> 
> > +				SET_ISA_EXT_MAP("zbb"     , RISCV_ISA_EXT_ZBB     );
> > +				SET_ISA_EXT_MAP("zbc"     , RISCV_ISA_EXT_ZBC     );
> > +				SET_ISA_EXT_MAP("zbs"     , RISCV_ISA_EXT_ZBS     );
> > +				SET_ISA_EXT_MAP("zbkb"    , RISCV_ISA_EXT_ZBKB    );
> > +				SET_ISA_EXT_MAP("zbkc"    , RISCV_ISA_EXT_ZBKC    );
> > +				SET_ISA_EXT_MAP("zbks"    , RISCV_ISA_EXT_ZBKX    );
> > +				SET_ISA_EXT_MAP("zknd"    , RISCV_ISA_EXT_ZKND    );
> > +				SET_ISA_EXT_MAP("zkne"    , RISCV_ISA_EXT_ZKNE    );
> > +				SET_ISA_EXT_MAP("zknh"    , RISCV_ISA_EXT_ZKNH    );
> > +				SET_ISA_EXT_MAP("zksed"   , RISCV_ISA_EXT_ZKSED   );
> > +				SET_ISA_EXT_MAP("zksh"    , RISCV_ISA_EXT_ZKSH    );
> > +				SET_ISA_EXT_MAP("zkr"     , RISCV_ISA_EXT_ZKR     );
> > +				SET_ISA_EXT_MAP("zkt"     , RISCV_ISA_EXT_ZKT     );
> > +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKB    );
> > +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKC    );
> > +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZBKX    );
> > +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKND    );
> > +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKNE    );
> > +				SET_ISA_EXT_MAP("zkn"     , RISCV_ISA_EXT_ZKNH    );
> > +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKB    );
> > +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKC    );
> > +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZBKX    );
> > +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZKSED   );
> > +				SET_ISA_EXT_MAP("zks"     , RISCV_ISA_EXT_ZKSH    );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKB    );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKC    );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZBKX    );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKND    );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKNE    );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKNH    );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKR     );
> > +				SET_ISA_EXT_MAP("zk"      , RISCV_ISA_EXT_ZKT     );
> >  			}
> >  #undef SET_ISA_EXT_MAP
> >  		}
> > 
> 
> 
> 
> 

[PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Samuel Ortiz]

From: "Hongren (Zenithal) Zheng" <i@zenithal.me>

This patch parses Zb/Zk related string from DT and
output them in cpuinfo

One thing worth noting is that if DT provides zk,
all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.

Note that zk is a valid extension name and the current
DT binding spec allows this.

This patch also changes the logical id of
existing multi-letter extensions and adds a statement
that instead of logical id compatibility, the order
is needed.

There currently lacks a mechanism to merge them when
producing cpuinfo. Namely if you provide a riscv,isa
"rv64imafdc_zk_zks", the cpuinfo output would be
"rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
_zksh_zkt"

Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 arch/riscv/include/asm/hwcap.h | 11 +++++++++++
 arch/riscv/kernel/cpu.c        | 11 +++++++++++
 arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index f041bfa7f6a0..b80ca6e77088 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -53,6 +53,17 @@
 #define RISCV_ISA_EXT_ZICSR		40
 #define RISCV_ISA_EXT_ZIFENCEI		41
 #define RISCV_ISA_EXT_ZIHPM		42
+#define RISCV_ISA_EXT_ZBC		43
+#define RISCV_ISA_EXT_ZBKB		44
+#define RISCV_ISA_EXT_ZBKC		45
+#define RISCV_ISA_EXT_ZBKX		46
+#define RISCV_ISA_EXT_ZKND		47
+#define RISCV_ISA_EXT_ZKNE		48
+#define RISCV_ISA_EXT_ZKNH		49
+#define RISCV_ISA_EXT_ZKR		50
+#define RISCV_ISA_EXT_ZKSED		51
+#define RISCV_ISA_EXT_ZKSH		52
+#define RISCV_ISA_EXT_ZKT		53
 
 #define RISCV_ISA_EXT_MAX		64
 #define RISCV_ISA_EXT_NAME_LEN_MAX	32
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index a2fc952318e9..10524322a4c0 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
 	__RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
 	__RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
 	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
+	__RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
+	__RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
+	__RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
+	__RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
 	__RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
+	__RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
+	__RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
+	__RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
+	__RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
+	__RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
+	__RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
+	__RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
 	__RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
 	__RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index bdcf460ea53d..447f853a5a4c 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
 				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
 				SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
 				SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
+				SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
 				SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
+				SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);
+				SET_ISA_EXT_MAP("zbkc", RISCV_ISA_EXT_ZBKC);
+				SET_ISA_EXT_MAP("zbks", RISCV_ISA_EXT_ZBKX);
 				SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
 				SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
 				SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
+				SET_ISA_EXT_MAP("zksed", RISCV_ISA_EXT_ZKSED);
+				SET_ISA_EXT_MAP("zksh", RISCV_ISA_EXT_ZKSH);
+				SET_ISA_EXT_MAP("zkr", RISCV_ISA_EXT_ZKR);
+				SET_ISA_EXT_MAP("zkt", RISCV_ISA_EXT_ZKT);
+				SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKB);
+				SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKC);
+				SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKX);
+				SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKND);
+				SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNE);
+				SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNH);
+				SET_ISA_EXT_MAP("zknd", RISCV_ISA_EXT_ZKND);
+				SET_ISA_EXT_MAP("zkne", RISCV_ISA_EXT_ZKNE);
+				SET_ISA_EXT_MAP("zknh", RISCV_ISA_EXT_ZKNH);
+				SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKB);
+				SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKC);
+				SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKX);
+				SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSED);
+				SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSH);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKB);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKC);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKX);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKND);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNE);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNH);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKR);
+				SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKT);
 			}
 #undef SET_ISA_EXT_MAP
 		}
-- 
2.41.0

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Evan Green]

On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
>
> From: "Hongren (Zenithal) Zheng" <i@zenithal.me>
>
> This patch parses Zb/Zk related string from DT and
> output them in cpuinfo
>
> One thing worth noting is that if DT provides zk,
> all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
>
> Note that zk is a valid extension name and the current
> DT binding spec allows this.
>
> This patch also changes the logical id of
> existing multi-letter extensions and adds a statement
> that instead of logical id compatibility, the order
> is needed.
>
> There currently lacks a mechanism to merge them when
> producing cpuinfo. Namely if you provide a riscv,isa
> "rv64imafdc_zk_zks", the cpuinfo output would be
> "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> _zksh_zkt"
>
> Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> ---
>  arch/riscv/include/asm/hwcap.h | 11 +++++++++++
>  arch/riscv/kernel/cpu.c        | 11 +++++++++++
>  arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index f041bfa7f6a0..b80ca6e77088 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -53,6 +53,17 @@
>  #define RISCV_ISA_EXT_ZICSR            40
>  #define RISCV_ISA_EXT_ZIFENCEI         41
>  #define RISCV_ISA_EXT_ZIHPM            42
> +#define RISCV_ISA_EXT_ZBC              43
> +#define RISCV_ISA_EXT_ZBKB             44
> +#define RISCV_ISA_EXT_ZBKC             45
> +#define RISCV_ISA_EXT_ZBKX             46
> +#define RISCV_ISA_EXT_ZKND             47
> +#define RISCV_ISA_EXT_ZKNE             48
> +#define RISCV_ISA_EXT_ZKNH             49
> +#define RISCV_ISA_EXT_ZKR              50
> +#define RISCV_ISA_EXT_ZKSED            51
> +#define RISCV_ISA_EXT_ZKSH             52
> +#define RISCV_ISA_EXT_ZKT              53
>
>  #define RISCV_ISA_EXT_MAX              64
>  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index a2fc952318e9..10524322a4c0 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
>         __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
>         __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
>         __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> +       __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> +       __RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> +       __RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> +       __RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
>         __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> +       __RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> +       __RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> +       __RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> +       __RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> +       __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> +       __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> +       __RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
>         __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
>         __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index bdcf460ea53d..447f853a5a4c 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
>                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
>                                 SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
>                                 SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> +                               SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
>                                 SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> +                               SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);
> +                               SET_ISA_EXT_MAP("zbkc", RISCV_ISA_EXT_ZBKC);
> +                               SET_ISA_EXT_MAP("zbks", RISCV_ISA_EXT_ZBKX);

Should "zbks" be "zbkx"?

>                                 SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
>                                 SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
>                                 SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> +                               SET_ISA_EXT_MAP("zksed", RISCV_ISA_EXT_ZKSED);
> +                               SET_ISA_EXT_MAP("zksh", RISCV_ISA_EXT_ZKSH);
> +                               SET_ISA_EXT_MAP("zkr", RISCV_ISA_EXT_ZKR);
> +                               SET_ISA_EXT_MAP("zkt", RISCV_ISA_EXT_ZKT);
> +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKB);
> +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKC);
> +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKX);
> +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKND);
> +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNE);
> +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNH);
> +                               SET_ISA_EXT_MAP("zknd", RISCV_ISA_EXT_ZKND);
> +                               SET_ISA_EXT_MAP("zkne", RISCV_ISA_EXT_ZKNE);
> +                               SET_ISA_EXT_MAP("zknh", RISCV_ISA_EXT_ZKNH);
> +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKB);
> +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKC);
> +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKX);
> +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSED);
> +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSH);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKB);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKC);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKX);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKND);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNE);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNH);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKR);
> +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKT);

It would be nice to consolidate the ones together that search for a
single string and set multiple bits, though I don't have any super
elegant ideas for how off the top of my head.

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Hongren (Zenithal) Zheng]

On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> >
> > From: "Hongren (Zenithal) Zheng" <i@zenithal.me>
> >
> > This patch parses Zb/Zk related string from DT and
> > output them in cpuinfo
> >
> > One thing worth noting is that if DT provides zk,
> > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> >
> > Note that zk is a valid extension name and the current
> > DT binding spec allows this.
> >
> > This patch also changes the logical id of
> > existing multi-letter extensions and adds a statement
> > that instead of logical id compatibility, the order
> > is needed.
> >
> > There currently lacks a mechanism to merge them when
> > producing cpuinfo. Namely if you provide a riscv,isa
> > "rv64imafdc_zk_zks", the cpuinfo output would be
> > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> > _zksh_zkt"
> >
> > Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> > Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>

I think an extra line of your own signed-off-by is needed

> > ---
> >  arch/riscv/include/asm/hwcap.h | 11 +++++++++++
> >  arch/riscv/kernel/cpu.c        | 11 +++++++++++
> >  arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
> >  3 files changed, 52 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index f041bfa7f6a0..b80ca6e77088 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -53,6 +53,17 @@
> >  #define RISCV_ISA_EXT_ZICSR            40
> >  #define RISCV_ISA_EXT_ZIFENCEI         41
> >  #define RISCV_ISA_EXT_ZIHPM            42
> > +#define RISCV_ISA_EXT_ZBC              43
> > +#define RISCV_ISA_EXT_ZBKB             44
> > +#define RISCV_ISA_EXT_ZBKC             45
> > +#define RISCV_ISA_EXT_ZBKX             46
> > +#define RISCV_ISA_EXT_ZKND             47
> > +#define RISCV_ISA_EXT_ZKNE             48
> > +#define RISCV_ISA_EXT_ZKNH             49
> > +#define RISCV_ISA_EXT_ZKR              50
> > +#define RISCV_ISA_EXT_ZKSED            51
> > +#define RISCV_ISA_EXT_ZKSH             52
> > +#define RISCV_ISA_EXT_ZKT              53
> >
> >  #define RISCV_ISA_EXT_MAX              64
> >  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index a2fc952318e9..10524322a4c0 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> >         __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
> >         __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> >         __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > +       __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > +       __RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> > +       __RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> > +       __RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> >         __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > +       __RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> > +       __RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> > +       __RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> > +       __RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> > +       __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> > +       __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> > +       __RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
> >         __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> >         __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index bdcf460ea53d..447f853a5a4c 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
> >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> >                                 SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
> >                                 SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> > +                               SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
> >                                 SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> > +                               SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);
> > +                               SET_ISA_EXT_MAP("zbkc", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zbks", RISCV_ISA_EXT_ZBKX);
> 
> Should "zbks" be "zbkx"?

Yes that is a nice catch!

> 
> >                                 SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> >                                 SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> >                                 SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > +                               SET_ISA_EXT_MAP("zksed", RISCV_ISA_EXT_ZKSED);
> > +                               SET_ISA_EXT_MAP("zksh", RISCV_ISA_EXT_ZKSH);
> > +                               SET_ISA_EXT_MAP("zkr", RISCV_ISA_EXT_ZKR);
> > +                               SET_ISA_EXT_MAP("zkt", RISCV_ISA_EXT_ZKT);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKB);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKX);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKND);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNE);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNH);
> > +                               SET_ISA_EXT_MAP("zknd", RISCV_ISA_EXT_ZKND);
> > +                               SET_ISA_EXT_MAP("zkne", RISCV_ISA_EXT_ZKNE);
> > +                               SET_ISA_EXT_MAP("zknh", RISCV_ISA_EXT_ZKNH);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKB);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKX);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSED);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSH);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKB);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKX);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKND);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNE);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNH);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKR);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKT);
> 
> It would be nice to consolidate the ones together that search for a
> single string and set multiple bits, though I don't have any super
> elegant ideas for how off the top of my head.

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Conor Dooley]

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

On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> >
> > From: "Hongren (Zenithal) Zheng" <i@zenithal.me>
> >
> > This patch parses Zb/Zk related string from DT and

%s/This patch//

> > output them in cpuinfo
> >
> > One thing worth noting is that if DT provides zk,
> > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.

Please explain why this is okay.

> > Note that zk is a valid extension name and the current
> > DT binding spec allows this.
> >
> > This patch also changes the logical id of
> > existing multi-letter extensions and adds a statement
> > that instead of logical id compatibility, the order
> > is needed.

Does it?

> > There currently lacks a mechanism to merge them when
> > producing cpuinfo. Namely if you provide a riscv,isa
> > "rv64imafdc_zk_zks", the cpuinfo output would be
> > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> > _zksh_zkt"

I think this is fine.

Please re-wrap this all to 72 characters.

> >
> > Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> > Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>

This is missing your SoB Samuel.

> > ---
> >  arch/riscv/include/asm/hwcap.h | 11 +++++++++++
> >  arch/riscv/kernel/cpu.c        | 11 +++++++++++
> >  arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
> >  3 files changed, 52 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index f041bfa7f6a0..b80ca6e77088 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -53,6 +53,17 @@
> >  #define RISCV_ISA_EXT_ZICSR            40
> >  #define RISCV_ISA_EXT_ZIFENCEI         41
> >  #define RISCV_ISA_EXT_ZIHPM            42
> > +#define RISCV_ISA_EXT_ZBC              43
> > +#define RISCV_ISA_EXT_ZBKB             44
> > +#define RISCV_ISA_EXT_ZBKC             45
> > +#define RISCV_ISA_EXT_ZBKX             46
> > +#define RISCV_ISA_EXT_ZKND             47
> > +#define RISCV_ISA_EXT_ZKNE             48
> > +#define RISCV_ISA_EXT_ZKNH             49
> > +#define RISCV_ISA_EXT_ZKR              50
> > +#define RISCV_ISA_EXT_ZKSED            51
> > +#define RISCV_ISA_EXT_ZKSH             52
> > +#define RISCV_ISA_EXT_ZKT              53
> >
> >  #define RISCV_ISA_EXT_MAX              64
> >  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index a2fc952318e9..10524322a4c0 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> >         __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
> >         __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> >         __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > +       __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > +       __RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> > +       __RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> > +       __RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> >         __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > +       __RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> > +       __RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> > +       __RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> > +       __RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> > +       __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> > +       __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> > +       __RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
> >         __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> >         __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index bdcf460ea53d..447f853a5a4c 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
> >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> >                                 SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
> >                                 SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> > +                               SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
> >                                 SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> > +                               SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);

This order does not look correct, please add them in alphanumerical
order as the comment these SET_ISA_EXT_MAP()s requests. Ditto below.

> > +                               SET_ISA_EXT_MAP("zbkc", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zbks", RISCV_ISA_EXT_ZBKX);
> 
> Should "zbks" be "zbkx"?
> 
> >                                 SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> >                                 SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> >                                 SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > +                               SET_ISA_EXT_MAP("zksed", RISCV_ISA_EXT_ZKSED);
> > +                               SET_ISA_EXT_MAP("zksh", RISCV_ISA_EXT_ZKSH);
> > +                               SET_ISA_EXT_MAP("zkr", RISCV_ISA_EXT_ZKR);
> > +                               SET_ISA_EXT_MAP("zkt", RISCV_ISA_EXT_ZKT);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKB);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKX);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKND);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNE);
> > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNH);
> > +                               SET_ISA_EXT_MAP("zknd", RISCV_ISA_EXT_ZKND);
> > +                               SET_ISA_EXT_MAP("zkne", RISCV_ISA_EXT_ZKNE);
> > +                               SET_ISA_EXT_MAP("zknh", RISCV_ISA_EXT_ZKNH);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKB);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKX);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSED);
> > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSH);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKB);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKC);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKX);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKND);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNE);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNH);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKR);
> > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKT);
> 
> It would be nice to consolidate the ones together that search for a
> single string and set multiple bits, though I don't have any super
> elegant ideas for how off the top of my head.

I've got a refactor of this code in progress, dropping all of these
copy-paste in place of a loop. It certainly looks more elegant than
this, but it will fall over a bit for these "one string matches many
extensions" cases. See here:
https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
My immediate thought is to add another element to riscv_isa_ext_data,
that contains "parent" extensions to check for. Should be fairly doable,
I'll whip something up on top of that...

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Hongren (Zenithal) Zheng]

On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> > >
> > > From: "Hongren (Zenithal) Zheng" <i@zenithal.me>
> > >
> > > This patch parses Zb/Zk related string from DT and
> 
> %s/This patch//
> 
> > > output them in cpuinfo
> > >
> > > One thing worth noting is that if DT provides zk,
> > > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> 
> Please explain why this is okay.

From riscv scalar crypto spec, zk is a shorthand
for zkn, zkr and zkt and zkn also includes zbkb, zbkc
and zbkx.

> 
> > > Note that zk is a valid extension name and the current
> > > DT binding spec allows this.
> > >
> > > This patch also changes the logical id of
> > > existing multi-letter extensions and adds a statement
> > > that instead of logical id compatibility, the order
> > > is needed.
> 
> Does it?

That is in the old version of this patch,
should be removed now
see https://lore.kernel.org/linux-riscv/YqY0aSngjI0Hc5d4@Sun/

> 
> > > There currently lacks a mechanism to merge them when
> > > producing cpuinfo. Namely if you provide a riscv,isa
> > > "rv64imafdc_zk_zks", the cpuinfo output would be
> > > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> > > _zksh_zkt"
> 
> I think this is fine.
> 
> Please re-wrap this all to 72 characters.
> 
> > >
> > > Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> > > Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> 
> This is missing your SoB Samuel.
> 
> > > ---
> > >  arch/riscv/include/asm/hwcap.h | 11 +++++++++++
> > >  arch/riscv/kernel/cpu.c        | 11 +++++++++++
> > >  arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
> > >  3 files changed, 52 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > index f041bfa7f6a0..b80ca6e77088 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -53,6 +53,17 @@
> > >  #define RISCV_ISA_EXT_ZICSR            40
> > >  #define RISCV_ISA_EXT_ZIFENCEI         41
> > >  #define RISCV_ISA_EXT_ZIHPM            42
> > > +#define RISCV_ISA_EXT_ZBC              43
> > > +#define RISCV_ISA_EXT_ZBKB             44
> > > +#define RISCV_ISA_EXT_ZBKC             45
> > > +#define RISCV_ISA_EXT_ZBKX             46
> > > +#define RISCV_ISA_EXT_ZKND             47
> > > +#define RISCV_ISA_EXT_ZKNE             48
> > > +#define RISCV_ISA_EXT_ZKNH             49
> > > +#define RISCV_ISA_EXT_ZKR              50
> > > +#define RISCV_ISA_EXT_ZKSED            51
> > > +#define RISCV_ISA_EXT_ZKSH             52
> > > +#define RISCV_ISA_EXT_ZKT              53
> > >
> > >  #define RISCV_ISA_EXT_MAX              64
> > >  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > index a2fc952318e9..10524322a4c0 100644
> > > --- a/arch/riscv/kernel/cpu.c
> > > +++ b/arch/riscv/kernel/cpu.c
> > > @@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> > >         __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
> > >         __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> > >         __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > > +       __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > > +       __RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> > > +       __RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> > > +       __RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> > >         __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > > +       __RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> > > +       __RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> > > +       __RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> > > +       __RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> > > +       __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> > > +       __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> > > +       __RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
> > >         __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > >         __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index bdcf460ea53d..447f853a5a4c 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
> > >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > >                                 SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
> > >                                 SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> > > +                               SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
> > >                                 SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> > > +                               SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);
> 
> This order does not look correct, please add them in alphanumerical
> order as the comment these SET_ISA_EXT_MAP()s requests. Ditto below.

Agreed. Seems that I did not worked carefully for this part.

> 
> > > +                               SET_ISA_EXT_MAP("zbkc", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zbks", RISCV_ISA_EXT_ZBKX);
> > 
> > Should "zbks" be "zbkx"?
> > 
> > >                                 SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> > >                                 SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> > >                                 SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > +                               SET_ISA_EXT_MAP("zksed", RISCV_ISA_EXT_ZKSED);
> > > +                               SET_ISA_EXT_MAP("zksh", RISCV_ISA_EXT_ZKSH);
> > > +                               SET_ISA_EXT_MAP("zkr", RISCV_ISA_EXT_ZKR);
> > > +                               SET_ISA_EXT_MAP("zkt", RISCV_ISA_EXT_ZKT);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKB);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKX);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKND);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNE);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNH);
> > > +                               SET_ISA_EXT_MAP("zknd", RISCV_ISA_EXT_ZKND);
> > > +                               SET_ISA_EXT_MAP("zkne", RISCV_ISA_EXT_ZKNE);
> > > +                               SET_ISA_EXT_MAP("zknh", RISCV_ISA_EXT_ZKNH);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKB);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKX);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSED);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSH);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKB);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKX);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKND);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNE);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNH);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKR);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKT);
> > 
> > It would be nice to consolidate the ones together that search for a
> > single string and set multiple bits, though I don't have any super
> > elegant ideas for how off the top of my head.
> 
> I've got a refactor of this code in progress, dropping all of these
> copy-paste in place of a loop. It certainly looks more elegant than
> this, but it will fall over a bit for these "one string matches many
> extensions" cases. See here:
> https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> My immediate thought is to add another element to riscv_isa_ext_data,
> that contains "parent" extensions to check for. Should be fairly doable,
> I'll whip something up on top of that...
> 
> Cheers,
> Conor.

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Conor Dooley]

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

On Wed, Jun 28, 2023 at 03:03:58AM +0800, Hongren (Zenithal) Zheng wrote:
> On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> > > >
> > > > From: "Hongren (Zenithal) Zheng" <i@zenithal.me>
> > > >
> > > > This patch parses Zb/Zk related string from DT and
> > 
> > %s/This patch//
> > 
> > > > output them in cpuinfo
> > > >
> > > > One thing worth noting is that if DT provides zk,
> > > > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> > 
> > Please explain why this is okay.
> 
> From riscv scalar crypto spec, zk is a shorthand
> for zkn, zkr and zkt and zkn also includes zbkb, zbkc
> and zbkx.

Hmm, seems you misunderstood, sorry about that.
What I was looking for is an explanation of this in the commit message.

Hope that helps,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Samuel Ortiz]

On Wed, Jun 28, 2023 at 03:03:58AM +0800, Hongren (Zenithal) Zheng wrote:
> On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> > > >
> > > > From: "Hongren (Zenithal) Zheng" <i@zenithal.me>
> > > >
> > > > This patch parses Zb/Zk related string from DT and
> > 
> > %s/This patch//
> > 
> > > > output them in cpuinfo
> > > >
> > > > One thing worth noting is that if DT provides zk,
> > > > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> > 
> > Please explain why this is okay.
> 
> From riscv scalar crypto spec, zk is a shorthand
> for zkn, zkr and zkt and zkn also includes zbkb, zbkc
> and zbkx.
> 
> > 
> > > > Note that zk is a valid extension name and the current
> > > > DT binding spec allows this.
> > > >
> > > > This patch also changes the logical id of
> > > > existing multi-letter extensions and adds a statement
> > > > that instead of logical id compatibility, the order
> > > > is needed.
> > 
> > Does it?
> 
> That is in the old version of this patch,
> should be removed now
> see https://lore.kernel.org/linux-riscv/YqY0aSngjI0Hc5d4@Sun/
> 
> > 
> > > > There currently lacks a mechanism to merge them when
> > > > producing cpuinfo. Namely if you provide a riscv,isa
> > > > "rv64imafdc_zk_zks", the cpuinfo output would be
> > > > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> > > > _zksh_zkt"
> > 
> > I think this is fine.
> > 
> > Please re-wrap this all to 72 characters.
> > 
> > > >
> > > > Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> > > > Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> > 
> > This is missing your SoB Samuel.
> > 
> > > > ---
> > > >  arch/riscv/include/asm/hwcap.h | 11 +++++++++++
> > > >  arch/riscv/kernel/cpu.c        | 11 +++++++++++
> > > >  arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
> > > >  3 files changed, 52 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > > index f041bfa7f6a0..b80ca6e77088 100644
> > > > --- a/arch/riscv/include/asm/hwcap.h
> > > > +++ b/arch/riscv/include/asm/hwcap.h
> > > > @@ -53,6 +53,17 @@
> > > >  #define RISCV_ISA_EXT_ZICSR            40
> > > >  #define RISCV_ISA_EXT_ZIFENCEI         41
> > > >  #define RISCV_ISA_EXT_ZIHPM            42
> > > > +#define RISCV_ISA_EXT_ZBC              43
> > > > +#define RISCV_ISA_EXT_ZBKB             44
> > > > +#define RISCV_ISA_EXT_ZBKC             45
> > > > +#define RISCV_ISA_EXT_ZBKX             46
> > > > +#define RISCV_ISA_EXT_ZKND             47
> > > > +#define RISCV_ISA_EXT_ZKNE             48
> > > > +#define RISCV_ISA_EXT_ZKNH             49
> > > > +#define RISCV_ISA_EXT_ZKR              50
> > > > +#define RISCV_ISA_EXT_ZKSED            51
> > > > +#define RISCV_ISA_EXT_ZKSH             52
> > > > +#define RISCV_ISA_EXT_ZKT              53
> > > >
> > > >  #define RISCV_ISA_EXT_MAX              64
> > > >  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > > index a2fc952318e9..10524322a4c0 100644
> > > > --- a/arch/riscv/kernel/cpu.c
> > > > +++ b/arch/riscv/kernel/cpu.c
> > > > @@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > >         __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
> > > >         __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> > > >         __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > > > +       __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > > > +       __RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> > > > +       __RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> > > > +       __RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> > > >         __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > > > +       __RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> > > > +       __RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> > > > +       __RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> > > > +       __RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> > > > +       __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> > > > +       __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> > > > +       __RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
> > > >         __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > >         __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > > >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > index bdcf460ea53d..447f853a5a4c 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
> > > >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > > >                                 SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
> > > >                                 SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> > > > +                               SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
> > > >                                 SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> > > > +                               SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);
> > 
> > This order does not look correct, please add them in alphanumerical
> > order as the comment these SET_ISA_EXT_MAP()s requests. Ditto below.
> 
> Agreed. Seems that I did not worked carefully for this part.

Or it could be me when rebasing that patch. In any case, it will be
fixed with v2 of the patch.

Cheers,
Samuel.

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Samuel Ortiz]

On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> > >
> > > From: "Hongren (Zenithal) Zheng" <i@zenithal.me>
> > >
> > > This patch parses Zb/Zk related string from DT and
> 
> %s/This patch//
> 
> > > output them in cpuinfo
> > >
> > > One thing worth noting is that if DT provides zk,
> > > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> 
> Please explain why this is okay.
> 
> > > Note that zk is a valid extension name and the current
> > > DT binding spec allows this.
> > >
> > > This patch also changes the logical id of
> > > existing multi-letter extensions and adds a statement
> > > that instead of logical id compatibility, the order
> > > is needed.
> 
> Does it?
> 
> > > There currently lacks a mechanism to merge them when
> > > producing cpuinfo. Namely if you provide a riscv,isa
> > > "rv64imafdc_zk_zks", the cpuinfo output would be
> > > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> > > _zksh_zkt"
> 
> I think this is fine.
> 
> Please re-wrap this all to 72 characters.
> 
> > >
> > > Tested-by: Jiatai He <jiatai2021@iscas.ac.cn>
> > > Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> 
> This is missing your SoB Samuel.
> 
> > > ---
> > >  arch/riscv/include/asm/hwcap.h | 11 +++++++++++
> > >  arch/riscv/kernel/cpu.c        | 11 +++++++++++
> > >  arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
> > >  3 files changed, 52 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > index f041bfa7f6a0..b80ca6e77088 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -53,6 +53,17 @@
> > >  #define RISCV_ISA_EXT_ZICSR            40
> > >  #define RISCV_ISA_EXT_ZIFENCEI         41
> > >  #define RISCV_ISA_EXT_ZIHPM            42
> > > +#define RISCV_ISA_EXT_ZBC              43
> > > +#define RISCV_ISA_EXT_ZBKB             44
> > > +#define RISCV_ISA_EXT_ZBKC             45
> > > +#define RISCV_ISA_EXT_ZBKX             46
> > > +#define RISCV_ISA_EXT_ZKND             47
> > > +#define RISCV_ISA_EXT_ZKNE             48
> > > +#define RISCV_ISA_EXT_ZKNH             49
> > > +#define RISCV_ISA_EXT_ZKR              50
> > > +#define RISCV_ISA_EXT_ZKSED            51
> > > +#define RISCV_ISA_EXT_ZKSH             52
> > > +#define RISCV_ISA_EXT_ZKT              53
> > >
> > >  #define RISCV_ISA_EXT_MAX              64
> > >  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > index a2fc952318e9..10524322a4c0 100644
> > > --- a/arch/riscv/kernel/cpu.c
> > > +++ b/arch/riscv/kernel/cpu.c
> > > @@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> > >         __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
> > >         __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> > >         __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > > +       __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > > +       __RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> > > +       __RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> > > +       __RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> > >         __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > > +       __RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> > > +       __RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> > > +       __RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> > > +       __RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> > > +       __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> > > +       __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> > > +       __RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
> > >         __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > >         __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index bdcf460ea53d..447f853a5a4c 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
> > >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > >                                 SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
> > >                                 SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> > > +                               SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
> > >                                 SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> > > +                               SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);
> 
> This order does not look correct, please add them in alphanumerical
> order as the comment these SET_ISA_EXT_MAP()s requests. Ditto below.
> 
> > > +                               SET_ISA_EXT_MAP("zbkc", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zbks", RISCV_ISA_EXT_ZBKX);
> > 
> > Should "zbks" be "zbkx"?
> > 
> > >                                 SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> > >                                 SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> > >                                 SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > +                               SET_ISA_EXT_MAP("zksed", RISCV_ISA_EXT_ZKSED);
> > > +                               SET_ISA_EXT_MAP("zksh", RISCV_ISA_EXT_ZKSH);
> > > +                               SET_ISA_EXT_MAP("zkr", RISCV_ISA_EXT_ZKR);
> > > +                               SET_ISA_EXT_MAP("zkt", RISCV_ISA_EXT_ZKT);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKB);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZBKX);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKND);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNE);
> > > +                               SET_ISA_EXT_MAP("zkn", RISCV_ISA_EXT_ZKNH);
> > > +                               SET_ISA_EXT_MAP("zknd", RISCV_ISA_EXT_ZKND);
> > > +                               SET_ISA_EXT_MAP("zkne", RISCV_ISA_EXT_ZKNE);
> > > +                               SET_ISA_EXT_MAP("zknh", RISCV_ISA_EXT_ZKNH);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKB);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZBKX);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSED);
> > > +                               SET_ISA_EXT_MAP("zks", RISCV_ISA_EXT_ZKSH);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKB);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKC);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZBKX);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKND);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNE);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKNH);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKR);
> > > +                               SET_ISA_EXT_MAP("zk", RISCV_ISA_EXT_ZKT);
> > 
> > It would be nice to consolidate the ones together that search for a
> > single string and set multiple bits, though I don't have any super
> > elegant ideas for how off the top of my head.
> 
> I've got a refactor of this code in progress, dropping all of these
> copy-paste in place of a loop. It certainly looks more elegant than
> this, but it will fall over a bit for these "one string matches many
> extensions" cases. See here:
> https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> My immediate thought is to add another element to riscv_isa_ext_data,
> that contains "parent" extensions to check for. Should be fairly doable,
> I'll whip something up on top of that...

Nice, and thanks for the review.
Should I wait for your refactor to be merged before pushing this one?

Cheers,
Samuel.

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Conor Dooley]

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

On Wed, Jun 28, 2023 at 12:01:11PM +0200, Samuel Ortiz wrote:
> On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:

> > > It would be nice to consolidate the ones together that search for a
> > > single string and set multiple bits, though I don't have any super
> > > elegant ideas for how off the top of my head.
> > 
> > I've got a refactor of this code in progress, dropping all of these
> > copy-paste in place of a loop. It certainly looks more elegant than
> > this, but it will fall over a bit for these "one string matches many
> > extensions" cases. See here:
> > https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> > My immediate thought is to add another element to riscv_isa_ext_data,
> > that contains "parent" extensions to check for. Should be fairly doable,
> > I'll whip something up on top of that...
> 
> Nice, and thanks for the review.

> Should I wait for your refactor to be merged before pushing this one?

I don't know. I think that you should continue on with your series here,
and whichever goes in second gets rebased on top of the other.
I don't think it makes material difference to review of this patchset as
to whether you rebase on top of what I'm working on, so I wouldn't
bother until it gets merged.

Rather hacky, had less time than expected this morning:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-extensions-strings-supersets
Clearly there's issues with looping to RISCV_ISA_MAX_SUPERSETS & I just
repurposed Zicsr for the sake of testing something in the time I had.

Evan, at a high level, does that look more elegant to you, or have I made
things worse?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Samuel Ortiz]

On Wed, Jun 28, 2023 at 01:21:01PM +0200, Markus Elfring wrote:
> …
> > This patch also changes the logical id of
> …
> 
> Does such a wording really fit to the known requirement “Solve only one problem per patch.”?

That comment is inaccurate anyways. I removed it from v2 of the patch
set.

Cheers,
Samuel.


> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n81
> 
> 
> Please choose an imperative change suggestion.
> 
> Regards,
> Markus

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Samuel Ortiz]

On Wed, Jun 28, 2023 at 12:10:11PM +0100, Conor Dooley wrote:
> On Wed, Jun 28, 2023 at 12:01:11PM +0200, Samuel Ortiz wrote:
> > On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> 
> > > > It would be nice to consolidate the ones together that search for a
> > > > single string and set multiple bits, though I don't have any super
> > > > elegant ideas for how off the top of my head.
> > > 
> > > I've got a refactor of this code in progress, dropping all of these
> > > copy-paste in place of a loop. It certainly looks more elegant than
> > > this, but it will fall over a bit for these "one string matches many
> > > extensions" cases. See here:
> > > https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> > > My immediate thought is to add another element to riscv_isa_ext_data,
> > > that contains "parent" extensions to check for. Should be fairly doable,
> > > I'll whip something up on top of that...
> > 
> > Nice, and thanks for the review.
> 
> > Should I wait for your refactor to be merged before pushing this one?
> 
> I don't know. I think that you should continue on with your series here,
> and whichever goes in second gets rebased on top of the other.

Sounds good to me, thanks.

Cheers,
Samuel.

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Conor Dooley]

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

On Wed, Jun 28, 2023 at 12:10:11PM +0100, Conor Dooley wrote:
> On Wed, Jun 28, 2023 at 12:01:11PM +0200, Samuel Ortiz wrote:
> > On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> 
> > > > It would be nice to consolidate the ones together that search for a
> > > > single string and set multiple bits, though I don't have any super
> > > > elegant ideas for how off the top of my head.
> > > 
> > > I've got a refactor of this code in progress, dropping all of these
> > > copy-paste in place of a loop. It certainly looks more elegant than
> > > this, but it will fall over a bit for these "one string matches many
> > > extensions" cases. See here:
> > > https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> > > My immediate thought is to add another element to riscv_isa_ext_data,
> > > that contains "parent" extensions to check for. Should be fairly doable,
> > > I'll whip something up on top of that...
> > 
> > Nice, and thanks for the review.
> 
> > Should I wait for your refactor to be merged before pushing this one?
> 
> I don't know. I think that you should continue on with your series here,
> and whichever goes in second gets rebased on top of the other.
> I don't think it makes material difference to review of this patchset as
> to whether you rebase on top of what I'm working on, so I wouldn't
> bother until it gets merged.
> 
> Rather hacky, had less time than expected this morning:
> https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-extensions-strings-supersets
> Clearly there's issues with looping to RISCV_ISA_MAX_SUPERSETS & I just
> repurposed Zicsr for the sake of testing something in the time I had.
> 
> Evan, at a high level, does that look more elegant to you, or have I made
> things worse?

Got some more time this afternoon, cleaned it up a bit. On top of what
is in
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=riscv-extensions-strings
IOW, the not-yet-sent v2 of
https://lore.kernel.org/all/20230626-provable-angrily-81760e8c3cc6@wendy/
here's some infra for doing superset stuff...

Going to send my v2 without this, as there's nothing in tree right now
that actually needs this sort of thing.

-- >8 --
From 0875e1aa2bf773b53cae02490ebc69e798e491c4 Mon Sep 17 00:00:00 2001
From: Conor Dooley <conor.dooley@microchip.com>
Date: Wed, 28 Jun 2023 12:01:40 +0100
Subject: [PATCH] RISC-V: detect extension support from superset extensions

Some extensions, for example scalar crypto's Zk extension, are comprised
of anumber of sub-extensions that may be implemented independently.
Provide some infrastructure for detecting support for an extension where
only its superset appears in DT or ACPI.
SET_ISA_EXT_MAP() already served little purpose, move the code into an
inline function instead, where the code can be reused more easily by the
riscv_try_set_ext_from_supersets().

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/hwcap.h |   3 +
 arch/riscv/kernel/cpufeature.c | 107 ++++++++++++++++++++++++++++-----
 2 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index c4d6faa5cdf8..5b41a7aa9ec5 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -73,11 +73,14 @@
 
 unsigned long riscv_get_elf_hwcap(void);
 
+#define RISCV_ISA_MAX_SUPERSETS 1
 struct riscv_isa_ext_data {
 	const unsigned int id;
 	const char *name;
 	const char *property;
 	const bool multi_letter;
+	const unsigned int num_supersets;
+	const char *supersets[RISCV_ISA_MAX_SUPERSETS];
 };
 
 extern const struct riscv_isa_ext_data riscv_isa_ext[];
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 53b38f6c0562..0d56f4a11a3e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -104,6 +104,16 @@ static bool riscv_isa_extension_check(int id)
 	.property = #_name,				\
 	.id = _id,					\
 	.multi_letter = _multi,				\
+	.num_supersets = 0,				\
+}
+
+#define __RISCV_ISA_EXT_DATA_SUBSET(_name, _id, _multi, _num_supersets, ...) {	\
+	.name = #_name,								\
+	.property = #_name,							\
+	.id = _id,								\
+	.multi_letter = _multi,							\
+	.num_supersets = _num_supersets,					\
+	.supersets = {__VA_ARGS__},						\
 }
 
 /*
@@ -180,6 +190,39 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 
 const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
 
+static inline int __init riscv_try_set_ext(const char *name, const unsigned int bit,
+					   const char *ext, const char *ext_end,
+					   struct riscv_isainfo *isainfo)
+{
+	if ((ext_end - ext == strlen(name)) &&
+	    !strncasecmp(ext, name, strlen(name)) &&
+	    riscv_isa_extension_check(bit)) {
+			set_bit(bit, isainfo->isa);
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+static inline void __init riscv_try_set_ext_from_supersets(struct riscv_isa_ext_data ext_data,
+							   const char *ext, const char *ext_end,
+							   struct riscv_isainfo *isainfo)
+{
+	for (int i = 0; i < ext_data.num_supersets; i++) {
+		const char *superset = ext_data.supersets[i];
+		const int bit = ext_data.id;
+		int ret;
+
+		/*
+		 * If the extension that's been found is a superset, there's no
+		 * reason to keep looking for other supersets.
+		 */
+		ret = riscv_try_set_ext(superset, bit, ext, ext_end, isainfo);
+		if (!ret)
+			return;
+	}
+}
+
 static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo,
 					  unsigned long *isa2hwcap, const char *isa)
 {
@@ -311,14 +354,6 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc
 		if (*isa == '_')
 			++isa;
 
-#define SET_ISA_EXT_MAP(name, bit)						\
-		do {								\
-			if ((ext_end - ext == sizeof(name) - 1) &&		\
-			     !strncasecmp(ext, name, sizeof(name) - 1) &&	\
-			     riscv_isa_extension_check(bit))			\
-				set_bit(bit, isainfo->isa);			\
-		} while (false)							\
-
 		if (unlikely(ext_err))
 			continue;
 		if (!ext_long) {
@@ -328,12 +363,27 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc
 				*this_hwcap |= isa2hwcap[nr];
 				set_bit(nr, isainfo->isa);
 			}
-		} else {
+
 			for (int i = 0; i < riscv_isa_ext_count; i++)
-				SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
-						riscv_isa_ext[i].id);
+				riscv_try_set_ext_from_supersets(riscv_isa_ext[i],
+								 ext, ext_end, isainfo);
+		} else {
+			for (int i = 0; i < riscv_isa_ext_count; i++) {
+				const char *name = riscv_isa_ext[i].name;
+				const int bit = riscv_isa_ext[i].id;
+				int ret;
+
+				ret = riscv_try_set_ext(name, bit, ext, ext_end, isainfo);
+
+				/*
+				 * There's no point checking if the extension that the parser has
+				 * just found is a superset, if it is the extension itself...
+				 */
+				if (!ret)
+					riscv_try_set_ext_from_supersets(riscv_isa_ext[i],
+									 ext, ext_end, isainfo);
+			}
 		}
-#undef SET_ISA_EXT_MAP
 	}
 }
 
@@ -416,6 +466,28 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
 		acpi_put_table((struct acpi_table_header *)rhct);
 }
 
+static inline bool riscv_ext_superset_present(struct riscv_isa_ext_data ext_data,
+					      struct device_node *cpu_node)
+{
+	if (!ext_data.num_supersets)
+		return false;
+
+	for (int i = 0; i < ext_data.num_supersets; i++) {
+		const char *superset = ext_data.supersets[i];
+		int ret;
+
+		/*
+		 * Once a single superset is found, there's no point looking
+		 * for any other ones.
+		 */
+		ret = of_property_match_string(cpu_node,"riscv,isa-extensions", superset);
+		if (ret >= 0)
+			return true;
+	}
+
+	return false;
+}
+
 static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
 {
 	unsigned int cpu;
@@ -435,8 +507,15 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
 			continue;
 
 		for (int i = 0; i < riscv_isa_ext_count; i++) {
-			if (of_property_match_string(cpu_node, "riscv,isa-extensions",
-						     riscv_isa_ext[i].name) < 0)
+			int ret = of_property_match_string(cpu_node, "riscv,isa-extensions",
+							   riscv_isa_ext[i].name);
+
+			/*
+			 * If the extension itself is not found it could be a
+			 * subset of another extension, so the supersets need to
+			 * be checked for too.
+			 */
+			if (ret < 0 && !riscv_ext_superset_present(riscv_isa_ext[i], cpu_node))
 				continue;
 
 			if (!riscv_isa_extension_check(riscv_isa_ext[i].id))
-- 
2.39.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Evan Green]

On Wed, Jun 28, 2023 at 4:10 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Wed, Jun 28, 2023 at 12:01:11PM +0200, Samuel Ortiz wrote:
> > On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
>
> > > > It would be nice to consolidate the ones together that search for a
> > > > single string and set multiple bits, though I don't have any super
> > > > elegant ideas for how off the top of my head.
> > >
> > > I've got a refactor of this code in progress, dropping all of these
> > > copy-paste in place of a loop. It certainly looks more elegant than
> > > this, but it will fall over a bit for these "one string matches many
> > > extensions" cases. See here:
> > > https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> > > My immediate thought is to add another element to riscv_isa_ext_data,
> > > that contains "parent" extensions to check for. Should be fairly doable,
> > > I'll whip something up on top of that...
> >
> > Nice, and thanks for the review.
>
> > Should I wait for your refactor to be merged before pushing this one?
>
> I don't know. I think that you should continue on with your series here,
> and whichever goes in second gets rebased on top of the other.
> I don't think it makes material difference to review of this patchset as
> to whether you rebase on top of what I'm working on, so I wouldn't
> bother until it gets merged.
>
> Rather hacky, had less time than expected this morning:
> https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-extensions-strings-supersets
> Clearly there's issues with looping to RISCV_ISA_MAX_SUPERSETS & I just
> repurposed Zicsr for the sake of testing something in the time I had.
>
> Evan, at a high level, does that look more elegant to you, or have I made
> things worse?
>

I see what you're going for at least. It's unfortunate that when
someone bumps up RISCV_ISA_MAX_SUPERSETS it squares the whole array.
Another way to go might be to define the elements in a separate array,
like:

unsigned int riscv_zks_exts[] = {
       RISCV_ISA_EXT_ZBKB,
       RISCV_ISA_EXT_ZBKC,
       ....
};

then the macro entry looks like:

SET_ISA_EXT_MAP_MULTI("zks", riscv_zks_exts),

where the SET_ISA_EXT_MAP_MULTI() could use ARRAY_SIZE() to stash both
the pointer to the array and the number of elements.

-Evan

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Conor Dooley]

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

On Wed, Jun 28, 2023 at 10:18:34AM -0700, Evan Green wrote:
> On Wed, Jun 28, 2023 at 4:10 AM Conor Dooley <conor.dooley@microchip.com> wrote:
> >
> > On Wed, Jun 28, 2023 at 12:01:11PM +0200, Samuel Ortiz wrote:
> > > On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > > > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> >
> > > > > It would be nice to consolidate the ones together that search for a
> > > > > single string and set multiple bits, though I don't have any super
> > > > > elegant ideas for how off the top of my head.
> > > >
> > > > I've got a refactor of this code in progress, dropping all of these
> > > > copy-paste in place of a loop. It certainly looks more elegant than
> > > > this, but it will fall over a bit for these "one string matches many
> > > > extensions" cases. See here:
> > > > https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> > > > My immediate thought is to add another element to riscv_isa_ext_data,
> > > > that contains "parent" extensions to check for. Should be fairly doable,
> > > > I'll whip something up on top of that...
> > >
> > > Nice, and thanks for the review.
> >
> > > Should I wait for your refactor to be merged before pushing this one?
> >
> > I don't know. I think that you should continue on with your series here,
> > and whichever goes in second gets rebased on top of the other.
> > I don't think it makes material difference to review of this patchset as
> > to whether you rebase on top of what I'm working on, so I wouldn't
> > bother until it gets merged.
> >
> > Rather hacky, had less time than expected this morning:
> > https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-extensions-strings-supersets
> > Clearly there's issues with looping to RISCV_ISA_MAX_SUPERSETS & I just
> > repurposed Zicsr for the sake of testing something in the time I had.
> >
> > Evan, at a high level, does that look more elegant to you, or have I made
> > things worse?
> >
> 
> I see what you're going for at least. It's unfortunate that when
> someone bumps up RISCV_ISA_MAX_SUPERSETS it squares the whole array.
> Another way to go might be to define the elements in a separate array,
> like:
> 
> unsigned int riscv_zks_exts[] = {
>        RISCV_ISA_EXT_ZBKB,
>        RISCV_ISA_EXT_ZBKC,
>        ....
> };
> 
> then the macro entry looks like:
> 
> SET_ISA_EXT_MAP_MULTI("zks", riscv_zks_exts),
> 
> where the SET_ISA_EXT_MAP_MULTI() could use ARRAY_SIZE() to stash both
> the pointer to the array and the number of elements.

Yup, I like the sound of that. I like the variadic stuff as it'd not
require defining a bunch of sub-arrays of supersets. I guess if it grows
too badly, we can just dump it off into another file or w/e.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

[Conor Dooley]

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

On Wed, Jun 28, 2023 at 06:24:40PM +0100, Conor Dooley wrote:
> On Wed, Jun 28, 2023 at 10:18:34AM -0700, Evan Green wrote:
> > On Wed, Jun 28, 2023 at 4:10 AM Conor Dooley <conor.dooley@microchip.com> wrote:
> > >
> > > On Wed, Jun 28, 2023 at 12:01:11PM +0200, Samuel Ortiz wrote:
> > > > On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > > > > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > > > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@rivosinc.com> wrote:
> > >
> > > > > > It would be nice to consolidate the ones together that search for a
> > > > > > single string and set multiple bits, though I don't have any super
> > > > > > elegant ideas for how off the top of my head.
> > > > >
> > > > > I've got a refactor of this code in progress, dropping all of these
> > > > > copy-paste in place of a loop. It certainly looks more elegant than
> > > > > this, but it will fall over a bit for these "one string matches many
> > > > > extensions" cases. See here:
> > > > > https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> > > > > My immediate thought is to add another element to riscv_isa_ext_data,
> > > > > that contains "parent" extensions to check for. Should be fairly doable,
> > > > > I'll whip something up on top of that...
> > > >
> > > > Nice, and thanks for the review.
> > >
> > > > Should I wait for your refactor to be merged before pushing this one?
> > >
> > > I don't know. I think that you should continue on with your series here,
> > > and whichever goes in second gets rebased on top of the other.
> > > I don't think it makes material difference to review of this patchset as
> > > to whether you rebase on top of what I'm working on, so I wouldn't
> > > bother until it gets merged.
> > >
> > > Rather hacky, had less time than expected this morning:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-extensions-strings-supersets
> > > Clearly there's issues with looping to RISCV_ISA_MAX_SUPERSETS & I just
> > > repurposed Zicsr for the sake of testing something in the time I had.
> > >
> > > Evan, at a high level, does that look more elegant to you, or have I made
> > > things worse?
> > >
> > 
> > I see what you're going for at least. It's unfortunate that when
> > someone bumps up RISCV_ISA_MAX_SUPERSETS it squares the whole array.
> > Another way to go might be to define the elements in a separate array,
> > like:
> > 
> > unsigned int riscv_zks_exts[] = {
> >        RISCV_ISA_EXT_ZBKB,
> >        RISCV_ISA_EXT_ZBKC,
> >        ....
> > };
> > 
> > then the macro entry looks like:
> > 
> > SET_ISA_EXT_MAP_MULTI("zks", riscv_zks_exts),
> > 
> > where the SET_ISA_EXT_MAP_MULTI() could use ARRAY_SIZE() to stash both
> > the pointer to the array and the number of elements.
> 
> Yup, I like the sound of that. I like the variadic stuff as it'd not
> require defining a bunch of sub-arrays of supersets. I guess if it grows
> too badly, we can just dump it off into another file or w/e.

Also, I realised the other day that I had a bug in my series - I was using
"name" to read the property, not "property", which is what required the
extra "supersets" property.
The simplest thing to do actually seems to be to expand the "property"
member to an array of strings named "properties", rather than
introducing a "supersets" or similar.

Perhaps I am forgetting a good reason for why I had it split, but I'll
give it a whirl and see what I think...

Cheers,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]