linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode
@ 2023-09-04 17:02 Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 1/6] RISC-V: Make zicbom/zicboz errors consistent Andrew Jones
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-04 17:02 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

In order for usermode to issue cbo.zero, it needs privilege granted to
issue the extension instruction (patch 2) and to know that the extension
is available and its block size (patch 3). Patch 1 could be separate from
this series (it just fixes up some error messages), patches 4-5 convert
the hwprobe selftest to a statically-linked, TAP test and patch 6 adds a
new hwprobe test for the new information as well as testing CBO
instructions can or cannot be issued as appropriate.

Thanks,
drew

v3:
  - rebased on for-next
  - Explained the use of CONFIG_RISCV_ALTERNATIVE in the commit message
    for patch 2
  - fixed a copy+paste error in the cbo selftest [Xiao]
  - while touching the cbo selftest improved its readability with a
    MK_CBO() macro and now skip the cbo.zero test if we see the first
    try is an illegal instruction
  - picked up a few r-b's

v2:
  - fixed build of the vector selftest
  - changed this-cpu wrappers to just cpu wrappers and then pass
    smp_processor_id() at the callsite
  - added comment to EXT_KEY macro
  - picked up a couple r-b's


Andrew Jones (6):
  RISC-V: Make zicbom/zicboz errors consistent
  RISC-V: Enable cbo.zero in usermode
  RISC-V: hwprobe: Expose Zicboz extension and its block size
  RISC-V: selftests: Statically link hwprobe test
  RISC-V: selftests: Convert hwprobe test to kselftest API
  RISC-V: selftests: Add CBO tests

 Documentation/riscv/hwprobe.rst               |   6 +
 arch/riscv/include/asm/cpufeature.h           |   1 +
 arch/riscv/include/asm/csr.h                  |   1 +
 arch/riscv/include/asm/hwcap.h                |  16 ++
 arch/riscv/include/asm/hwprobe.h              |   2 +-
 arch/riscv/include/uapi/asm/hwprobe.h         |   2 +
 arch/riscv/kernel/cpufeature.c                |  10 +-
 arch/riscv/kernel/setup.c                     |   4 +
 arch/riscv/kernel/smpboot.c                   |   4 +
 arch/riscv/kernel/sys_riscv.c                 |  46 +++--
 .../testing/selftests/riscv/hwprobe/Makefile  |   9 +-
 tools/testing/selftests/riscv/hwprobe/cbo.c   | 170 ++++++++++++++++++
 .../testing/selftests/riscv/hwprobe/hwprobe.c |  64 +++----
 .../testing/selftests/riscv/hwprobe/hwprobe.h |  15 ++
 14 files changed, 287 insertions(+), 63 deletions(-)
 create mode 100644 tools/testing/selftests/riscv/hwprobe/cbo.c
 create mode 100644 tools/testing/selftests/riscv/hwprobe/hwprobe.h

-- 
2.41.0


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

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

* [PATCH v3 1/6] RISC-V: Make zicbom/zicboz errors consistent
  2023-09-04 17:02 [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
@ 2023-09-04 17:02 ` Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 2/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-04 17:02 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

commit c818fea83de4 ("riscv: say disabling zicbom if no or bad
riscv,cbom-block-size found") improved the error messages for
zicbom but zicboz was missed since its patches were in flight
at the same time. Get 'em now.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/kernel/cpufeature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1cfbba65d11a..f9ac2717bc7d 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -93,10 +93,10 @@ static bool riscv_isa_extension_check(int id)
 		return true;
 	case RISCV_ISA_EXT_ZICBOZ:
 		if (!riscv_cboz_block_size) {
-			pr_err("Zicboz detected in ISA string, but no cboz-block-size found\n");
+			pr_err("Zicboz detected in ISA string, disabling as no cboz-block-size found\n");
 			return false;
 		} else if (!is_power_of_2(riscv_cboz_block_size)) {
-			pr_err("cboz-block-size present, but is not a power-of-2\n");
+			pr_err("Zicboz disabled as cboz-block-size present, but is not a power-of-2\n");
 			return false;
 		}
 		return true;
-- 
2.41.0


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

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

* [PATCH v3 2/6] RISC-V: Enable cbo.zero in usermode
  2023-09-04 17:02 [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 1/6] RISC-V: Make zicbom/zicboz errors consistent Andrew Jones
@ 2023-09-04 17:02 ` Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size Andrew Jones
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-04 17:02 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

When Zicboz is present, enable its instruction (cbo.zero) in
usermode by setting its respective senvcfg bit. We don't bother
trying to set this bit per-task, which would also require an
interface for tasks to request enabling and/or disabling. Instead,
permanently set the bit for each hart which has the extension when
bringing it online.

This patch also introduces riscv_cpu_has_extension_[un]likely()
functions to check a specific hart's ISA bitmap for extensions.
Prior to checking the specific hart's bitmap in these functions
we try the bitmap which represents the LCD of extensions, but only
when we know it will use its optimized, alternatives path by gating
its call on CONFIG_RISCV_ALTERNATIVE. When alternatives are used, the
compiler ensures that the invocation of the LCD search becomes a
constant true or false. When it's true, even the new functions will
completely vanish from their callsites. OTOH, when the LCD check is
false, we need to do a search of the hart's ISA bitmap. Had we also
checked the LCD bitmap without the use of alternatives, then we would
have ended up with two bitmap searches instead of one.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/cpufeature.h |  1 +
 arch/riscv/include/asm/csr.h        |  1 +
 arch/riscv/include/asm/hwcap.h      | 16 ++++++++++++++++
 arch/riscv/kernel/cpufeature.c      |  6 ++++++
 arch/riscv/kernel/setup.c           |  4 ++++
 arch/riscv/kernel/smpboot.c         |  4 ++++
 6 files changed, 32 insertions(+)

diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
index d0345bd659c9..13b7d35648a9 100644
--- a/arch/riscv/include/asm/cpufeature.h
+++ b/arch/riscv/include/asm/cpufeature.h
@@ -31,5 +31,6 @@ DECLARE_PER_CPU(long, misaligned_access_speed);
 extern struct riscv_isainfo hart_isa[NR_CPUS];
 
 void check_unaligned_access(int cpu);
+void riscv_user_isa_enable(void);
 
 #endif
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 7bac43a3176e..e187e76e3df4 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -273,6 +273,7 @@
 #define CSR_SIE			0x104
 #define CSR_STVEC		0x105
 #define CSR_SCOUNTEREN		0x106
+#define CSR_SENVCFG		0x10a
 #define CSR_SSCRATCH		0x140
 #define CSR_SEPC		0x141
 #define CSR_SCAUSE		0x142
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index b7b58258f6c7..31774bcdf1c6 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -70,6 +70,7 @@
 #ifndef __ASSEMBLY__
 
 #include <linux/jump_label.h>
+#include <asm/cpufeature.h>
 
 unsigned long riscv_get_elf_hwcap(void);
 
@@ -137,6 +138,21 @@ riscv_has_extension_unlikely(const unsigned long ext)
 	return true;
 }
 
