public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode
@ 2025-01-13  8:36 Yunhui Cui
  2025-01-13  8:36 ` [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size Yunhui Cui
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Yunhui Cui @ 2025-01-13  8:36 UTC (permalink / raw)
  To: ajones, alexghiti, andybnac, aou, charlie, cleger, conor.dooley,
	conor, corbet, cuiyunhui, evan, jesse, linux-doc, linux-kernel,
	linux-kselftest, linux-riscv, palmer, paul.walmsley,
	samuel.holland, shuah

Enabling cbo.clean and cbo.flush in user mode makes it more
convenient to manage the cache state and achieve better performance.

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 arch/riscv/kernel/cpufeature.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index c0916ed318c2..60d180b98f52 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -30,6 +30,7 @@
 #define NUM_ALPHA_EXTS ('z' - 'a' + 1)
 
 static bool any_cpu_has_zicboz;
+static bool any_cpu_has_zicbom;
 
 unsigned long elf_hwcap __read_mostly;
 
@@ -87,6 +88,8 @@ static int riscv_ext_zicbom_validate(const struct riscv_isa_ext_data *data,
 		pr_err("Zicbom disabled as cbom-block-size present, but is not a power-of-2\n");
 		return -EINVAL;
 	}
+
+	any_cpu_has_zicbom = true;
 	return 0;
 }
 
@@ -944,6 +947,11 @@ void __init riscv_user_isa_enable(void)
 		current->thread.envcfg |= ENVCFG_CBZE;
 	else if (any_cpu_has_zicboz)
 		pr_warn("Zicboz disabled as it is unavailable on some harts\n");
+
+	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOM))
+		current->thread.envcfg |= ENVCFG_CBCFE;
+	else if (any_cpu_has_zicbom)
+		pr_warn("Zicbom disabled as it is unavailable on some harts\n");
 }
 
 #ifdef CONFIG_RISCV_ALTERNATIVE
-- 
2.39.2


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

* [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size
  2025-01-13  8:36 [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode Yunhui Cui
@ 2025-01-13  8:36 ` Yunhui Cui
  2025-01-13  9:07   ` Andrew Jones
  2025-01-13  8:36 ` [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests Yunhui Cui
  2025-01-13  8:53 ` [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode Andrew Jones
  2 siblings, 1 reply; 10+ messages in thread
From: Yunhui Cui @ 2025-01-13  8:36 UTC (permalink / raw)
  To: ajones, alexghiti, andybnac, aou, charlie, cleger, conor.dooley,
	conor, corbet, cuiyunhui, evan, jesse, linux-doc, linux-kernel,
	linux-kselftest, linux-riscv, palmer, paul.walmsley,
	samuel.holland, shuah

Expose Zicbom through hwprobe and also provide a key to extract its
respective block size.

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 Documentation/arch/riscv/hwprobe.rst  | 6 ++++++
 arch/riscv/include/asm/hwprobe.h      | 2 +-
 arch/riscv/include/uapi/asm/hwprobe.h | 2 ++
 arch/riscv/kernel/sys_hwprobe.c       | 6 ++++++
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index 955fbcd19ce9..7a47cbdbcf8e 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -94,6 +94,9 @@ The following keys are defined:
   * :c:macro:`RISCV_HWPROBE_EXT_ZICBOZ`: The Zicboz extension is supported, as
        ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
 
+  * :c:macro:`RISCV_HWPROBE_EXT_ZICBOM`: The Zicbom extension is supported, as
+       ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
+
   * :c:macro:`RISCV_HWPROBE_EXT_ZBC` The Zbc extension is supported, as defined
        in version 1.0 of the Bit-Manipulation ISA extensions.
 
@@ -273,6 +276,9 @@ The following keys are defined:
 * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
   represents the size of the Zicboz block in bytes.
 
+* :c:macro:`RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE`: An unsigned int which
+  represents the size of the Zicbom block in bytes.
+
 * :c:macro:`RISCV_HWPROBE_KEY_HIGHEST_VIRT_ADDRESS`: An unsigned long which
   represent the highest userspace virtual address usable.
 
diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
index 1ce1df6d0ff3..89379f9a2e6e 100644
--- a/arch/riscv/include/asm/hwprobe.h
+++ b/arch/riscv/include/asm/hwprobe.h
@@ -8,7 +8,7 @@
 
 #include <uapi/asm/hwprobe.h>
 
-#define RISCV_HWPROBE_MAX_KEY 10
+#define RISCV_HWPROBE_MAX_KEY 11
 
 static inline bool riscv_hwprobe_key_is_valid(__s64 key)
 {
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 3af142b99f77..892dd71a3793 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -73,6 +73,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZCMOP		(1ULL << 47)
 #define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 48)
 #define		RISCV_HWPROBE_EXT_SUPM		(1ULL << 49)
+#define		RISCV_HWPROBE_EXT_ZICBOM	(1ULL << 50)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
 #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
@@ -90,6 +91,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_MISALIGNED_SCALAR_FAST		3
 #define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED	4
 #define RISCV_HWPROBE_KEY_MISALIGNED_VECTOR_PERF	10
+#define RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE	11
 #define		RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN		0
 #define		RISCV_HWPROBE_MISALIGNED_VECTOR_SLOW		2
 #define		RISCV_HWPROBE_MISALIGNED_VECTOR_FAST		3
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index cb93adfffc48..affcc3e58df9 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -107,6 +107,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 		EXT_KEY(ZCB);
 		EXT_KEY(ZCMOP);
 		EXT_KEY(ZICBOZ);
+		EXT_KEY(ZICBOM);
 		EXT_KEY(ZICOND);
 		EXT_KEY(ZIHINTNTL);
 		EXT_KEY(ZIHINTPAUSE);
@@ -278,6 +279,11 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
 		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
 			pair->value = riscv_cboz_block_size;
 		break;
+	case RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE:
+		pair->value = 0;
+		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOM))
+			pair->value = riscv_cbom_block_size;
+		break;
 	case RISCV_HWPROBE_KEY_HIGHEST_VIRT_ADDRESS:
 		pair->value = user_max_virt_addr();
 		break;
-- 
2.39.2


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

* [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests
  2025-01-13  8:36 [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode Yunhui Cui
  2025-01-13  8:36 ` [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size Yunhui Cui
@ 2025-01-13  8:36 ` Yunhui Cui
  2025-01-13  9:18   ` Andrew Jones
  2025-01-13  8:53 ` [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode Andrew Jones
  2 siblings, 1 reply; 10+ messages in thread
From: Yunhui Cui @ 2025-01-13  8:36 UTC (permalink / raw)
  To: ajones, alexghiti, andybnac, aou, charlie, cleger, conor.dooley,
	conor, corbet, cuiyunhui, evan, jesse, linux-doc, linux-kernel,
	linux-kselftest, linux-riscv, palmer, paul.walmsley,
	samuel.holland, shuah

Add test for Zicbom and its block size into CBO tests, when
Zicbom is present, test that cbo.clean/flush may be issued and works.
As the software can't verify the clean/flush functions, we just judged
that cbo.clean/flush isn't executed illegally.

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 tools/testing/selftests/riscv/hwprobe/cbo.c | 49 ++++++++++++++++++---
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
index a40541bb7c7d..b63e23f95e08 100644
--- a/tools/testing/selftests/riscv/hwprobe/cbo.c
+++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
@@ -81,6 +81,30 @@ static bool is_power_of_2(__u64 n)
 	return n != 0 && (n & (n - 1)) == 0;
 }
 
+static void test_zicbom(void *arg)
+{
+	struct riscv_hwprobe pair = {
+		.key = RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE,
+	};
+	cpu_set_t *cpus = (cpu_set_t *)arg;
+	__u64 block_size;
+	long rc;
+
+	rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)cpus, 0);
+	block_size = pair.value;
+	ksft_test_result(rc == 0 && pair.key == RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE &&
+			 is_power_of_2(block_size), "Zicbom block size\n");
+	ksft_print_msg("Zicbom block size: %llu\n", block_size);
+
+	illegal_insn = false;
+	cbo_clean(&mem[block_size]);
+	ksft_test_result(!illegal_insn, "cbo.clean\n");
+
+	illegal_insn = false;
+	cbo_flush(&mem[block_size]);
+	ksft_test_result(!illegal_insn, "cbo.flush\n");
+}
+
 static void test_zicboz(void *arg)
 {
 	struct riscv_hwprobe pair = {
@@ -129,7 +153,7 @@ static void test_zicboz(void *arg)
 	ksft_test_result_pass("cbo.zero check\n");
 }
 
-static void check_no_zicboz_cpus(cpu_set_t *cpus)
+static void check_no_zicbo_cpus(cpu_set_t *cpus, __u64 cbo)
 {
 	struct riscv_hwprobe pair = {
 		.key = RISCV_HWPROBE_KEY_IMA_EXT_0,
@@ -137,6 +161,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
 	cpu_set_t one_cpu;
 	int i = 0, c = 0;
 	long rc;
+	char *cbostr;
 
 	while (i++ < CPU_COUNT(cpus)) {
 		while (!CPU_ISSET(c, cpus))
@@ -148,10 +173,13 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
 		rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)&one_cpu, 0);
 		assert(rc == 0 && pair.key == RISCV_HWPROBE_KEY_IMA_EXT_0);
 
-		if (pair.value & RISCV_HWPROBE_EXT_ZICBOZ)
-			ksft_exit_fail_msg("Zicboz is only present on a subset of harts.\n"
-					   "Use taskset to select a set of harts where Zicboz\n"
-					   "presence (present or not) is consistent for each hart\n");
+		cbostr = cbo == RISCV_HWPROBE_EXT_ZICBOZ ? "Zicboz" : "Zicbom";
+
+		if (pair.value & cbo)
+			ksft_exit_fail_msg("%s is only present on a subset of harts.\n"
+					   "Use taskset to select a set of harts where %s\n"
+					   "presence (present or not) is consistent for each hart\n",
+					   cbostr, cbostr);
 		++c;
 	}
 }
@@ -159,6 +187,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
 enum {
 	TEST_ZICBOZ,
 	TEST_NO_ZICBOZ,
+	TEST_ZICBOM,
 	TEST_NO_ZICBOM,
 };
 
@@ -169,6 +198,7 @@ static struct test_info {
 } tests[] = {
 	[TEST_ZICBOZ]		= { .nr_tests = 3, test_zicboz },
 	[TEST_NO_ZICBOZ]	= { .nr_tests = 1, test_no_zicboz },
+	[TEST_ZICBOM]		= { .nr_tests = 3, test_zicbom },
 	[TEST_NO_ZICBOM]	= { .nr_tests = 3, test_no_zicbom },
 };
 
@@ -206,7 +236,14 @@ int main(int argc, char **argv)
 		tests[TEST_ZICBOZ].enabled = true;
 		tests[TEST_NO_ZICBOZ].enabled = false;
 	} else {
-		check_no_zicboz_cpus(&cpus);
+		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOZ);
+	}
+
+	if (pair.value & RISCV_HWPROBE_EXT_ZICBOM) {
+		tests[TEST_ZICBOM].enabled = true;
+		tests[TEST_NO_ZICBOM].enabled = false;
+	} else {
+		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOM);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(tests); ++i)
-- 
2.39.2


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

* Re: [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode
  2025-01-13  8:36 [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode Yunhui Cui
  2025-01-13  8:36 ` [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size Yunhui Cui
  2025-01-13  8:36 ` [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests Yunhui Cui
@ 2025-01-13  8:53 ` Andrew Jones
  2 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2025-01-13  8:53 UTC (permalink / raw)
  To: Yunhui Cui
  Cc: alexghiti, andybnac, aou, charlie, cleger, conor.dooley, conor,
	corbet, evan, jesse, linux-doc, linux-kernel, linux-kselftest,
	linux-riscv, palmer, paul.walmsley, samuel.holland, shuah

On Mon, Jan 13, 2025 at 04:36:33PM +0800, Yunhui Cui wrote:
> Enabling cbo.clean and cbo.flush in user mode makes it more
> convenient to manage the cache state and achieve better performance.
> 
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  arch/riscv/kernel/cpufeature.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index c0916ed318c2..60d180b98f52 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -30,6 +30,7 @@
>  #define NUM_ALPHA_EXTS ('z' - 'a' + 1)
>  
>  static bool any_cpu_has_zicboz;
> +static bool any_cpu_has_zicbom;
>  
>  unsigned long elf_hwcap __read_mostly;
>  
> @@ -87,6 +88,8 @@ static int riscv_ext_zicbom_validate(const struct riscv_isa_ext_data *data,
>  		pr_err("Zicbom disabled as cbom-block-size present, but is not a power-of-2\n");
>  		return -EINVAL;
>  	}
> +
> +	any_cpu_has_zicbom = true;
>  	return 0;
>  }
>  
> @@ -944,6 +947,11 @@ void __init riscv_user_isa_enable(void)
>  		current->thread.envcfg |= ENVCFG_CBZE;
>  	else if (any_cpu_has_zicboz)
>  		pr_warn("Zicboz disabled as it is unavailable on some harts\n");
> +
> +	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOM))
> +		current->thread.envcfg |= ENVCFG_CBCFE;
> +	else if (any_cpu_has_zicbom)
> +		pr_warn("Zicbom disabled as it is unavailable on some harts\n");
>  }
>  
>  #ifdef CONFIG_RISCV_ALTERNATIVE
> -- 
> 2.39.2
>

