[PATCH v3 2/2] selftests/riscv: Add Zicbop prefetch test

Yao Zihong posted 2 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v3 2/2] selftests/riscv: Add Zicbop prefetch test
Posted by Yao Zihong 3 months, 3 weeks ago
Add selftest to cbo.c to verify Zicbop extension behavior.

The test checks:
- That hwprobe correctly reports Zicbop presence and block size.
- That prefetch instructions execute without exception on valid and NULL
  addresses when Zicbop is present.
- That prefetch.{i,r,w} do not trigger SIGILL even when Zicbop is absent,
  since Zicbop instructions are defined as hints.

Signed-off-by: Yao Zihong <zihong.plct@isrc.iscas.ac.cn>
---
 tools/testing/selftests/riscv/hwprobe/cbo.c | 188 +++++++++++++++++---
 1 file changed, 159 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
index 5e96ef785d0d..163228e3f2c7 100644
--- a/tools/testing/selftests/riscv/hwprobe/cbo.c
+++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
@@ -15,24 +15,30 @@
 #include <linux/compiler.h>
 #include <linux/kernel.h>
 #include <asm/ucontext.h>
+#include <getopt.h>
 
 #include "hwprobe.h"
 #include "../../kselftest.h"
 
 #define MK_CBO(fn) le32_bswap((uint32_t)(fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15)
+#define MK_PREFETCH(fn) \
+	le32_bswap(0 << 25 | (uint32_t)(fn) << 20 | 10 << 15 | 6 << 12 | 0 << 7 | 19)
 
 static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 };
 
-static bool illegal_insn;
+static bool got_fault;
 
 static void sigill_handler(int sig, siginfo_t *info, void *context)
 {
 	unsigned long *regs = (unsigned long *)&((ucontext_t *)context)->uc_mcontext;
 	uint32_t insn = *(uint32_t *)regs[0];
 
-	assert(insn == MK_CBO(regs[11]));
+	if (regs[12] == 0)
+		assert(insn == MK_CBO(regs[11]));
+	else
+		assert(insn == MK_PREFETCH(regs[11]));
 
-	illegal_insn = true;
+	got_fault = true;
 	regs[0] += 4;
 }
 
@@ -41,43 +47,75 @@ static void sigill_handler(int sig, siginfo_t *info, void *context)
 	asm volatile(								\
 	"mv	a0, %0\n"							\
 	"li	a1, %1\n"							\
+	"li	a2, 0\n"							\
 	".4byte	%2\n"								\
-	: : "r" (base), "i" (fn), "i" (MK_CBO(fn)) : "a0", "a1", "memory");	\
+	: : "r" (base), "i" (fn), "i" (MK_CBO(fn)) : "a0", "a1", "a2", "memory");	\
+})
+
+#define prefetch_insn(base, fn)							\
+({										\
+	asm volatile(								\
+	"mv	a0, %0\n"							\
+	"li	a1, %1\n"							\
+	"li	a2, 1\n"							\
+	".4byte	%2\n"								\
+	: : "r" (base), "i" (fn), "i" (MK_PREFETCH(fn)) : "a0", "a1", "a2");\
 })
 
 static void cbo_inval(char *base) { cbo_insn(base, 0); }
 static void cbo_clean(char *base) { cbo_insn(base, 1); }
 static void cbo_flush(char *base) { cbo_insn(base, 2); }
 static void cbo_zero(char *base)  { cbo_insn(base, 4); }