+static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
+{
+	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
+		return true;
+
+	return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
+}
+
+static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
+{
+	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
+		return true;
+
+	return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
+}
 #endif
 
 #endif /* _ASM_RISCV_HWCAP_H */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index f9ac2717bc7d..8ad6da03ee34 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -653,6 +653,12 @@ static int check_unaligned_access_boot_cpu(void)
 
 arch_initcall(check_unaligned_access_boot_cpu);
 
+void riscv_user_isa_enable(void)
+{
+	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
+		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
+}
+
 #ifdef CONFIG_RISCV_ALTERNATIVE
 /*
  * Alternative patch sites consider 48 bits when determining when to patch
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 32c2e1eb71bd..7b796f5f7dad 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -25,6 +25,7 @@
 #include <asm/acpi.h>
 #include <asm/alternative.h>
 #include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
 #include <asm/early_ioremap.h>
 #include <asm/pgtable.h>
@@ -313,10 +314,13 @@ void __init setup_arch(char **cmdline_p)
 	riscv_fill_hwcap();
 	init_rt_signal_env();
 	apply_boot_alternatives();
+
 	if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
 	    riscv_isa_extension_available(NULL, ZICBOM))
 		riscv_noncoherent_supported();
 	riscv_set_dma_cache_alignment();
+
+	riscv_user_isa_enable();
 }
 
 static int __init topology_init(void)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 1b8da4e40a4d..d1b0a6fc3adf 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -25,6 +25,8 @@
 #include <linux/of.h>
 #include <linux/sched/task_stack.h>
 #include <linux/sched/mm.h>
+
+#include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
 #include <asm/cpufeature.h>
 #include <asm/irq.h>
@@ -253,6 +255,8 @@ asmlinkage __visible void smp_callin(void)
 			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
 	}
 
+	riscv_user_isa_enable();
+
 	/*
 	 * Remote TLB flushes are ignored while the CPU is offline, so emit
 	 * a local TLB flush right now just in case.
-- 
2.41.0


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

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

* [PATCH v3 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size
  2023-09-04 17:02 [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 1/6] RISC-V: Make zicbom/zicboz errors consistent Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 2/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
@ 2023-09-04 17:02 ` Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 4/6] RISC-V: selftests: Statically link hwprobe test Andrew Jones
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-04 17:02 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

Expose Zicboz through hwprobe and also provide a key to extract its
respective block size. Opportunistically add a macro and apply it to
current extensions in order to avoid duplicating code.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Evan Green <evan@rivosinc.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/riscv/hwprobe.rst       |  6 ++++
 arch/riscv/include/asm/hwprobe.h      |  2 +-
 arch/riscv/include/uapi/asm/hwprobe.h |  2 ++
 arch/riscv/kernel/sys_riscv.c         | 46 +++++++++++++++++++--------
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
index a52996b22f75..7b2384de471f 100644
--- a/Documentation/riscv/hwprobe.rst
+++ b/Documentation/riscv/hwprobe.rst
@@ -77,6 +77,9 @@ The following keys are defined:
   * :c:macro:`RISCV_HWPROBE_EXT_ZBS`: The Zbs extension is supported, as defined
        in version 1.0 of the Bit-Manipulation ISA extensions.
 
+  * :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_KEY_CPUPERF_0`: A bitmask that contains performance
   information about the selected set of processors.
 
@@ -96,3 +99,6 @@ The following keys are defined:
 
   * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
     not supported at all and will generate a misaligned address fault.
+
+* :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
+  represents the size of the Zicboz block in bytes.
diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
index 78936f4ff513..39df8604fea1 100644
--- a/arch/riscv/include/asm/hwprobe.h
+++ b/arch/riscv/include/asm/hwprobe.h
@@ -8,6 +8,6 @@
 
 #include <uapi/asm/hwprobe.h>
 
-#define RISCV_HWPROBE_MAX_KEY 5
+#define RISCV_HWPROBE_MAX_KEY 6
 
 #endif
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 006bfb48343d..86d08a0e617b 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -29,6 +29,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZBA		(1 << 3)
 #define		RISCV_HWPROBE_EXT_ZBB		(1 << 4)
 #define		RISCV_HWPROBE_EXT_ZBS		(1 << 5)
+#define		RISCV_HWPROBE_EXT_ZICBOZ	(1 << 6)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
 #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
@@ -36,6 +37,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_MISALIGNED_FAST		(3 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
+#define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
 /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
 
 #endif
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index 473159b5f303..d7266a9abc66 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -145,26 +145,38 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 	for_each_cpu(cpu, cpus) {
 		struct riscv_isainfo *isainfo = &hart_isa[cpu];
 
-		if (riscv_isa_extension_available(isainfo->isa, ZBA))
-			pair->value |= RISCV_HWPROBE_EXT_ZBA;
-		else
-			missing |= RISCV_HWPROBE_EXT_ZBA;
-
-		if (riscv_isa_extension_available(isainfo->isa, ZBB))
-			pair->value |= RISCV_HWPROBE_EXT_ZBB;
-		else
-			missing |= RISCV_HWPROBE_EXT_ZBB;
-
-		if (riscv_isa_extension_available(isainfo->isa, ZBS))
-			pair->value |= RISCV_HWPROBE_EXT_ZBS;
-		else
-			missing |= RISCV_HWPROBE_EXT_ZBS;
+#define EXT_KEY(ext)									\
+	do {										\
+		if (__riscv_isa_extension_available(isainfo->isa, RISCV_ISA_EXT_##ext))	\
+			pair->value |= RISCV_HWPROBE_EXT_##ext;				\
+		else									\
+			missing |= RISCV_HWPROBE_EXT_##ext;				\
+	} while (false)
+
+		/*
+		 * Only use EXT_KEY() for extensions which can be exposed to userspace,
+		 * regardless of the kernel's configuration, as no other checks, besides
+		 * presence in the hart_isa bitmap, are made.
+		 */
+		EXT_KEY(ZBA);
+		EXT_KEY(ZBB);
+		EXT_KEY(ZBS);
+		EXT_KEY(ZICBOZ);
+#undef EXT_KEY
 	}
 
 	/* Now turn off reporting features if any CPU is missing it. */
 	pair->value &= ~missing;
 }
 