Please use a cover letter with a changelog or at least changelogs under
the '---' of each patch.

Thanks,
drew

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

* Re: [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size
  2025-01-13  8:36 ` [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size Yunhui Cui
@ 2025-01-13  9:07   ` Andrew Jones
  2025-01-13 11:29     ` [External] " yunhui cui
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Jones @ 2025-01-13  9:07 UTC (permalink / raw)
  To: Yunhui Cui
  Cc: alexghiti, andybnac, aou, charlie, cleger, conor.dooley, conor,
	corbet, evan, jesse, linux-doc, linux-kernel, linux-kselftest,
	linux-riscv, palmer, paul.walmsley, samuel.holland, shuah

On Mon, Jan 13, 2025 at 04:36:34PM +0800, Yunhui Cui wrote:
> Expose Zicbom through hwprobe and also provide a key to extract its
> respective block size.
> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  Documentation/arch/riscv/hwprobe.rst  | 6 ++++++
>  arch/riscv/include/asm/hwprobe.h      | 2 +-
>  arch/riscv/include/uapi/asm/hwprobe.h | 2 ++
>  arch/riscv/kernel/sys_hwprobe.c       | 6 ++++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> index 955fbcd19ce9..7a47cbdbcf8e 100644
> --- a/Documentation/arch/riscv/hwprobe.rst
> +++ b/Documentation/arch/riscv/hwprobe.rst
> @@ -94,6 +94,9 @@ The following keys are defined:
>    * :c:macro:`RISCV_HWPROBE_EXT_ZICBOZ`: The Zicboz extension is supported, as
>         ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
>  
> +  * :c:macro:`RISCV_HWPROBE_EXT_ZICBOM`: The Zicbom extension is supported, as
> +       ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> +

This should come after RISCV_HWPROBE_EXT_SUPM since this document has the
defines sorted in the order in which they are introduced (although I
personally wouldn't mind if we ordered them alphabetically instead)

>    * :c:macro:`RISCV_HWPROBE_EXT_ZBC` The Zbc extension is supported, as defined
>         in version 1.0 of the Bit-Manipulation ISA extensions.
>  
> @@ -273,6 +276,9 @@ The following keys are defined:
>  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
>    represents the size of the Zicboz block in bytes.
>  
> +* :c:macro:`RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE`: An unsigned int which
> +  represents the size of the Zicbom block in bytes.
> +

Should be moved below RISCV_HWPROBE_KEY_TIME_CSR_FREQ

>  * :c:macro:`RISCV_HWPROBE_KEY_HIGHEST_VIRT_ADDRESS`: An unsigned long which
>    represent the highest userspace virtual address usable.
>  
> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> index 1ce1df6d0ff3..89379f9a2e6e 100644
> --- a/arch/riscv/include/asm/hwprobe.h
> +++ b/arch/riscv/include/asm/hwprobe.h
> @@ -8,7 +8,7 @@
>  
>  #include <uapi/asm/hwprobe.h>
>  
> -#define RISCV_HWPROBE_MAX_KEY 10
> +#define RISCV_HWPROBE_MAX_KEY 11
>  
>  static inline bool riscv_hwprobe_key_is_valid(__s64 key)
>  {
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 3af142b99f77..892dd71a3793 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -73,6 +73,7 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_EXT_ZCMOP		(1ULL << 47)
>  #define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 48)
>  #define		RISCV_HWPROBE_EXT_SUPM		(1ULL << 49)
> +#define		RISCV_HWPROBE_EXT_ZICBOM	(1ULL << 50)
>  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
>  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> @@ -90,6 +91,7 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_MISALIGNED_SCALAR_FAST		3
>  #define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED	4
>  #define RISCV_HWPROBE_KEY_MISALIGNED_VECTOR_PERF	10
> +#define RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE	11

Move below the bit defines of RISCV_HWPROBE_KEY_MISALIGNED_VECTOR_PERF
(notice how its bit defines are indented, indicating they belong to it)

>  #define		RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN		0
>  #define		RISCV_HWPROBE_MISALIGNED_VECTOR_SLOW		2
>  #define		RISCV_HWPROBE_MISALIGNED_VECTOR_FAST		3
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index cb93adfffc48..affcc3e58df9 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -107,6 +107,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  		EXT_KEY(ZCB);
>  		EXT_KEY(ZCMOP);
>  		EXT_KEY(ZICBOZ);
> +		EXT_KEY(ZICBOM);

This list is in alphabetical order, which means ZICBOM should come before
ZICBOZ.

>  		EXT_KEY(ZICOND);
>  		EXT_KEY(ZIHINTNTL);
>  		EXT_KEY(ZIHINTPAUSE);
> @@ -278,6 +279,11 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>  		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
>  			pair->value = riscv_cboz_block_size;
>  		break;
> +	case RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE:
> +		pair->value = 0;
> +		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOM))
> +			pair->value = riscv_cbom_block_size;
> +		break;
>  	case RISCV_HWPROBE_KEY_HIGHEST_VIRT_ADDRESS:
>  		pair->value = user_max_virt_addr();
>  		break;
> -- 
> 2.39.2
>

Thanks,
drew

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

* Re: [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests
  2025-01-13  8:36 ` [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests Yunhui Cui
@ 2025-01-13  9:18   ` Andrew Jones
  2025-01-13 11:49     ` [External] " yunhui cui
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Jones @ 2025-01-13  9:18 UTC (permalink / raw)
  To: Yunhui Cui
  Cc: alexghiti, andybnac, aou, charlie, cleger, conor.dooley, conor,
	corbet, evan, jesse, linux-doc, linux-kernel, linux-kselftest,
	linux-riscv, palmer, paul.walmsley, samuel.holland, shuah

On Mon, Jan 13, 2025 at 04:36:35PM +0800, Yunhui Cui wrote:
> Add test for Zicbom and its block size into CBO tests, when
> Zicbom is present, test that cbo.clean/flush may be issued and works.
> As the software can't verify the clean/flush functions, we just judged
> that cbo.clean/flush isn't executed illegally.
> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  tools/testing/selftests/riscv/hwprobe/cbo.c | 49 ++++++++++++++++++---
>  1 file changed, 43 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
> index a40541bb7c7d..b63e23f95e08 100644
> --- a/tools/testing/selftests/riscv/hwprobe/cbo.c
> +++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
> @@ -81,6 +81,30 @@ static bool is_power_of_2(__u64 n)
>  	return n != 0 && (n & (n - 1)) == 0;
>  }
>  
> +static void test_zicbom(void *arg)
> +{
> +	struct riscv_hwprobe pair = {
> +		.key = RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE,
> +	};
> +	cpu_set_t *cpus = (cpu_set_t *)arg;
> +	__u64 block_size;
> +	long rc;
> +
> +	rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)cpus, 0);
> +	block_size = pair.value;
> +	ksft_test_result(rc == 0 && pair.key == RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE &&
> +			 is_power_of_2(block_size), "Zicbom block size\n");
> +	ksft_print_msg("Zicbom block size: %llu\n", block_size);
> +
> +	illegal_insn = false;
> +	cbo_clean(&mem[block_size]);
> +	ksft_test_result(!illegal_insn, "cbo.clean\n");
> +
> +	illegal_insn = false;
> +	cbo_flush(&mem[block_size]);
> +	ksft_test_result(!illegal_insn, "cbo.flush\n");
> +}
> +
>  static void test_zicboz(void *arg)
>  {
>  	struct riscv_hwprobe pair = {
> @@ -129,7 +153,7 @@ static void test_zicboz(void *arg)
>  	ksft_test_result_pass("cbo.zero check\n");
>  }
>  
> -static void check_no_zicboz_cpus(cpu_set_t *cpus)
> +static void check_no_zicbo_cpus(cpu_set_t *cpus, __u64 cbo)
>  {
>  	struct riscv_hwprobe pair = {
>  		.key = RISCV_HWPROBE_KEY_IMA_EXT_0,
> @@ -137,6 +161,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
>  	cpu_set_t one_cpu;
>  	int i = 0, c = 0;
>  	long rc;
> +	char *cbostr;
>  
>  	while (i++ < CPU_COUNT(cpus)) {
>  		while (!CPU_ISSET(c, cpus))
> @@ -148,10 +173,13 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
>  		rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)&one_cpu, 0);
>  		assert(rc == 0 && pair.key == RISCV_HWPROBE_KEY_IMA_EXT_0);
>  
> -		if (pair.value & RISCV_HWPROBE_EXT_ZICBOZ)
> -			ksft_exit_fail_msg("Zicboz is only present on a subset of harts.\n"
> -					   "Use taskset to select a set of harts where Zicboz\n"
> -					   "presence (present or not) is consistent for each hart\n");
> +		cbostr = cbo == RISCV_HWPROBE_EXT_ZICBOZ ? "Zicboz" : "Zicbom";
> +
> +		if (pair.value & cbo)
> +			ksft_exit_fail_msg("%s is only present on a subset of harts.\n"
> +					   "Use taskset to select a set of harts where %s\n"
> +					   "presence (present or not) is consistent for each hart\n",
> +					   cbostr, cbostr);
>  		++c;
>  	}
>  }
> @@ -159,6 +187,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
>  enum {
>  	TEST_ZICBOZ,
>  	TEST_NO_ZICBOZ,
> +	TEST_ZICBOM,
>  	TEST_NO_ZICBOM,
>  };
>  
> @@ -169,6 +198,7 @@ static struct test_info {
>  } tests[] = {
>  	[TEST_ZICBOZ]		= { .nr_tests = 3, test_zicboz },
>  	[TEST_NO_ZICBOZ]	= { .nr_tests = 1, test_no_zicboz },
> +	[TEST_ZICBOM]		= { .nr_tests = 3, test_zicbom },
>  	[TEST_NO_ZICBOM]	= { .nr_tests = 3, test_no_zicbom },
>  };
>  
> @@ -206,7 +236,14 @@ int main(int argc, char **argv)
>  		tests[TEST_ZICBOZ].enabled = true;
>  		tests[TEST_NO_ZICBOZ].enabled = false;
>  	} else {
> -		check_no_zicboz_cpus(&cpus);
> +		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOZ);
> +	}
> +
> +	if (pair.value & RISCV_HWPROBE_EXT_ZICBOM) {
> +		tests[TEST_ZICBOM].enabled = true;
> +		tests[TEST_NO_ZICBOM].enabled = false;
> +	} else {
> +		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOM);
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(tests); ++i)
> -- 
> 2.39.2
>