+static void prefetch_i(char *base) { prefetch_insn(base, 0); }
+static void prefetch_r(char *base) { prefetch_insn(base, 1); }
+static void prefetch_w(char *base) { prefetch_insn(base, 3); }
+
+static void test_no_zicbop(void *arg)
+{
+	/* Zicbop prefetch.* are HINT instructions. */
+	ksft_print_msg("Testing Zicbop instructions\n");
+
+	got_fault = false;
+	prefetch_i(&mem[0]);
+	ksft_test_result(!got_fault, "No prefetch.i\n");
+
+	got_fault = false;
+	prefetch_r(&mem[0]);
+	ksft_test_result(!got_fault, "No prefetch.r\n");
+
+	got_fault = false;
+	prefetch_w(&mem[0]);
+	ksft_test_result(!got_fault, "No prefetch.w\n");
+}
 
 static void test_no_cbo_inval(void *arg)
 {
 	ksft_print_msg("Testing cbo.inval instruction remain privileged\n");
-	illegal_insn = false;
+	got_fault = false;
 	cbo_inval(&mem[0]);
-	ksft_test_result(illegal_insn, "No cbo.inval\n");
+	ksft_test_result(got_fault, "No cbo.inval\n");
 }
 
 static void test_no_zicbom(void *arg)
 {
 	ksft_print_msg("Testing Zicbom instructions remain privileged\n");
 
-	illegal_insn = false;
+	got_fault = false;
 	cbo_clean(&mem[0]);
-	ksft_test_result(illegal_insn, "No cbo.clean\n");
+	ksft_test_result(got_fault, "No cbo.clean\n");
 
-	illegal_insn = false;
+	got_fault = false;
 	cbo_flush(&mem[0]);
-	ksft_test_result(illegal_insn, "No cbo.flush\n");
+	ksft_test_result(got_fault, "No cbo.flush\n");
 }
 
 static void test_no_zicboz(void *arg)
 {
 	ksft_print_msg("No Zicboz, testing cbo.zero remains privileged\n");
 
-	illegal_insn = false;
+	got_fault = false;
 	cbo_zero(&mem[0]);
-	ksft_test_result(illegal_insn, "No cbo.zero\n");
+	ksft_test_result(got_fault, "No cbo.zero\n");
 }
 
 static bool is_power_of_2(__u64 n)
@@ -85,6 +123,34 @@ static bool is_power_of_2(__u64 n)
 	return n != 0 && (n & (n - 1)) == 0;
 }
 
+static void test_zicbop(void *arg)
+{
+	struct riscv_hwprobe pair = {
+		.key = RISCV_HWPROBE_KEY_ZICBOP_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_ZICBOP_BLOCK_SIZE &&
+			 is_power_of_2(block_size), "Zicbop block size\n");
+	ksft_print_msg("Zicbop block size: %llu\n", block_size);
+
+	got_fault = false;
+	prefetch_i(&mem[0]);
+	prefetch_r(&mem[0]);
+	prefetch_w(&mem[0]);
+	ksft_test_result(!got_fault, "Zicbop prefetch.* on valid address\n");
+
+	got_fault = false;
+	prefetch_i(NULL);
+	prefetch_r(NULL);
+	prefetch_w(NULL);
+	ksft_test_result(!got_fault, "Zicbop prefetch.* on NULL\n");
+}
+
 static void test_zicbom(void *arg)
 {
 	struct riscv_hwprobe pair = {
@@ -100,13 +166,13 @@ static void test_zicbom(void *arg)
 			 is_power_of_2(block_size), "Zicbom block size\n");
 	ksft_print_msg("Zicbom block size: %llu\n", block_size);
 
-	illegal_insn = false;
+	got_fault = false;
 	cbo_clean(&mem[block_size]);
-	ksft_test_result(!illegal_insn, "cbo.clean\n");
+	ksft_test_result(!got_fault, "cbo.clean\n");
 
-	illegal_insn = false;
+	got_fault = false;
 	cbo_flush(&mem[block_size]);
-	ksft_test_result(!illegal_insn, "cbo.flush\n");
+	ksft_test_result(!got_fault, "cbo.flush\n");
 }
 
 static void test_zicboz(void *arg)
@@ -125,11 +191,11 @@ static void test_zicboz(void *arg)
 			 is_power_of_2(block_size), "Zicboz block size\n");
 	ksft_print_msg("Zicboz block size: %llu\n", block_size);
 
-	illegal_insn = false;
+	got_fault = false;
 	cbo_zero(&mem[block_size]);
-	ksft_test_result(!illegal_insn, "cbo.zero\n");
+	ksft_test_result(!got_fault, "cbo.zero\n");
 
-	if (illegal_insn || !is_power_of_2(block_size)) {
+	if (got_fault || !is_power_of_2(block_size)) {
 		ksft_test_result_skip("cbo.zero check\n");
 		return;
 	}
@@ -148,7 +214,7 @@ static void test_zicboz(void *arg)
 			if (mem[i * block_size + j] != expected) {
 				ksft_test_result_fail("cbo.zero check\n");
 				ksft_print_msg("cbo.zero check: mem[%llu] != 0x%x\n",
-					       i * block_size + j, expected);
+						   i * block_size + j, expected);
 				return;
 			}
 		}