+static bool hwprobe_ext0_has(const struct cpumask *cpus, unsigned long ext)
+{
+	struct riscv_hwprobe pair;
+
+	hwprobe_isa_ext0(&pair, cpus);
+	return (pair.value & ext);
+}
+
 static u64 hwprobe_misaligned(const struct cpumask *cpus)
 {
 	int cpu;
@@ -215,6 +227,12 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
 		pair->value = hwprobe_misaligned(cpus);
 		break;
 
+	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
+		pair->value = 0;
+		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
+			pair->value = riscv_cboz_block_size;
+		break;
+
 	/*
 	 * For forward compatibility, unknown keys don't fail the whole
 	 * call, but get their element key set to -1 and value set to 0
-- 
2.41.0


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

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

* [PATCH v3 4/6] RISC-V: selftests: Statically link hwprobe test
  2023-09-04 17:02 [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
                   ` (2 preceding siblings ...)
  2023-09-04 17:02 ` [PATCH v3 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size Andrew Jones
@ 2023-09-04 17:02 ` Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 5/6] RISC-V: selftests: Convert hwprobe test to kselftest API Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 6/6] RISC-V: selftests: Add CBO tests Andrew Jones
  5 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-04 17:02 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

Statically linking makes it more convenient to copy the test to a
minimal busybox environment.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 tools/testing/selftests/riscv/hwprobe/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/riscv/hwprobe/Makefile b/tools/testing/selftests/riscv/hwprobe/Makefile
index ebdbb3c22e54..5f614c3ba598 100644
--- a/tools/testing/selftests/riscv/hwprobe/Makefile
+++ b/tools/testing/selftests/riscv/hwprobe/Makefile
@@ -7,4 +7,4 @@ TEST_GEN_PROGS := hwprobe
 include ../../lib.mk
 
 $(OUTPUT)/hwprobe: hwprobe.c sys_hwprobe.S
-	$(CC) -o$@ $(CFLAGS) $(LDFLAGS) $^
+	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
-- 
2.41.0


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

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

* [PATCH v3 5/6] RISC-V: selftests: Convert hwprobe test to kselftest API
  2023-09-04 17:02 [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
                   ` (3 preceding siblings ...)
  2023-09-04 17:02 ` [PATCH v3 4/6] RISC-V: selftests: Statically link hwprobe test Andrew Jones
@ 2023-09-04 17:02 ` Andrew Jones
  2023-09-04 17:02 ` [PATCH v3 6/6] RISC-V: selftests: Add CBO tests Andrew Jones
  5 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-04 17:02 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

Returning (exiting with) negative exit codes isn't user friendly,
because the user must output the exit code with the shell, convert it
from its unsigned 8-bit value back to the negative value, and then
look up where that comes from in the code (which may be multiple
places). Use the kselftests TAP interface, instead.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 .../testing/selftests/riscv/hwprobe/hwprobe.c | 54 +++++++------------
 1 file changed, 20 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/riscv/hwprobe/hwprobe.c b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
index 09f290a67420..4f15f1f3b4c3 100644
--- a/tools/testing/selftests/riscv/hwprobe/hwprobe.c
+++ b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
@@ -2,6 +2,8 @@
 #include <stddef.h>
 #include <asm/hwprobe.h>
 
+#include "../../kselftest.h"
+
 /*
  * Rather than relying on having a new enough libc to define this, just do it
  * ourselves.  This way we don't need to be coupled to a new-enough libc to
@@ -16,6 +18,9 @@ int main(int argc, char **argv)
 	unsigned long cpus;
 	long out;
 
+	ksft_print_header();
+	ksft_set_plan(5);
+
 	/* Fake the CPU_SET ops. */
 	cpus = -1;
 
@@ -25,13 +30,16 @@ int main(int argc, char **argv)
 	 */
 	for (long i = 0; i < 8; i++)
 		pairs[i].key = i;
+
 	out = riscv_hwprobe(pairs, 8, 1, &cpus, 0);
 	if (out != 0)
-		return -1;
+		ksft_exit_fail_msg("hwprobe() failed with %ld\n", out);
+
 	for (long i = 0; i < 4; ++i) {
 		/* Fail if the kernel claims not to recognize a base key. */
 		if ((i < 4) && (pairs[i].key != i))
-			return -2;
+			ksft_exit_fail_msg("Failed to recognize base key: key != i, "
+					   "key=%ld, i=%ld\n", pairs[i].key, i);
 
 		if (pairs[i].key != RISCV_HWPROBE_KEY_BASE_BEHAVIOR)
 			continue;
@@ -39,52 +47,30 @@ int main(int argc, char **argv)
 		if (pairs[i].value & RISCV_HWPROBE_BASE_BEHAVIOR_IMA)
 			continue;
 
-		return -3;
+		ksft_exit_fail_msg("Unexpected pair: (%ld, %ld)\n", pairs[i].key, pairs[i].value);
 	}
 
-	/*
-	 * This should also work with a NULL CPU set, but should not work
-	 * with an improperly supplied CPU set.
-	 */
 	out = riscv_hwprobe(pairs, 8, 0, 0, 0);
-	if (out != 0)
-		return -4;
+	ksft_test_result(out == 0, "NULL CPU set\n");
 
 	out = riscv_hwprobe(pairs, 8, 0, &cpus, 0);
-	if (out == 0)
-		return -5;
+	ksft_test_result(out != 0, "Bad CPU set\n");
 
 	out = riscv_hwprobe(pairs, 8, 1, 0, 0);
-	if (out == 0)
-		return -6;
+	ksft_test_result(out != 0, "NULL CPU set with non-zero count\n");
 
-	/*
-	 * Check that keys work by providing one that we know exists, and
-	 * checking to make sure the resultig pair is what we asked for.
-	 */
 	pairs[0].key = RISCV_HWPROBE_KEY_BASE_BEHAVIOR;
 	out = riscv_hwprobe(pairs, 1, 1, &cpus, 0);
-	if (out != 0)
-		return -7;
-	if (pairs[0].key != RISCV_HWPROBE_KEY_BASE_BEHAVIOR)
-		return -8;
+	ksft_test_result(out == 0 && pairs[0].key == RISCV_HWPROBE_KEY_BASE_BEHAVIOR,
+			 "Existing key is maintained\n");
 
-	/*
-	 * Check that an unknown key gets overwritten with -1,
-	 * but doesn't block elements after it.
-	 */
 	pairs[0].key = 0x5555;
 	pairs[1].key = 1;
 	pairs[1].value = 0xAAAA;
 	out = riscv_hwprobe(pairs, 2, 0, 0, 0);
-	if (out != 0)
-		return -9;
-
-	if (pairs[0].key != -1)
-		return -10;
-
-	if ((pairs[1].key != 1) || (pairs[1].value == 0xAAAA))
-		return -11;
+	ksft_test_result(out == 0 && pairs[0].key == -1 &&
+			 pairs[1].key == 1 && pairs[1].value != 0xAAAA,
+			 "Unknown key overwritten with -1 and doesn't block other elements\n");
 
-	return 0;
+	ksft_finished();
 }