The test_no_zicbom() test needs to have the illegal instruction SIGILL
test for cbo.inval moved out into its own test. So, even when we have
zicbom we still test that cbo.inval generates a SIGILL.

Thanks,
drew

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

* Re: [External] Re: [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size
  2025-01-13  9:07   ` Andrew Jones
@ 2025-01-13 11:29     ` yunhui cui
  2025-01-13 12:41       ` Andrew Jones
  0 siblings, 1 reply; 10+ messages in thread
From: yunhui cui @ 2025-01-13 11:29 UTC (permalink / raw)
  To: Andrew Jones
  Cc: alexghiti, andybnac, aou, charlie, cleger, conor.dooley, conor,
	corbet, evan, jesse, linux-doc, linux-kernel, linux-kselftest,
	linux-riscv, palmer, paul.walmsley, samuel.holland, shuah

Hi drew,

On Mon, Jan 13, 2025 at 5:07 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Jan 13, 2025 at 04:36:34PM +0800, Yunhui Cui wrote:
> > Expose Zicbom through hwprobe and also provide a key to extract its
> > respective block size.
> >
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> >  Documentation/arch/riscv/hwprobe.rst  | 6 ++++++
> >  arch/riscv/include/asm/hwprobe.h      | 2 +-
> >  arch/riscv/include/uapi/asm/hwprobe.h | 2 ++
> >  arch/riscv/kernel/sys_hwprobe.c       | 6 ++++++
> >  4 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > index 955fbcd19ce9..7a47cbdbcf8e 100644
> > --- a/Documentation/arch/riscv/hwprobe.rst
> > +++ b/Documentation/arch/riscv/hwprobe.rst
> > @@ -94,6 +94,9 @@ The following keys are defined:
> >    * :c:macro:`RISCV_HWPROBE_EXT_ZICBOZ`: The Zicboz extension is supported, as
> >         ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> >
> > +  * :c:macro:`RISCV_HWPROBE_EXT_ZICBOM`: The Zicbom extension is supported, as
> > +       ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> > +
>
> This should come after RISCV_HWPROBE_EXT_SUPM since this document has the
> defines sorted in the order in which they are introduced (although I
> personally wouldn't mind if we ordered them alphabetically instead)
OK

>
> >    * :c:macro:`RISCV_HWPROBE_EXT_ZBC` The Zbc extension is supported, as defined
> >         in version 1.0 of the Bit-Manipulation ISA extensions.
> >
> > @@ -273,6 +276,9 @@ The following keys are defined:
> >  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
> >    represents the size of the Zicboz block in bytes.
> >
> > +* :c:macro:`RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE`: An unsigned int which
> > +  represents the size of the Zicbom block in bytes.
> > +
>
> Should be moved below RISCV_HWPROBE_KEY_TIME_CSR_FREQ

Why should it be moved below RISCV_HWPROBE_KEY_TIME_CSR_FREQ?


> >  * :c:macro:`RISCV_HWPROBE_KEY_HIGHEST_VIRT_ADDRESS`: An unsigned long which
> >    represent the highest userspace virtual address usable.
> >
> > diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> > index 1ce1df6d0ff3..89379f9a2e6e 100644
> > --- a/arch/riscv/include/asm/hwprobe.h
> > +++ b/arch/riscv/include/asm/hwprobe.h
> > @@ -8,7 +8,7 @@
> >
> >  #include <uapi/asm/hwprobe.h>
> >
> > -#define RISCV_HWPROBE_MAX_KEY 10
> > +#define RISCV_HWPROBE_MAX_KEY 11
> >
> >  static inline bool riscv_hwprobe_key_is_valid(__s64 key)
> >  {
> > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > index 3af142b99f77..892dd71a3793 100644
> > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > @@ -73,6 +73,7 @@ struct riscv_hwprobe {
> >  #define              RISCV_HWPROBE_EXT_ZCMOP         (1ULL << 47)
> >  #define              RISCV_HWPROBE_EXT_ZAWRS         (1ULL << 48)
> >  #define              RISCV_HWPROBE_EXT_SUPM          (1ULL << 49)
> > +#define              RISCV_HWPROBE_EXT_ZICBOM        (1ULL << 50)
> >  #define RISCV_HWPROBE_KEY_CPUPERF_0  5
> >  #define              RISCV_HWPROBE_MISALIGNED_UNKNOWN        (0 << 0)
> >  #define              RISCV_HWPROBE_MISALIGNED_EMULATED       (1 << 0)
> > @@ -90,6 +91,7 @@ struct riscv_hwprobe {
> >  #define              RISCV_HWPROBE_MISALIGNED_SCALAR_FAST            3
> >  #define              RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED     4
> >  #define RISCV_HWPROBE_KEY_MISALIGNED_VECTOR_PERF     10
> > +#define RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE  11
>
> Move below the bit defines of RISCV_HWPROBE_KEY_MISALIGNED_VECTOR_PERF
> (notice how its bit defines are indented, indicating they belong to it)
OK

>
> >  #define              RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN         0
> >  #define              RISCV_HWPROBE_MISALIGNED_VECTOR_SLOW            2
> >  #define              RISCV_HWPROBE_MISALIGNED_VECTOR_FAST            3
> > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > index cb93adfffc48..affcc3e58df9 100644
> > --- a/arch/riscv/kernel/sys_hwprobe.c
> > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > @@ -107,6 +107,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
> >               EXT_KEY(ZCB);
> >               EXT_KEY(ZCMOP);
> >               EXT_KEY(ZICBOZ);
> > +             EXT_KEY(ZICBOM);
>
> This list is in alphabetical order, which means ZICBOM should come before
> ZICBOZ.
OK

>
> >               EXT_KEY(ZICOND);
> >               EXT_KEY(ZIHINTNTL);
> >               EXT_KEY(ZIHINTPAUSE);
> > @@ -278,6 +279,11 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
> >               if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
> >                       pair->value = riscv_cboz_block_size;
> >               break;
> > +     case RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE:
> > +             pair->value = 0;
> > +             if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOM))
> > +                     pair->value = riscv_cbom_block_size;
> > +             break;
> >       case RISCV_HWPROBE_KEY_HIGHEST_VIRT_ADDRESS:
> >               pair->value = user_max_virt_addr();
> >               break;
> > --
> > 2.39.2
> >
>
> Thanks,
> drew

Thanks,
Yunhui

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

* Re: [External] Re: [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests
  2025-01-13  9:18   ` Andrew Jones
@ 2025-01-13 11:49     ` yunhui cui
  2025-01-13 12:42       ` Andrew Jones
  0 siblings, 1 reply; 10+ messages in thread
From: yunhui cui @ 2025-01-13 11:49 UTC (permalink / raw)
  To: Andrew Jones
  Cc: alexghiti, andybnac, aou, charlie, cleger, conor.dooley, conor,
	corbet, evan, jesse, linux-doc, linux-kernel, linux-kselftest,
	linux-riscv, palmer, paul.walmsley, samuel.holland, shuah

Hi drew,

On Mon, Jan 13, 2025 at 5:18 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Jan 13, 2025 at 04:36:35PM +0800, Yunhui Cui wrote:
> > Add test for Zicbom and its block size into CBO tests, when
> > Zicbom is present, test that cbo.clean/flush may be issued and works.
> > As the software can't verify the clean/flush functions, we just judged
> > that cbo.clean/flush isn't executed illegally.
> >
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> >  tools/testing/selftests/riscv/hwprobe/cbo.c | 49 ++++++++++++++++++---
> >  1 file changed, 43 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
> > index a40541bb7c7d..b63e23f95e08 100644
> > --- a/tools/testing/selftests/riscv/hwprobe/cbo.c
> > +++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
> > @@ -81,6 +81,30 @@ static bool is_power_of_2(__u64 n)
> >       return n != 0 && (n & (n - 1)) == 0;
> >  }
> >
> > +static void test_zicbom(void *arg)
> > +{
> > +     struct riscv_hwprobe pair = {
> > +             .key = RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE,
> > +     };
> > +     cpu_set_t *cpus = (cpu_set_t *)arg;
> > +     __u64 block_size;
> > +     long rc;
> > +
> > +     rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)cpus, 0);
> > +     block_size = pair.value;
> > +     ksft_test_result(rc == 0 && pair.key == RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE &&
> > +                      is_power_of_2(block_size), "Zicbom block size\n");
> > +     ksft_print_msg("Zicbom block size: %llu\n", block_size);
> > +
> > +     illegal_insn = false;
> > +     cbo_clean(&mem[block_size]);
> > +     ksft_test_result(!illegal_insn, "cbo.clean\n");
> > +
> > +     illegal_insn = false;
> > +     cbo_flush(&mem[block_size]);
> > +     ksft_test_result(!illegal_insn, "cbo.flush\n");
> > +}
> > +
> >  static void test_zicboz(void *arg)
> >  {
> >       struct riscv_hwprobe pair = {
> > @@ -129,7 +153,7 @@ static void test_zicboz(void *arg)
> >       ksft_test_result_pass("cbo.zero check\n");
> >  }
> >
> > -static void check_no_zicboz_cpus(cpu_set_t *cpus)
> > +static void check_no_zicbo_cpus(cpu_set_t *cpus, __u64 cbo)
> >  {
> >       struct riscv_hwprobe pair = {
> >               .key = RISCV_HWPROBE_KEY_IMA_EXT_0,
> > @@ -137,6 +161,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
> >       cpu_set_t one_cpu;
> >       int i = 0, c = 0;
> >       long rc;
> > +     char *cbostr;
> >
> >       while (i++ < CPU_COUNT(cpus)) {
> >               while (!CPU_ISSET(c, cpus))
> > @@ -148,10 +173,13 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
> >               rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)&one_cpu, 0);
> >               assert(rc == 0 && pair.key == RISCV_HWPROBE_KEY_IMA_EXT_0);
> >
> > -             if (pair.value & RISCV_HWPROBE_EXT_ZICBOZ)
> > -                     ksft_exit_fail_msg("Zicboz is only present on a subset of harts.\n"
> > -                                        "Use taskset to select a set of harts where Zicboz\n"
> > -                                        "presence (present or not) is consistent for each hart\n");
> > +             cbostr = cbo == RISCV_HWPROBE_EXT_ZICBOZ ? "Zicboz" : "Zicbom";
> > +
> > +             if (pair.value & cbo)
> > +                     ksft_exit_fail_msg("%s is only present on a subset of harts.\n"
> > +                                        "Use taskset to select a set of harts where %s\n"
> > +                                        "presence (present or not) is consistent for each hart\n",
> > +                                        cbostr, cbostr);
> >               ++c;
> >       }
> >  }
> > @@ -159,6 +187,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
> >  enum {
> >       TEST_ZICBOZ,
> >       TEST_NO_ZICBOZ,
> > +     TEST_ZICBOM,
> >       TEST_NO_ZICBOM,
> >  };
> >
> > @@ -169,6 +198,7 @@ static struct test_info {
> >  } tests[] = {
> >       [TEST_ZICBOZ]           = { .nr_tests = 3, test_zicboz },
> >       [TEST_NO_ZICBOZ]        = { .nr_tests = 1, test_no_zicboz },
> > +     [TEST_ZICBOM]           = { .nr_tests = 3, test_zicbom },
> >       [TEST_NO_ZICBOM]        = { .nr_tests = 3, test_no_zicbom },
> >  };
> >
> > @@ -206,7 +236,14 @@ int main(int argc, char **argv)
> >               tests[TEST_ZICBOZ].enabled = true;
> >               tests[TEST_NO_ZICBOZ].enabled = false;
> >       } else {
> > -             check_no_zicboz_cpus(&cpus);
> > +             check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOZ);
> > +     }
> > +
> > +     if (pair.value & RISCV_HWPROBE_EXT_ZICBOM) {
> > +             tests[TEST_ZICBOM].enabled = true;
> > +             tests[TEST_NO_ZICBOM].enabled = false;
> > +     } else {
> > +             check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOM);
> >       }
> >
> >       for (i = 0; i < ARRAY_SIZE(tests); ++i)
> > --
> > 2.39.2
> >
>
> The test_no_zicbom() test needs to have the illegal instruction SIGILL
> test for cbo.inval moved out into its own test. So, even when we have
> zicbom we still test that cbo.inval generates a SIGILL.