@@ -177,7 +243,19 @@ static void check_no_zicbo_cpus(cpu_set_t *cpus, __u64 cbo)
 		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);
 
-		cbostr = cbo == RISCV_HWPROBE_EXT_ZICBOZ ? "Zicboz" : "Zicbom";
+		switch (cbo) {
+		case RISCV_HWPROBE_EXT_ZICBOZ:
+			cbostr = "Zicboz";
+			break;
+		case RISCV_HWPROBE_EXT_ZICBOM:
+			cbostr = "Zicbom";
+			break;
+		case RISCV_HWPROBE_EXT_ZICBOP:
+			cbostr = "Zicbop";
+			break;
+		default:
+			ksft_exit_fail_msg("Internal error: invalid cbo %llu\n", cbo);
+		}
 
 		if (pair.value & cbo)
 			ksft_exit_fail_msg("%s is only present on a subset of harts.\n"
@@ -194,6 +272,8 @@ enum {
 	TEST_ZICBOM,
 	TEST_NO_ZICBOM,
 	TEST_NO_CBO_INVAL,
+	TEST_ZICBOP,
+	TEST_NO_ZICBOP
 };
 
 static struct test_info {
@@ -206,27 +286,70 @@ static struct test_info {
 	[TEST_ZICBOM]		= { .nr_tests = 3, test_zicbom },
 	[TEST_NO_ZICBOM]	= { .nr_tests = 2, test_no_zicbom },
 	[TEST_NO_CBO_INVAL]	= { .nr_tests = 1, test_no_cbo_inval },
+	[TEST_ZICBOP]		= { .nr_tests = 3, test_zicbop },
+	[TEST_NO_ZICBOP]	= { .nr_tests = 3, test_no_zicbop },
 };
 
-int main(int argc, char **argv)
+static const struct option long_opts[] = {
+	{"sigill", no_argument, 0, 'i'},
+	{"sigsegv", no_argument, 0, 's'},
+	{"sigbus", no_argument, 0, 'b'},
+	{0, 0, 0, 0}
+};
+
+static void install_sigaction(int argc, char **argv)
 {
+	int rc, opt, long_index;
+	bool sigill;
+
 	struct sigaction act = {
 		.sa_sigaction = &sigill_handler,
 		.sa_flags = SA_SIGINFO,
 	};
+
+	long_index = 0;
+	sigill = false;
+
+	while ((opt = getopt_long(argc, argv, "isb", long_opts, &long_index)) != -1) {
+		switch (opt) {
+		case 'i':
+			rc = sigaction(SIGILL, &act, NULL);
+			assert(rc == 0);
+			sigill = true;
+			break;
+		case 's':
+			rc = sigaction(SIGSEGV, &act, NULL);
+			assert(rc == 0);
+			break;
+		case 'b':
+			rc = sigaction(SIGBUS, &act, NULL);
+			assert(rc == 0);
+			break;
+		case '?':
+			fprintf(stderr, "Unknown option or missing arg\n");
+			exit(1);
+		default:
+			break;
+		}
+	}
+
+	if (sigill) {
+		tests[TEST_NO_ZICBOZ].enabled = true;
+		tests[TEST_NO_ZICBOM].enabled = true;
+		tests[TEST_NO_CBO_INVAL].enabled = true;
+		tests[TEST_NO_ZICBOP].enabled = true;
+	}
+}
+
+int main(int argc, char **argv)
+{
 	struct riscv_hwprobe pair;
 	unsigned int plan = 0;
 	cpu_set_t cpus;
 	long rc;
 	int i;
 
-	if (argc > 1 && !strcmp(argv[1], "--sigill")) {
-		rc = sigaction(SIGILL, &act, NULL);
-		assert(rc == 0);
-		tests[TEST_NO_ZICBOZ].enabled = true;
-		tests[TEST_NO_ZICBOM].enabled = true;
-		tests[TEST_NO_CBO_INVAL].enabled = true;
-	}
+	install_sigaction(argc, argv);
 
 	rc = sched_getaffinity(0, sizeof(cpu_set_t), &cpus);
 	assert(rc == 0);
@@ -253,6 +376,13 @@ int main(int argc, char **argv)
 		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOM);
 	}
 
+	if (pair.value & RISCV_HWPROBE_EXT_ZICBOP) {
+		tests[TEST_ZICBOP].enabled = true;
+		tests[TEST_NO_ZICBOP].enabled = false;
+	} else {
+		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOP);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(tests); ++i)
 		plan += tests[i].enabled ? tests[i].nr_tests : 0;
 
-- 
2.47.2
Re: [PATCH v3 2/2] selftests/riscv: Add Zicbop prefetch test
Posted by Andrew Jones 3 months, 3 weeks ago
On Mon, Oct 20, 2025 at 11:37:20PM +0800, Yao Zihong wrote:
> Add selftest to cbo.c to verify Zicbop extension behavior.
> 
> The test checks:
> - That hwprobe correctly reports Zicbop presence and block size.
> - That prefetch instructions execute without exception on valid and NULL
>   addresses when Zicbop is present.
> - That prefetch.{i,r,w} do not trigger SIGILL even when Zicbop is absent,
>   since Zicbop instructions are defined as hints.
> 
> Signed-off-by: Yao Zihong <zihong.plct@isrc.iscas.ac.cn>
> ---
>  tools/testing/selftests/riscv/hwprobe/cbo.c | 188 +++++++++++++++++---
>  1 file changed, 159 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
> index 5e96ef785d0d..163228e3f2c7 100644
> --- a/tools/testing/selftests/riscv/hwprobe/cbo.c
> +++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
> @@ -15,24 +15,30 @@
>  #include <linux/compiler.h>
>  #include <linux/kernel.h>
>  #include <asm/ucontext.h>
> +#include <getopt.h>
>  
>  #include "hwprobe.h"
>  #include "../../kselftest.h"
>  
>  #define MK_CBO(fn) le32_bswap((uint32_t)(fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15)
> +#define MK_PREFETCH(fn) \
> +	le32_bswap(0 << 25 | (uint32_t)(fn) << 20 | 10 << 15 | 6 << 12 | 0 << 7 | 19)
>  
>  static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 };
>  
> -static bool illegal_insn;
> +static bool got_fault;
>  
>  static void sigill_handler(int sig, siginfo_t *info, void *context)
>  {
>  	unsigned long *regs = (unsigned long *)&((ucontext_t *)context)->uc_mcontext;
>  	uint32_t insn = *(uint32_t *)regs[0];
>  
> -	assert(insn == MK_CBO(regs[11]));
> +	if (regs[12] == 0)
> +		assert(insn == MK_CBO(regs[11]));
> +	else
> +		assert(insn == MK_PREFETCH(regs[11]));
>  
> -	illegal_insn = true;
> +	got_fault = true;
>  	regs[0] += 4;
>  }
>  
> @@ -41,43 +47,75 @@ static void sigill_handler(int sig, siginfo_t *info, void *context)
>  	asm volatile(								\
>  	"mv	a0, %0\n"							\
>  	"li	a1, %1\n"							\
> +	"li	a2, 0\n"							\
>  	".4byte	%2\n"								\
> -	: : "r" (base), "i" (fn), "i" (MK_CBO(fn)) : "a0", "a1", "memory");	\
> +	: : "r" (base), "i" (fn), "i" (MK_CBO(fn)) : "a0", "a1", "a2", "memory");	\
> +})
> +
> +#define prefetch_insn(base, fn)							\
> +({										\
> +	asm volatile(								\
> +	"mv	a0, %0\n"							\
> +	"li	a1, %1\n"							\
> +	"li	a2, 1\n"							\
> +	".4byte	%2\n"								\
> +	: : "r" (base), "i" (fn), "i" (MK_PREFETCH(fn)) : "a0", "a1", "a2");\
>  })
>  
>  static void cbo_inval(char *base) { cbo_insn(base, 0); }
>  static void cbo_clean(char *base) { cbo_insn(base, 1); }
>  static void cbo_flush(char *base) { cbo_insn(base, 2); }
>  static void cbo_zero(char *base)  { cbo_insn(base, 4); }
> +static void prefetch_i(char *base) { prefetch_insn(base, 0); }
> +static void prefetch_r(char *base) { prefetch_insn(base, 1); }
> +static void prefetch_w(char *base) { prefetch_insn(base, 3); }
> +
> +static void test_no_zicbop(void *arg)
> +{
> +	/* Zicbop prefetch.* are HINT instructions. */
> +	ksft_print_msg("Testing Zicbop instructions\n");
> +
> +	got_fault = false;
> +	prefetch_i(&mem[0]);
> +	ksft_test_result(!got_fault, "No prefetch.i\n");
> +
> +	got_fault = false;
> +	prefetch_r(&mem[0]);
> +	ksft_test_result(!got_fault, "No prefetch.r\n");
> +
> +	got_fault = false;
> +	prefetch_w(&mem[0]);
> +	ksft_test_result(!got_fault, "No prefetch.w\n");
> +}
>  
>  static void test_no_cbo_inval(void *arg)
>  {
>  	ksft_print_msg("Testing cbo.inval instruction remain privileged\n");
> -	illegal_insn = false;
> +	got_fault = false;
>  	cbo_inval(&mem[0]);
> -	ksft_test_result(illegal_insn, "No cbo.inval\n");
> +	ksft_test_result(got_fault, "No cbo.inval\n");
>  }
>  
>  static void test_no_zicbom(void *arg)
>  {
>  	ksft_print_msg("Testing Zicbom instructions remain privileged\n");
>  
> -	illegal_insn = false;
> +	got_fault = false;
>  	cbo_clean(&mem[0]);
> -	ksft_test_result(illegal_insn, "No cbo.clean\n");
> +	ksft_test_result(got_fault, "No cbo.clean\n");
>  
> -	illegal_insn = false;
> +	got_fault = false;
>  	cbo_flush(&mem[0]);
> -	ksft_test_result(illegal_insn, "No cbo.flush\n");
> +	ksft_test_result(got_fault, "No cbo.flush\n");
>  }
>  
>  static void test_no_zicboz(void *arg)
>  {
>  	ksft_print_msg("No Zicboz, testing cbo.zero remains privileged\n");
>  
> -	illegal_insn = false;
> +	got_fault = false;
>  	cbo_zero(&mem[0]);
> -	ksft_test_result(illegal_insn, "No cbo.zero\n");
> +	ksft_test_result(got_fault, "No cbo.zero\n");
>  }
>  
>  static bool is_power_of_2(__u64 n)
> @@ -85,6 +123,34 @@ static bool is_power_of_2(__u64 n)
>  	return n != 0 && (n & (n - 1)) == 0;
>  }
>  
> +static void test_zicbop(void *arg)
> +{
> +	struct riscv_hwprobe pair = {
> +		.key = RISCV_HWPROBE_KEY_ZICBOP_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_ZICBOP_BLOCK_SIZE &&
> +			 is_power_of_2(block_size), "Zicbop block size\n");
> +	ksft_print_msg("Zicbop block size: %llu\n", block_size);
> +
> +	got_fault = false;
> +	prefetch_i(&mem[0]);
> +	prefetch_r(&mem[0]);
> +	prefetch_w(&mem[0]);
> +	ksft_test_result(!got_fault, "Zicbop prefetch.* on valid address\n");
> +
> +	got_fault = false;
> +	prefetch_i(NULL);
> +	prefetch_r(NULL);
> +	prefetch_w(NULL);
> +	ksft_test_result(!got_fault, "Zicbop prefetch.* on NULL\n");
> +}
> +
>  static void test_zicbom(void *arg)
>  {
>  	struct riscv_hwprobe pair = {
> @@ -100,13 +166,13 @@ static void test_zicbom(void *arg)
>  			 is_power_of_2(block_size), "Zicbom block size\n");
>  	ksft_print_msg("Zicbom block size: %llu\n", block_size);
>  
> -	illegal_insn = false;
> +	got_fault = false;
>  	cbo_clean(&mem[block_size]);
> -	ksft_test_result(!illegal_insn, "cbo.clean\n");
> +	ksft_test_result(!got_fault, "cbo.clean\n");
>  
> -	illegal_insn = false;
> +	got_fault = false;
>  	cbo_flush(&mem[block_size]);
> -	ksft_test_result(!illegal_insn, "cbo.flush\n");
> +	ksft_test_result(!got_fault, "cbo.flush\n");
>  }
>  
>  static void test_zicboz(void *arg)
> @@ -125,11 +191,11 @@ static void test_zicboz(void *arg)
>  			 is_power_of_2(block_size), "Zicboz block size\n");
>  	ksft_print_msg("Zicboz block size: %llu\n", block_size);
>  
> -	illegal_insn = false;
> +	got_fault = false;
>  	cbo_zero(&mem[block_size]);
> -	ksft_test_result(!illegal_insn, "cbo.zero\n");
> +	ksft_test_result(!got_fault, "cbo.zero\n");
>  
> -	if (illegal_insn || !is_power_of_2(block_size)) {
> +	if (got_fault || !is_power_of_2(block_size)) {
>  		ksft_test_result_skip("cbo.zero check\n");
>  		return;
>  	}
> @@ -148,7 +214,7 @@ static void test_zicboz(void *arg)
>  			if (mem[i * block_size + j] != expected) {
>  				ksft_test_result_fail("cbo.zero check\n");
>  				ksft_print_msg("cbo.zero check: mem[%llu] != 0x%x\n",
> -					       i * block_size + j, expected);
> +						   i * block_size + j, expected);

Undesired reformatting

>  				return;
>  			}
>  		}
> @@ -177,7 +243,19 @@ static void check_no_zicbo_cpus(cpu_set_t *cpus, __u64 cbo)
>  		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);
>  
> -		cbostr = cbo == RISCV_HWPROBE_EXT_ZICBOZ ? "Zicboz" : "Zicbom";
> +		switch (cbo) {
> +		case RISCV_HWPROBE_EXT_ZICBOZ:
> +			cbostr = "Zicboz";
> +			break;
> +		case RISCV_HWPROBE_EXT_ZICBOM:
> +			cbostr = "Zicbom";
> +			break;
> +		case RISCV_HWPROBE_EXT_ZICBOP:
> +			cbostr = "Zicbop";
> +			break;
> +		default:
> +			ksft_exit_fail_msg("Internal error: invalid cbo %llu\n", cbo);
> +		}
>  
>  		if (pair.value & cbo)
>  			ksft_exit_fail_msg("%s is only present on a subset of harts.\n"
> @@ -194,6 +272,8 @@ enum {
>  	TEST_ZICBOM,
>  	TEST_NO_ZICBOM,
>  	TEST_NO_CBO_INVAL,
> +	TEST_ZICBOP,
> +	TEST_NO_ZICBOP
>  };
>  
>  static struct test_info {
> @@ -206,27 +286,70 @@ static struct test_info {
>  	[TEST_ZICBOM]		= { .nr_tests = 3, test_zicbom },
>  	[TEST_NO_ZICBOM]	= { .nr_tests = 2, test_no_zicbom },
>  	[TEST_NO_CBO_INVAL]	= { .nr_tests = 1, test_no_cbo_inval },
> +	[TEST_ZICBOP]		= { .nr_tests = 3, test_zicbop },
> +	[TEST_NO_ZICBOP]	= { .nr_tests = 3, test_no_zicbop },
>  };
>  
> -int main(int argc, char **argv)
> +static const struct option long_opts[] = {
> +	{"sigill", no_argument, 0, 'i'},
> +	{"sigsegv", no_argument, 0, 's'},
> +	{"sigbus", no_argument, 0, 'b'},
> +	{0, 0, 0, 0}
> +};
> +
> +static void install_sigaction(int argc, char **argv)
>  {
> +	int rc, opt, long_index;
> +	bool sigill;
> +
>  	struct sigaction act = {
>  		.sa_sigaction = &sigill_handler,
>  		.sa_flags = SA_SIGINFO,
>  	};
> +
> +	long_index = 0;
> +	sigill = false;
> +
> +	while ((opt = getopt_long(argc, argv, "isb", long_opts, &long_index)) != -1) {
> +		switch (opt) {
> +		case 'i':
> +			rc = sigaction(SIGILL, &act, NULL);
> +			assert(rc == 0);
> +			sigill = true;
> +			break;
> +		case 's':
> +			rc = sigaction(SIGSEGV, &act, NULL);
> +			assert(rc == 0);
> +			break;
> +		case 'b':
> +			rc = sigaction(SIGBUS, &act, NULL);
> +			assert(rc == 0);
> +			break;

Should we just unconditionally register handlers for SIGSEGV and SIGBUS?

> +		case '?':
> +			fprintf(stderr, "Unknown option or missing arg\n");
> +			exit(1);
> +		default:
> +			break;
> +		}
> +	}
> +
> +	if (sigill) {
> +		tests[TEST_NO_ZICBOZ].enabled = true;
> +		tests[TEST_NO_ZICBOM].enabled = true;
> +		tests[TEST_NO_CBO_INVAL].enabled = true;
> +		tests[TEST_NO_ZICBOP].enabled = true;
> +	}
> +}
> +
> +int main(int argc, char **argv)
> +{
>  	struct riscv_hwprobe pair;
>  	unsigned int plan = 0;
>  	cpu_set_t cpus;
>  	long rc;
>  	int i;
>  
> -	if (argc > 1 && !strcmp(argv[1], "--sigill")) {
> -		rc = sigaction(SIGILL, &act, NULL);
> -		assert(rc == 0);
> -		tests[TEST_NO_ZICBOZ].enabled = true;
> -		tests[TEST_NO_ZICBOM].enabled = true;
> -		tests[TEST_NO_CBO_INVAL].enabled = true;
> -	}
> +	install_sigaction(argc, argv);
>  
>  	rc = sched_getaffinity(0, sizeof(cpu_set_t), &cpus);
>  	assert(rc == 0);
> @@ -253,6 +376,13 @@ int main(int argc, char **argv)
>  		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOM);
>  	}
>  
> +	if (pair.value & RISCV_HWPROBE_EXT_ZICBOP) {
> +		tests[TEST_ZICBOP].enabled = true;
> +		tests[TEST_NO_ZICBOP].enabled = false;
> +	} else {
> +		check_no_zicbo_cpus(&cpus, RISCV_HWPROBE_EXT_ZICBOP);
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(tests); ++i)
>  		plan += tests[i].enabled ? tests[i].nr_tests : 0;
>  
> -- 
> 2.47.2
>

Otherwise

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Re: [PATCH v3 2/2] selftests/riscv: Add Zicbop prefetch test
Posted by Yao Zihong 3 months, 2 weeks ago
> Undesired reformatting
My bad. It will be fixed in the next version.

> Should we just unconditionally register handlers for SIGSEGV and SIGBUS?
I think it depends on what the flags --sig{segv,ill,bus} are intended to mean:

  a) We intend to handle these faults that might be raised inside the test
     (i.e., catch and convert them into pass/fail results without crashing the
     test binary, rather than letting something else handle them externally).
  b) We expect these signals to be raised as part of the test scenario and
     handle them within the test program accordingly.

I'm not sure if (a) is appropriate, since it might mess up someone’s CI
or other automation setups.

If we’re going with (b), then registering handlers for SIGSEGV and SIGBUS
based on flags would be inconsistent with that semantics, since prefetch.*
should never legitimately raise them. In that case, this design probably
doesn’t make sense. Would it also make sense to rename the '--sigill' flag
to make this clearer?

Thanks,
Zihong

Re: [PATCH v3 2/2] selftests/riscv: Add Zicbop prefetch test
Posted by Andrew Jones 3 months, 1 week ago
On Thu, Oct 23, 2025 at 11:48:18PM +0800, Yao Zihong wrote:
> > Undesired reformatting
> My bad. It will be fixed in the next version.
> 
> > Should we just unconditionally register handlers for SIGSEGV and SIGBUS?
> I think it depends on what the flags --sig{segv,ill,bus} are intended to mean:
> 
>   a) We intend to handle these faults that might be raised inside the test
>      (i.e., catch and convert them into pass/fail results without crashing the
>      test binary, rather than letting something else handle them externally).
>   b) We expect these signals to be raised as part of the test scenario and
>      handle them within the test program accordingly.
> 
> I'm not sure if (a) is appropriate, since it might mess up someone’s CI
> or other automation setups.

The intention of --sigill is to tell the test that a SIGILL is expected
and tests should be run to ensure they are raised. We should probably
allow that expectation to be extension specific though, i.e. have both a
--zicbom-raises-sigill and a --zicboz-raises-sigill

> 
> If we’re going with (b), then registering handlers for SIGSEGV and SIGBUS
> based on flags would be inconsistent with that semantics, since prefetch.*
> should never legitimately raise them. In that case, this design probably
> doesn’t make sense. Would it also make sense to rename the '--sigill' flag
> to make this clearer?

Since SIGSEGV and SIGBUS are never expected, then we could always handle
those in order to report failures, but we should narrow the time the
handlers are registered to be just around the use of instructions under
test.

Thanks,
drew