-- 
2.41.0


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

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

* [PATCH v3 6/6] RISC-V: selftests: Add CBO tests
  2023-09-04 17:02 [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
                   ` (4 preceding siblings ...)
  2023-09-04 17:02 ` [PATCH v3 5/6] RISC-V: selftests: Convert hwprobe test to kselftest API Andrew Jones
@ 2023-09-04 17:02 ` Andrew Jones
  2023-09-05 14:36   ` Wang, Xiao W
  2023-09-18 10:41   ` Andrew Jones
  5 siblings, 2 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-04 17:02 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

Add hwprobe test for Zicboz and its block size. Also, when Zicboz is
present, test that cbo.zero may be issued and works. Additionally
test that the Zicbom instructions cause SIGILL and also that cbo.zero
causes SIGILL when Zicboz is not present. Pinning the test to a subset
of cpus with taskset will also restrict the hwprobe calls to that set.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 .../testing/selftests/riscv/hwprobe/Makefile  |   7 +-
 tools/testing/selftests/riscv/hwprobe/cbo.c   | 170 ++++++++++++++++++
 .../testing/selftests/riscv/hwprobe/hwprobe.c |  12 +-
 .../testing/selftests/riscv/hwprobe/hwprobe.h |  15 ++
 4 files changed, 192 insertions(+), 12 deletions(-)
 create mode 100644 tools/testing/selftests/riscv/hwprobe/cbo.c
 create mode 100644 tools/testing/selftests/riscv/hwprobe/hwprobe.h

diff --git a/tools/testing/selftests/riscv/hwprobe/Makefile b/tools/testing/selftests/riscv/hwprobe/Makefile
index 5f614c3ba598..f224b84591fb 100644
--- a/tools/testing/selftests/riscv/hwprobe/Makefile
+++ b/tools/testing/selftests/riscv/hwprobe/Makefile
@@ -2,9 +2,14 @@
 # Copyright (C) 2021 ARM Limited
 # Originally tools/testing/arm64/abi/Makefile
 
-TEST_GEN_PROGS := hwprobe
+CFLAGS += -I$(top_srcdir)/tools/include
+
+TEST_GEN_PROGS := hwprobe cbo
 
 include ../../lib.mk
 
 $(OUTPUT)/hwprobe: hwprobe.c sys_hwprobe.S
 	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
+
+$(OUTPUT)/cbo: cbo.c sys_hwprobe.S
+	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
new file mode 100644
index 000000000000..50e85f31a2c7
--- /dev/null
+++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Ventana Micro Systems Inc.
+ *
+ * Run with 'taskset -c <cpu-list> cbo' to only execute hwprobe on a
+ * subset of cpus, as well as only executing the tests on those cpus.
+ */
+#define _GNU_SOURCE
+#include <stdbool.h>
+#include <stdint.h>
+#include <sched.h>
+#include <signal.h>
+#include <assert.h>
+#include <linux/compiler.h>
+#include <asm/ucontext.h>
+
+#include "hwprobe.h"
+#include "../../kselftest.h"
+
+#define MK_CBO(fn) ((fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15)
+
+static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 };
+
+static bool illegal_insn;
+
+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]));
+
+	illegal_insn = true;
+	regs[0] += 4;
+}
+
+static void cbo_insn(char *base, int fn)
+{
+	uint32_t insn = MK_CBO(fn);
+
+	asm volatile(
+	"mv	a0, %0\n"
+	"li	a1, %1\n"
+	".4byte	%2\n"
+	: : "r" (base), "i" (fn), "i" (insn) : "a0", "a1", "memory");
+}
+
+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 test_no_zicbom(void)
+{
+	illegal_insn = false;
+	cbo_clean(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.clean\n");
+
+	illegal_insn = false;
+	cbo_flush(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.flush\n");
+
+	illegal_insn = false;
+	cbo_inval(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.inval\n");
+}
+
+static void test_no_zicboz(void)
+{
+	illegal_insn = false;
+	cbo_zero(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.zero\n");
+}
+
+static bool is_power_of_2(__u64 n)
+{
+	return n != 0 && (n & (n - 1)) == 0;
+}
+
+static void test_zicboz(__u64 block_size)
+{
+	int i, j;
+
+	if (!is_power_of_2(block_size)) {
+		ksft_test_result_skip("cbo.zero check\n");
+		return;
+	}
+
+	assert(block_size <= 1024);
+
+	illegal_insn = false;
+	cbo_zero(&mem[block_size]);
+	ksft_test_result(!illegal_insn, "cbo.zero\n");
+
+	if (illegal_insn) {
+		ksft_test_result_skip("cbo.zero check\n");
+		return;
+	}
+
+	for (i = 0; i < 4096 / block_size; ++i) {
+		if (i % 2)
+			cbo_zero(&mem[i * block_size]);
+	}
+
+	for (i = 0; i < 4096 / block_size; ++i) {
+		char expected = i % 2 ? 0x0 : 0xa5;
+
+		for (j = 0; j < block_size; ++j) {
+			if (mem[i * block_size + j] != expected) {
+				ksft_test_result_fail("cbo.zero check\n");
+				ksft_print_msg("cbo.zero check: mem[%d] != 0x%x\n",
+					       i * block_size + j, expected);
+				return;
+			}
+		}
+	}
+
+	ksft_test_result_pass("cbo.zero check\n");
+}
+
+int main(int argc, char **argv)
+{
+	struct sigaction act = {
+		.sa_sigaction = &sigill_handler,
+		.sa_flags = SA_SIGINFO,
+	};
+	bool has_zicboz = false;
+	struct riscv_hwprobe pair;
+	cpu_set_t cpus;
+	size_t nr_cpus;
+	long rc;
+
+	rc = sigaction(SIGILL, &act, NULL);
+	assert(rc == 0);
+
+	rc = sched_getaffinity(0, sizeof(cpu_set_t), &cpus);
+	assert(rc == 0);
+	nr_cpus = CPU_COUNT(&cpus);
+
+	ksft_print_header();
+
+	pair.key = RISCV_HWPROBE_KEY_IMA_EXT_0;
+	rc = riscv_hwprobe(&pair, 1, nr_cpus, (unsigned long *)&cpus, 0);
+	if (rc < 0)
+		ksft_exit_fail_msg("hwprobe() failed with %d\n", rc);
+
+	if (pair.key != -1 && (pair.value & RISCV_HWPROBE_EXT_ZICBOZ)) {
+		has_zicboz = true;
+		ksft_set_plan(6);
+	} else {
+		ksft_print_msg("No Zicboz, testing cbo.zero remains privileged\n");
+		ksft_set_plan(4);
+	}
+
+	/* Ensure Zicbom instructions remain privileged */
+	test_no_zicbom();
+
+	if (has_zicboz) {
+		pair.key = RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE;
+		rc = riscv_hwprobe(&pair, 1, nr_cpus, (unsigned long *)&cpus, 0);
+		ksft_test_result(rc == 0 && pair.key == RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE &&
+				 is_power_of_2(pair.value), "Zicboz block size\n");
+		ksft_print_msg("Zicboz block size: %ld\n", pair.value);
+		test_zicboz(pair.value);
+	} else {
+		test_no_zicboz();
+	}
+
+	ksft_finished();
+}
diff --git a/tools/testing/selftests/riscv/hwprobe/hwprobe.c b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
index 4f15f1f3b4c3..c474891df307 100644
--- a/tools/testing/selftests/riscv/hwprobe/hwprobe.c
+++ b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
@@ -1,17 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
-#include <stddef.h>
-#include <asm/hwprobe.h>
-
+#include "hwprobe.h"
 #include "../../kselftest.h"
 
-/*
- * Rather than relying on having a new enough libc to define this, just do it
- * ourselves.  This way we don't need to be coupled to a new-enough libc to
- * contain the call.
- */
-long riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-		   size_t cpu_count, unsigned long *cpus, unsigned int flags);
-
 int main(int argc, char **argv)
 {
 	struct riscv_hwprobe pairs[8];
diff --git a/tools/testing/selftests/riscv/hwprobe/hwprobe.h b/tools/testing/selftests/riscv/hwprobe/hwprobe.h
new file mode 100644
index 000000000000..721b0ce73a56
--- /dev/null
+++ b/tools/testing/selftests/riscv/hwprobe/hwprobe.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTEST_RISCV_HWPROBE_H
+#define SELFTEST_RISCV_HWPROBE_H
+#include <stddef.h>
+#include <asm/hwprobe.h>
+
+/*
+ * Rather than relying on having a new enough libc to define this, just do it
+ * ourselves.  This way we don't need to be coupled to a new-enough libc to
+ * contain the call.
+ */
+long riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
+		   size_t cpu_count, unsigned long *cpus, unsigned int flags);
+
+#endif
-- 
2.41.0


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

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

* RE: [PATCH v3 6/6] RISC-V: selftests: Add CBO tests
  2023-09-04 17:02 ` [PATCH v3 6/6] RISC-V: selftests: Add CBO tests Andrew Jones
@ 2023-09-05 14:36   ` Wang, Xiao W
  2023-09-05 16:10     ` Andrew Jones
  2023-09-18 10:41   ` Andrew Jones
  1 sibling, 1 reply; 11+ messages in thread
From: Wang, Xiao W @ 2023-09-05 14:36 UTC (permalink / raw)
  To: Andrew Jones, linux-riscv@lists.infradead.org
  Cc: paul.walmsley@sifive.com, palmer@dabbelt.com,
	aou@eecs.berkeley.edu, evan@rivosinc.com,
	conor.dooley@microchip.com, apatel@ventanamicro.com

Hi,

> -----Original Message-----
> From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of
> Andrew Jones
> Sent: Tuesday, September 5, 2023 1:02 AM
> To: linux-riscv@lists.infradead.org
> Cc: paul.walmsley@sifive.com; palmer@dabbelt.com;
> aou@eecs.berkeley.edu; evan@rivosinc.com; conor.dooley@microchip.com;
> apatel@ventanamicro.com
> Subject: [PATCH v3 6/6] RISC-V: selftests: Add CBO tests
> 
> Add hwprobe test for Zicboz and its block size. Also, when Zicboz is
> present, test that cbo.zero may be issued and works. Additionally
> test that the Zicbom instructions cause SIGILL and also that cbo.zero
> causes SIGILL when Zicboz is not present. Pinning the test to a subset
> of cpus with taskset will also restrict the hwprobe calls to that set.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  .../testing/selftests/riscv/hwprobe/Makefile  |   7 +-
>  tools/testing/selftests/riscv/hwprobe/cbo.c   | 170 ++++++++++++++++++
>  .../testing/selftests/riscv/hwprobe/hwprobe.c |  12 +-
>  .../testing/selftests/riscv/hwprobe/hwprobe.h |  15 ++
>  4 files changed, 192 insertions(+), 12 deletions(-)
>  create mode 100644 tools/testing/selftests/riscv/hwprobe/cbo.c
>  create mode 100644 tools/testing/selftests/riscv/hwprobe/hwprobe.h
> 
> diff --git a/tools/testing/selftests/riscv/hwprobe/Makefile
> b/tools/testing/selftests/riscv/hwprobe/Makefile
> index 5f614c3ba598..f224b84591fb 100644
> --- a/tools/testing/selftests/riscv/hwprobe/Makefile
> +++ b/tools/testing/selftests/riscv/hwprobe/Makefile
> @@ -2,9 +2,14 @@
>  # Copyright (C) 2021 ARM Limited
>  # Originally tools/testing/arm64/abi/Makefile
> 
> -TEST_GEN_PROGS := hwprobe
> +CFLAGS += -I$(top_srcdir)/tools/include
> +
> +TEST_GEN_PROGS := hwprobe cbo
> 
>  include ../../lib.mk
> 
>  $(OUTPUT)/hwprobe: hwprobe.c sys_hwprobe.S
>  	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
> +
> +$(OUTPUT)/cbo: cbo.c sys_hwprobe.S
> +	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
> diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c
> b/tools/testing/selftests/riscv/hwprobe/cbo.c
> new file mode 100644
> index 000000000000..50e85f31a2c7
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + *
> + * Run with 'taskset -c <cpu-list> cbo' to only execute hwprobe on a
> + * subset of cpus, as well as only executing the tests on those cpus.
> + */

Patch 3/6 exposes a common subset extensions of the cpu list to user space, so, if some of the cores support ZICBOZ while other don't, hwprobe() won't expose ZICBOZ ext, but in this case the cbo_zero insn might run w/o illegal insn exception when the task is scheduled to run on a core that support ZICBOZ. The test result may vary for each run for this case.
Maybe we can add some comment for this special test case?

> +#define _GNU_SOURCE
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <sched.h>
> +#include <signal.h>
> +#include <assert.h>
> +#include <linux/compiler.h>
> +#include <asm/ucontext.h>
> +
> +#include "hwprobe.h"
> +#include "../../kselftest.h"
> +
> +#define MK_CBO(fn) ((fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15)
> +
> +static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 };
> +
> +static bool illegal_insn;
> +
> +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]));


The byte order of insn should always be little endian, while the CPU may be a big-endian one, then the check might fail.
Maybe we can use __le32_to_cpu(insn) to convert it before the check.

BRs,
Xiao

> +
> +	illegal_insn = true;
> +	regs[0] += 4;
> +}
> +
> +static void cbo_insn(char *base, int fn)
> +{
> +	uint32_t insn = MK_CBO(fn);
> +
> +	asm volatile(
> +	"mv	a0, %0\n"
> +	"li	a1, %1\n"
> +	".4byte	%2\n"
> +	: : "r" (base), "i" (fn), "i" (insn) : "a0", "a1", "memory");
> +}
> +
> +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 test_no_zicbom(void)
> +{
> +	illegal_insn = false;
> +	cbo_clean(&mem[0]);
> +	ksft_test_result(illegal_insn, "No cbo.clean\n");
> +
> +	illegal_insn = false;
> +	cbo_flush(&mem[0]);
> +	ksft_test_result(illegal_insn, "No cbo.flush\n");
> +
> +	illegal_insn = false;
> +	cbo_inval(&mem[0]);
> +	ksft_test_result(illegal_insn, "No cbo.inval\n");
> +}
> +
[...]
> --
> 2.41.0
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

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

* Re: [PATCH v3 6/6] RISC-V: selftests: Add CBO tests
  2023-09-05 14:36   ` Wang, Xiao W
@ 2023-09-05 16:10     ` Andrew Jones
  2023-09-05 20:18       ` Andrew Jones
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Jones @ 2023-09-05 16:10 UTC (permalink / raw)
  To: Wang, Xiao W
  Cc: linux-riscv@lists.infradead.org, paul.walmsley@sifive.com,
	palmer@dabbelt.com, aou@eecs.berkeley.edu, evan@rivosinc.com,
	conor.dooley@microchip.com, apatel@ventanamicro.com

On Tue, Sep 05, 2023 at 02:36:44PM +0000, Wang, Xiao W wrote:
> Hi,
> 
> > -----Original Message-----
> > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of
> > Andrew Jones
> > Sent: Tuesday, September 5, 2023 1:02 AM
> > To: linux-riscv@lists.infradead.org
> > Cc: paul.walmsley@sifive.com; palmer@dabbelt.com;
> > aou@eecs.berkeley.edu; evan@rivosinc.com; conor.dooley@microchip.com;
> > apatel@ventanamicro.com
> > Subject: [PATCH v3 6/6] RISC-V: selftests: Add CBO tests
> > 
> > Add hwprobe test for Zicboz and its block size. Also, when Zicboz is
> > present, test that cbo.zero may be issued and works. Additionally
> > test that the Zicbom instructions cause SIGILL and also that cbo.zero
> > causes SIGILL when Zicboz is not present. Pinning the test to a subset
> > of cpus with taskset will also restrict the hwprobe calls to that set.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  .../testing/selftests/riscv/hwprobe/Makefile  |   7 +-
> >  tools/testing/selftests/riscv/hwprobe/cbo.c   | 170 ++++++++++++++++++
> >  .../testing/selftests/riscv/hwprobe/hwprobe.c |  12 +-
> >  .../testing/selftests/riscv/hwprobe/hwprobe.h |  15 ++
> >  4 files changed, 192 insertions(+), 12 deletions(-)
> >  create mode 100644 tools/testing/selftests/riscv/hwprobe/cbo.c
> >  create mode 100644 tools/testing/selftests/riscv/hwprobe/hwprobe.h
> > 
> > diff --git a/tools/testing/selftests/riscv/hwprobe/Makefile
> > b/tools/testing/selftests/riscv/hwprobe/Makefile
> > index 5f614c3ba598..f224b84591fb 100644
> > --- a/tools/testing/selftests/riscv/hwprobe/Makefile
> > +++ b/tools/testing/selftests/riscv/hwprobe/Makefile
> > @@ -2,9 +2,14 @@
> >  # Copyright (C) 2021 ARM Limited
> >  # Originally tools/testing/arm64/abi/Makefile
> > 
> > -TEST_GEN_PROGS := hwprobe
> > +CFLAGS += -I$(top_srcdir)/tools/include
> > +
> > +TEST_GEN_PROGS := hwprobe cbo
> > 
> >  include ../../lib.mk
> > 
> >  $(OUTPUT)/hwprobe: hwprobe.c sys_hwprobe.S
> >  	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
> > +
> > +$(OUTPUT)/cbo: cbo.c sys_hwprobe.S
> > +	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
> > diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c
> > b/tools/testing/selftests/riscv/hwprobe/cbo.c
> > new file mode 100644
> > index 000000000000..50e85f31a2c7
> > --- /dev/null
> > +++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2023 Ventana Micro Systems Inc.
> > + *
> > + * Run with 'taskset -c <cpu-list> cbo' to only execute hwprobe on a
> > + * subset of cpus, as well as only executing the tests on those cpus.
> > + */
> 
> Patch 3/6 exposes a common subset extensions of the cpu list to user space, so, if some of the cores support ZICBOZ while other don't, hwprobe() won't expose ZICBOZ ext, but in this case the cbo_zero insn might run w/o illegal insn exception when the task is scheduled to run on a core that support ZICBOZ. The test result may vary for each run for this case.
> Maybe we can add some comment for this special test case?

Ah, you're right, and I'm afraid a comment isn't sufficient, since testers
may not have the code handy nor think to check for comments before
reporting issues. The test code needs to handle this case, at least by
outputting helpful directions to the tester when the case is detected.

It'd be nice to have a hwprobe variant which takes an extension as input
and returns a cpu set describing each cpu which supports the extension.
Instead, I'll create an inefficient function which does that, based on
multiple hwprobe calls. Then, on a mixed set, the test will complain and
skip, informing the tester to taskset a subset of cpus which are
consistent wrt the extension.

> 
> > +#define _GNU_SOURCE
> > +#include <stdbool.h>
> > +#include <stdint.h>
> > +#include <sched.h>
> > +#include <signal.h>
> > +#include <assert.h>
> > +#include <linux/compiler.h>
> > +#include <asm/ucontext.h>
> > +
> > +#include "hwprobe.h"
> > +#include "../../kselftest.h"
> > +
> > +#define MK_CBO(fn) ((fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15)
> > +
> > +static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 };
> > +
> > +static bool illegal_insn;
> > +
> > +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]));
> 
> 
> The byte order of insn should always be little endian, while the CPU may be a big-endian one, then the check might fail.
> Maybe we can use __le32_to_cpu(insn) to convert it before the check.