Do you mean moving cbo_inval() into test_zicbom()? Then does
cbo_inval(&mem[0]) also need to be tested in test_no_zicbom()?



> Thanks,
> drew

Thanks,
Yunhui

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

* Re: [External] Re: [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size
  2025-01-13 11:29     ` [External] " yunhui cui
@ 2025-01-13 12:41       ` Andrew Jones
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2025-01-13 12:41 UTC (permalink / raw)
  To: yunhui cui
  Cc: alexghiti, andybnac, aou, charlie, cleger, conor.dooley, conor,
	corbet, evan, jesse, linux-doc, linux-kernel, linux-kselftest,
	linux-riscv, palmer, paul.walmsley, samuel.holland, shuah

On Mon, Jan 13, 2025 at 07:29:39PM +0800, yunhui cui wrote:
> Hi drew,
> 
> On Mon, Jan 13, 2025 at 5:07 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Mon, Jan 13, 2025 at 04:36:34PM +0800, Yunhui Cui wrote:
> > > Expose Zicbom through hwprobe and also provide a key to extract its
> > > respective block size.
> > >
> > > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > ---
> > >  Documentation/arch/riscv/hwprobe.rst  | 6 ++++++
> > >  arch/riscv/include/asm/hwprobe.h      | 2 +-
> > >  arch/riscv/include/uapi/asm/hwprobe.h | 2 ++
> > >  arch/riscv/kernel/sys_hwprobe.c       | 6 ++++++
> > >  4 files changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > > index 955fbcd19ce9..7a47cbdbcf8e 100644
> > > --- a/Documentation/arch/riscv/hwprobe.rst
> > > +++ b/Documentation/arch/riscv/hwprobe.rst
> > > @@ -94,6 +94,9 @@ The following keys are defined:
> > >    * :c:macro:`RISCV_HWPROBE_EXT_ZICBOZ`: The Zicboz extension is supported, as
> > >         ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> > >
> > > +  * :c:macro:`RISCV_HWPROBE_EXT_ZICBOM`: The Zicbom extension is supported, as
> > > +       ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> > > +
> >
> > This should come after RISCV_HWPROBE_EXT_SUPM since this document has the
> > defines sorted in the order in which they are introduced (although I
> > personally wouldn't mind if we ordered them alphabetically instead)
> OK
> 
> >
> > >    * :c:macro:`RISCV_HWPROBE_EXT_ZBC` The Zbc extension is supported, as defined
> > >         in version 1.0 of the Bit-Manipulation ISA extensions.
> > >
> > > @@ -273,6 +276,9 @@ The following keys are defined:
> > >  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
> > >    represents the size of the Zicboz block in bytes.
> > >
> > > +* :c:macro:`RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE`: An unsigned int which
> > > +  represents the size of the Zicbom block in bytes.
> > > +
> >
> > Should be moved below RISCV_HWPROBE_KEY_TIME_CSR_FREQ
> 
> Why should it be moved below RISCV_HWPROBE_KEY_TIME_CSR_FREQ?

Oh, right. It should be below RISCV_HWPROBE_KEY_MISALIGNED_VECTOR_PERF for
the same reason as above (order of introduction).

Thanks,
drew

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

* Re: [External] Re: [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests
  2025-01-13 11:49     ` [External] " yunhui cui
@ 2025-01-13 12:42       ` Andrew Jones
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2025-01-13 12:42 UTC (permalink / raw)
  To: yunhui cui
  Cc: alexghiti, andybnac, aou, charlie, cleger, conor.dooley, conor,
	corbet, evan, jesse, linux-doc, linux-kernel, linux-kselftest,
	linux-riscv, palmer, paul.walmsley, samuel.holland, shuah

On Mon, Jan 13, 2025 at 07:49:39PM +0800, yunhui cui wrote:
> Hi drew,
> 
> On Mon, Jan 13, 2025 at 5:18 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Mon, Jan 13, 2025 at 04:36:35PM +0800, Yunhui Cui wrote:
> > > Add test for Zicbom and its block size into CBO tests, when
> > > Zicbom is present, test that cbo.clean/flush may be issued and works.
> > > As the software can't verify the clean/flush functions, we just judged
> > > that cbo.clean/flush isn't executed illegally.
> > >
> > > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > ---
> > >  tools/testing/selftests/riscv/hwprobe/cbo.c | 49 ++++++++++++++++++---
> > >  1 file changed, 43 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
> > > index a40541bb7c7d..b63e23f95e08 100644
> > > --- a/tools/testing/selftests/riscv/hwprobe/cbo.c
> > > +++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
> > > @@ -81,6 +81,30 @@ static bool is_power_of_2(__u64 n)
> > >       return n != 0 && (n & (n - 1)) == 0;
> > >  }
> > >
> > > +static void test_zicbom(void *arg)
> > > +{
> > > +     struct riscv_hwprobe pair = {
> > > +             .key = RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE,
> > > +     };
> > > +     cpu_set_t *cpus = (cpu_set_t *)arg;
> > > +     __u64 block_size;
> > > +     long rc;
> > > +
> > > +     rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)cpus, 0);
> > > +     block_size = pair.value;
> > > +     ksft_test_result(rc == 0 && pair.key == RISCV_HWPROBE_KEY_ZICBOM_BLOCK_SIZE &&
> > > +                      is_power_of_2(block_size), "Zicbom block size\n");
> > > +     ksft_print_msg("Zicbom block size: %llu\n", block_size);
> > > +
> > > +     illegal_insn = false;
> > > +     cbo_clean(&mem[block_size]);
> > > +     ksft_test_result(!illegal_insn, "cbo.clean\n");
> > > +
> > > +     illegal_insn = false;
> > > +     cbo_flush(&mem[block_size]);
> > > +     ksft_test_result(!illegal_insn, "cbo.flush\n");
> > > +}
> > > +
> > >  static void test_zicboz(void *arg)
> > >  {
> > >       struct riscv_hwprobe pair = {
> > > @@ -129,7 +153,7 @@ static void test_zicboz(void *arg)
> > >       ksft_test_result_pass("cbo.zero check\n");
> > >  }
> > >
> > > -static void check_no_zicboz_cpus(cpu_set_t *cpus)
> > > +static void check_no_zicbo_cpus(cpu_set_t *cpus, __u64 cbo)
> > >  {
> > >       struct riscv_hwprobe pair = {
> > >               .key = RISCV_HWPROBE_KEY_IMA_EXT_0,
> > > @@ -137,6 +161,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
> > >       cpu_set_t one_cpu;
> > >       int i = 0, c = 0;
> > >       long rc;
> > > +     char *cbostr;
> > >
> > >       while (i++ < CPU_COUNT(cpus)) {
> > >               while (!CPU_ISSET(c, cpus))
> > > @@ -148,10 +173,13 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
> > >               rc = riscv_hwprobe(&pair, 1, sizeof(cpu_set_t), (unsigned long *)&one_cpu, 0);
> > >               assert(rc == 0 && pair.key == RISCV_HWPROBE_KEY_IMA_EXT_0);
> > >
> > > -             if (pair.value & RISCV_HWPROBE_EXT_ZICBOZ)
> > > -                     ksft_exit_fail_msg("Zicboz is only present on a subset of harts.\n"
> > > -                                        "Use taskset to select a set of harts where Zicboz\n"
> > > -                                        "presence (present or not) is consistent for each hart\n");
> > > +             cbostr = cbo == RISCV_HWPROBE_EXT_ZICBOZ ? "Zicboz" : "Zicbom";
> > > +
> > > +             if (pair.value & cbo)
> > > +                     ksft_exit_fail_msg("%s is only present on a subset of harts.\n"
> > > +                                        "Use taskset to select a set of harts where %s\n"
> > > +                                        "presence (present or not) is consistent for each hart\n",
> > > +                                        cbostr, cbostr);
> > >               ++c;
> > >       }
> > >  }
> > > @@ -159,6 +187,7 @@ static void check_no_zicboz_cpus(cpu_set_t *cpus)
> > >  enum {
> > >       TEST_ZICBOZ,
> > >       TEST_NO_ZICBOZ,
> > > +     TEST_ZICBOM,
> > >       TEST_NO_ZICBOM,
> > >  };
> > >
> > > @@ -169,6 +198,7 @@ static struct test_info {
> > >  } tests[] = {
> > >       [TEST_ZICBOZ]           = { .nr_tests = 3, test_zicboz },
> > >       [TEST_NO_ZICBOZ]        = { .nr_tests = 1, test_no_zicboz },
> > > +     [TEST_ZICBOM]           = { .nr_tests = 3, test_zicbom },
> > >       [TEST_NO_ZICBOM]        = { .nr_tests = 3, test_no_zicbom },
> > >  };
> > >
> > > @@ -206,7 +236,14 @@ int main(int argc, char **argv)
> > >               tests[TEST_ZICBOZ].enabled = true;
> > >               tests[TEST_NO_ZICBOZ].enabled = false;
> > >       } else {
> > > -             check_no_zicboz_cpus(&cpus);
> > > +             check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOZ);
> > > +     }
> > > +
> > > +     if (pair.value & RISCV_HWPROBE_EXT_ZICBOM) {
> > > +             tests[TEST_ZICBOM].enabled = true;
> > > +             tests[TEST_NO_ZICBOM].enabled = false;
> > > +     } else {
> > > +             check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOM);
> > >       }
> > >
> > >       for (i = 0; i < ARRAY_SIZE(tests); ++i)
> > > --
> > > 2.39.2
> > >
> >
> > The test_no_zicbom() test needs to have the illegal instruction SIGILL
> > test for cbo.inval moved out into its own test. So, even when we have
> > zicbom we still test that cbo.inval generates a SIGILL.
> 
> Do you mean moving cbo_inval() into test_zicbom()? Then does
> cbo_inval(&mem[0]) also need to be tested in test_no_zicbom()?

No, I'd create a new test named test_no_cbo_inval(), which should always
run regardless of zicbom/zicboz detection.

Thanks,
drew

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

end of thread, other threads:[~2025-01-13 12:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-13  8:36 [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode Yunhui Cui
2025-01-13  8:36 ` [PATCH v3 2/3] RISC-V: hwprobe: Expose Zicbom extension and its block size Yunhui Cui
2025-01-13  9:07   ` Andrew Jones
2025-01-13 11:29     ` [External] " yunhui cui
2025-01-13 12:41       ` Andrew Jones
2025-01-13  8:36 ` [PATCH v3 3/3] RISC-V: selftests: Add TEST_ZICBOM into CBO tests Yunhui Cui
2025-01-13  9:18   ` Andrew Jones
2025-01-13 11:49     ` [External] " yunhui cui
2025-01-13 12:42       ` Andrew Jones
2025-01-13  8:53 ` [PATCH v3 1/3] RISC-V: Enable cbo.clean/flush in usermode Andrew Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox