core-asm-riscv.h: support riscv zicboz ext cbo.zero instruction

[woodrow-shen]

Zicboz is part of RISC-V CMO extension to support cache block based instructions to manipulate clean, flush, inval, and zero etc. So far only cbo.zero can be used for userspace, and this should provide a basic capability to DCache.

[ColinIanKing]

Thanks for this commit, just a couple of minor things:

1. The #defines named below that have been added to core-arch.h are best to be added to core-asm-riscv.h if possible

define STRESS_ZICBOZ_CBO_ZERO (4)

define STRESS_ZICBOZ_RS1 (10)

define STRESS_ZICBOZ_FUNCT3 (2)

define STRESS_ZICBOZ_OPCODE (15)

2. The inline function `stress_asm_riscv_cbo_zero() `in core-asm-riscv.h uses the `CBO_INSN` macro, perhaps it's better just to put the inlined assembler of the `CBO_INSN` macro into the inlined function and remove the `CBO_INSN` macro.

3. The inlined assembler should be `__asm__ __volatile__( )` instead of `asm volatile()`

4. I see the inlined function `stress_asm_riscv_cbo_zero` has been added but it's not being used, are there further commits coming along to use this function?

thanks!  Colin

[woodrow-shen]

Hi @ColinIanKing,

Regarding 1 to 3., I've updated the code for reviewing. I also concern about if the similar case from kernel selftest (https://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git/commit/tools/testing/selftests/riscv/hwprobe?h=for-next&id=0de65288d75ff96c30e216557d979fb9342c4323) would be hit on stress-ng, but so far I didn't see error on cross-compile.

Yeap, I would like to discuss further commit that should introduce stress_asm_riscv_cbo_zero in stress-cache and what it does is only clear relevant cache blocks with zero, not flushing/invalidate them at all. I'm thinking if it's really straightforward to inject the code inside function stress_cache

diff --git a/stress-cache.c b/stress-cache.c
index 8a77218ad..b58c141ca 100644
--- a/stress-cache.c
+++ b/stress-cache.c
@@ -19,6 +19,7 @@
  */
 #include "stress-ng.h"
 #include "core-asm-x86.h"
+#include "core-asm-riscv.h"
 #include "core-builtin.h"
 #include "core-cpu-cache.h"
 #include "core-put.h"
@@ -960,6 +961,31 @@ static int stress_cache(stress_args_t *args)
 #endif
                (void)shim_cacheflush((char *)stress_cache, 8192, SHIM_ICACHE);
                (void)shim_cacheflush((char *)buffer, (int)buffer_size, SHIM_DCACHE);
+#if defined(HAVE_ASM_RISCV_CBO_ZERO)   \
+    defined(__NR_riscv_hwprobe)
+               {
+                       int ret;
+                       struct riscv_hwprobe pair;
+                       cpu_set_t cpus;
+
+                       ret = sched_getaffinity(0, sizeof(cpu_set_t), &cpus);
+
+                       pair.key = RISCV_HWPROBE_KEY_IMA_EXT_0;
+                       ret = (int)syscall(__NR_riscv_hwprobe, &pair, 1, sizeof(cpu_set_t), &cpus, 0);
+
+                       if (pair.value & RISCV_HWPROBE_EXT_ZICBOZ) {
+                               uint64_t block_size;
+                               pair.key = RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE;
+
+                               ret = (int)syscall(__NR_riscv_hwprobe, &pair, 1, sizeof(cpu_set_t), &cpus, 0);
+                               block_size = pair.value;
+
+                               for (int i = 0; i < (int)buffer_size / block_size; ++i) {
+                                       (void)stress_asm_riscv_cbo_zero((char *)&buffer[i * block_size]);
+                               }
+                       }
+               }
+#endif

Above is just rough testing the current buffer used, so I should wait for your comment before moving on. Thank you. Woodrow

[ColinIanKing]

Commit applied and pushed:

commit 1af186230cb777af44b96e4369db495506773beb Author: Hsieh-Tseng Shen woodrow.shen@sifive.com Date: Thu May 30 03:43:22 2024 -0700

core-asm-riscv.h: support riscv zicboz ext cbo.zero instruction

Thank you for your contribution to stress-ng

[ColinIanKing]

Hi,

The added code to stress-cache.c looks OK to me, but I would prefer if that code is put into static inlined function that is called from inside the do-while loop just so that main do-while loop is less cluttered.

Also, the for (int i = 0; i < (int)buffer_size / block_size; ++i) should be re-worked to match the stress-ng coding style and to remove some expensive division and multiplications:

replace:

         for (int i = 0; i < (int)buffer_size / block_size; ++i) {
                (void)stress_asm_riscv_cbo_zero((char *)&buffer[i * block_size]);
         }

with something like this:

        register uint8_t *ptr;
        const uint8_t *buffer_end = buffer + buffer_size;

        for (ptr = buffer; ptr < buffer_end; ptr += block_size)
                (void)stress_asm_riscv_cbo_zero((char *)ptr);

[ColinIanKing]

The amended patch has been applied and pushed:

commit 1af186230cb777af44b96e4369db495506773beb Author: Hsieh-Tseng Shen woodrow.shen@sifive.com Date: Thu May 30 03:43:22 2024 -0700