Sounds good (minus the leading __, since we don't have __le32_to_cpu() in
tools).

Thanks,
drew

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

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

* Re: [PATCH v3 6/6] RISC-V: selftests: Add CBO tests
  2023-09-05 16:10     ` Andrew Jones
@ 2023-09-05 20:18       ` Andrew Jones
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-05 20:18 UTC (permalink / raw)
  To: Wang, Xiao W
  Cc: linux-riscv@lists.infradead.org, paul.walmsley@sifive.com,
	palmer@dabbelt.com, aou@eecs.berkeley.edu, evan@rivosinc.com,
	conor.dooley@microchip.com, apatel@ventanamicro.com

On Tue, Sep 05, 2023 at 06:10:25PM +0200, Andrew Jones wrote:
> On Tue, Sep 05, 2023 at 02:36:44PM +0000, Wang, Xiao W wrote:
> > Hi,
> > 
> > > -----Original Message-----
> > > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of
> > > Andrew Jones
...
> > > +#define MK_CBO(fn) ((fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15)
> > > +
> > > +static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 };
> > > +
> > > +static bool illegal_insn;
> > > +
> > > +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]));
> > 
> > 
> > The byte order of insn should always be little endian, while the CPU may be a big-endian one, then the check might fail.
> > Maybe we can use __le32_to_cpu(insn) to convert it before the check.
> 
> Sounds good (minus the leading __, since we don't have __le32_to_cpu() in
> tools).
>

Actually, don't I also need to ensure byte order when creating the
instruction in cbo_insn() with ".4byte %2" where %2 is insn which
is MK_CBO(fn)? I presume so, which means the proper fix would be
to add a cpu_to_le32() to MK_CBO(), i.e.

 #define MK_CBO(fn) cpu_to_le32((fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15)

Unless somebody tells me otherwise, then I'll do that for v4.

Thanks,
drew

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

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

* Re: [PATCH v3 6/6] RISC-V: selftests: Add CBO tests
  2023-09-04 17:02 ` [PATCH v3 6/6] RISC-V: selftests: Add CBO tests Andrew Jones
  2023-09-05 14:36   ` Wang, Xiao W
@ 2023-09-18 10:41   ` Andrew Jones
  1 sibling, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2023-09-18 10:41 UTC (permalink / raw)
  To: linux-riscv; +Cc: paul.walmsley, palmer, aou, evan, conor.dooley, apatel

On Mon, Sep 04, 2023 at 07:02:27PM +0200, Andrew Jones wrote:
> Add hwprobe test for Zicboz and its block size. Also, when Zicboz is
> present, test that cbo.zero may be issued and works. Additionally
> test that the Zicbom instructions cause SIGILL and also that cbo.zero
> causes SIGILL when Zicboz is not present. Pinning the test to a subset
> of cpus with taskset will also restrict the hwprobe calls to that set.

Palmer pointed out that there's no guarantee to get illegal-instruction
exceptions when Zicbom/Zicboz are not present. Indeed, the unpriv spec
states

"""
 The behavior upon decoding a reserved instruction is UNSPECIFIED.

   Some platforms may require that opcodes reserved for standard use raise
   an illegal-instruction exception. Other platforms may permit reserved
   opcode space be used for non-conforming extensions.
"""

This implies that cannot do the illegal-instruction checks for when
hwprobe reports the extension isn't present. That's a pity, but I
guess I should rip it out. Or, maybe I'll keep it, but only run it
under a command line switch.

Thanks,
drew

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

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

end of thread, other threads:[~2023-09-18 10:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-04 17:02 [PATCH v3 0/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
2023-09-04 17:02 ` [PATCH v3 1/6] RISC-V: Make zicbom/zicboz errors consistent Andrew Jones
2023-09-04 17:02 ` [PATCH v3 2/6] RISC-V: Enable cbo.zero in usermode Andrew Jones
2023-09-04 17:02 ` [PATCH v3 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size Andrew Jones
2023-09-04 17:02 ` [PATCH v3 4/6] RISC-V: selftests: Statically link hwprobe test Andrew Jones
2023-09-04 17:02 ` [PATCH v3 5/6] RISC-V: selftests: Convert hwprobe test to kselftest API Andrew Jones
2023-09-04 17:02 ` [PATCH v3 6/6] RISC-V: selftests: Add CBO tests Andrew Jones
2023-09-05 14:36   ` Wang, Xiao W
2023-09-05 16:10     ` Andrew Jones
2023-09-05 20:18       ` Andrew Jones
2023-09-18 10:41   ` Andrew Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).