linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
@ 2022-10-27 13:02 Andrew Jones
  2022-10-27 13:02 ` [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop Andrew Jones
                   ` (11 more replies)
  0 siblings, 12 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

When the Zicboz extension is available we can more rapidly zero naturally
aligned Zicboz block sized chunks of memory. As pages are always page
aligned and are larger than any Zicboz block size will be, then
clear_page() appears to be a good candidate for the extension. While cycle
count and energy consumption should also be considered, we can be pretty
certain that implementing clear_page() with the Zicboz extension is a win
by comparing the new dynamic instruction count with its current count[1].
Doing so we see that the new count is less than half the old count (see
patch4's commit message for more details). Another candidate for the
extension is memset(), but, since memset() isn't just used for zeroing
memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
it's not as obvious if adding support for Zicboz will be an overall win.
In order to make a determination, I've done some analysis and wrote my
conclusions in the bullets below.

* When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
  doesn't change, so that's fine.

* The overhead added to memset() when the Zicboz extension isn't present,
  but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
  which I believe is fine.

* The overhead added to a memset() invocation which is not zeroing memory
  is 7 instructions, where 3 are branches. This seems fine and,
  furthermore, memset() is almost always invoked to zero memory (99% [2]).

* When memset() is invoked to zero memory, the proposed Zicboz extended
  memset() always has a lower dynamic instruction count than the current
  memset() as long as the input address is Zicboz block aligned and the
  length is >= the block size.

* When memset() is invoked to zero memory, the proposed Zicboz extended
  memset() is always worse for unaligned or too small inputs than the
  current memset(), but it's only at most a few dozen instructions worse.
  I think this is probably fine, especially considering the large majority
  of zeroing invocations are 64 bytes or larger and are aligned to a
  power-of-2 boundary, 64-byte or larger (77% [2]).

[1] I ported the functions under test to userspace and linked them with
    a test program. Then, I ran them under gdb with a script[3] which
    counted instructions by single stepping.

[2] I wrote bpftrace scripts[4] to count memset() invocations to see the
    frequency of it being used to zero memory and have block size aligned
    input addresses with block size or larger lengths. The workload was
    just random desktop stuff including streaming video and compiling.
    While I did run this on my x86 notebook, I still expect the data to
    be representative on RISC-V. Note, x86 has clear_page() so the
    memset() data regarding alignment and size weren't over inflated by
    page zeroing invocations. Grepping also shows the large majority of
    memset() calls are to zero memory (93%).

[3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
[4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe

These patches are based on the following pending series

1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
   20221024091309.406906-1-ajones@ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
   20221006070818.3616-1-jszhang@kernel.org
   Also including the additional patch proposed here
   20221013162038.ehseju2neic2xu5z@kamzik

The patches are also available here
https://github.com/jones-drew/linux/commits/riscv/zicboz

To test over QEMU this branch may be used to enable Zicboz
https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz

To test running a KVM guest with Zicboz this kvmtool branch may be used
https://github.com/jones-drew/kvmtool/commits/riscv/zicboz

Thanks,
drew

Andrew Jones (9):
  RISC-V: Factor out body of riscv_init_cbom_blocksize loop
  RISC-V: Add Zicboz detection and block size parsing
  RISC-V: insn-def: Define cbo.zero
  RISC-V: Use Zicboz in clear_page when available
  RISC-V: KVM: Provide UAPI for Zicboz block size
  RISC-V: KVM: Expose Zicboz to the guest
  RISC-V: lib: Improve memset assembler formatting
  RISC-V: lib: Use named labels in memset
  RISC-V: Use Zicboz in memset when available

 arch/riscv/Kconfig                  |  13 ++
 arch/riscv/include/asm/cacheflush.h |   3 +-
 arch/riscv/include/asm/hwcap.h      |   1 +
 arch/riscv/include/asm/insn-def.h   |  50 ++++++
 arch/riscv/include/asm/page.h       |   6 +-
 arch/riscv/include/uapi/asm/kvm.h   |   2 +
 arch/riscv/kernel/cpu.c             |   1 +
 arch/riscv/kernel/cpufeature.c      |  10 ++
 arch/riscv/kernel/setup.c           |   2 +-
 arch/riscv/kvm/vcpu.c               |  11 ++
 arch/riscv/lib/Makefile             |   1 +
 arch/riscv/lib/clear_page.S         |  28 ++++
 arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
 arch/riscv/mm/cacheflush.c          |  64 +++++---
 14 files changed, 325 insertions(+), 108 deletions(-)
 create mode 100644 arch/riscv/lib/clear_page.S

-- 
2.37.3


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

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

* [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-27 14:58   ` Heiko Stübner
  2022-10-30 20:31   ` Conor Dooley
  2022-10-27 13:02 ` [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing Andrew Jones
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

Refactor riscv_init_cbom_blocksize() to prepare for it to be used
for both cbom block size and cboz block size.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/mm/cacheflush.c | 45 +++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
index 57b40a350420..f096b9966cae 100644
--- a/arch/riscv/mm/cacheflush.c
+++ b/arch/riscv/mm/cacheflush.c
@@ -91,34 +91,39 @@ void flush_icache_pte(pte_t pte)
 unsigned int riscv_cbom_block_size;
 EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
 
+static void cbo_get_block_size(struct device_node *node,
+			       const char *name, u32 *blksz,
+			       unsigned long *first_hartid)
+{
+	unsigned long hartid;
+	u32 val;
+
+	if (riscv_of_processor_hartid(node, &hartid))
+		return;
+
+	if (of_property_read_u32(node, name, &val))
+		return;
+
+	if (!*blksz) {
+		*blksz = val;
+		*first_hartid = hartid;
+	} else if (*blksz != val) {
+		pr_warn("%s mismatched between harts %lu and %lu\n",
+			name, *first_hartid, hartid);
+	}
+}
+
 void riscv_init_cbom_blocksize(void)
 {
 	struct device_node *node;
 	unsigned long cbom_hartid;
-	u32 val, probed_block_size;
-	int ret;
+	u32 probed_block_size;
 
 	probed_block_size = 0;
 	for_each_of_cpu_node(node) {
-		unsigned long hartid;
-
-		ret = riscv_of_processor_hartid(node, &hartid);
-		if (ret)
-			continue;
-
 		/* set block-size for cbom extension if available */
-		ret = of_property_read_u32(node, "riscv,cbom-block-size", &val);
-		if (ret)
-			continue;
-
-		if (!probed_block_size) {
-			probed_block_size = val;
-			cbom_hartid = hartid;
-		} else {
-			if (probed_block_size != val)
-				pr_warn("cbom-block-size mismatched between harts %lu and %lu\n",
-					cbom_hartid, hartid);
-		}
+		cbo_get_block_size(node, "riscv,cbom-block-size",
+				   &probed_block_size, &cbom_hartid);
 	}
 
 	if (probed_block_size)
-- 
2.37.3


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

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

* [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
  2022-10-27 13:02 ` [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-27 15:03   ` Heiko Stübner
  2022-10-30 20:47   ` Conor Dooley
  2022-10-27 13:02 ` [PATCH 3/9] RISC-V: insn-def: Define cbo.zero Andrew Jones
                   ` (9 subsequent siblings)
  11 siblings, 2 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

Mostly follow the same pattern as Zicbom, but leave out the toolchain
checks as we plan to use the insn-def framework for the cbo.zero
instruction.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/Kconfig                  | 13 +++++++++++++
 arch/riscv/include/asm/cacheflush.h |  3 ++-
 arch/riscv/include/asm/hwcap.h      |  1 +
 arch/riscv/kernel/cpu.c             |  1 +
 arch/riscv/kernel/cpufeature.c      | 10 ++++++++++
 arch/riscv/kernel/setup.c           |  2 +-
 arch/riscv/mm/cacheflush.c          | 23 +++++++++++++++--------
 7 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 6b48a3ae9843..c20e6fa0c0b1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -433,6 +433,19 @@ config RISCV_ISA_ZICBOM
 
 	   If you don't know what to do here, say Y.
 
+config RISCV_ISA_ZICBOZ
+	bool "Zicboz extension support for faster zeroing of memory"
+	depends on !XIP_KERNEL && MMU
+	select RISCV_ALTERNATIVE
+	default y
+	help
+	   Adds support to dynamically detect the presence of the ZICBOZ
+	   extension (cbo.zero instruction) and enable its usage.
+
+	   The Zicboz extension is used for faster zeroing of memory.
+
+	   If you don't know what to do here, say Y.
+
 config FPU
 	bool "FPU support"
 	default y
diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
index f6fbe7042f1c..5b31568cf5e6 100644
--- a/arch/riscv/include/asm/cacheflush.h
+++ b/arch/riscv/include/asm/cacheflush.h
@@ -43,7 +43,8 @@ void flush_icache_mm(struct mm_struct *mm, bool local);
 #endif /* CONFIG_SMP */
 
 extern unsigned int riscv_cbom_block_size;
-void riscv_init_cbom_blocksize(void);
+extern unsigned int riscv_cboz_block_size;
+void riscv_init_cbo_blocksizes(void);
 
 #ifdef CONFIG_RISCV_DMA_NONCOHERENT
 void riscv_noncoherent_supported(void);
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 5d6492bde446..eaa5a972ad2d 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -45,6 +45,7 @@
 #define RISCV_ISA_EXT_ZIHINTPAUSE	29
 #define RISCV_ISA_EXT_SSTC		30
 #define RISCV_ISA_EXT_SVINVAL		31
+#define RISCV_ISA_EXT_ZICBOZ		32
 
 #define RISCV_ISA_EXT_ID_MAX		RISCV_ISA_EXT_MAX
 
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index fa427bdcf773..bf969218f609 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -144,6 +144,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
 	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
 	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
 	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
+	__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
 	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
 };
 
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 18b9ed4df1f4..e13b3391de76 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -78,6 +78,15 @@ static bool riscv_isa_extension_check(int id)
 			return false;
 		}
 		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");
+			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");
+			return false;
+		}
+		return true;
 	}
 
 	return true;
@@ -225,6 +234,7 @@ void __init riscv_fill_hwcap(void)
 				SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
 				SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
 				SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
+				SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
 			}
 #undef SET_ISA_EXT_MAP
 		}
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index a07917551027..26de0d8fd23d 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -296,7 +296,7 @@ void __init setup_arch(char **cmdline_p)
 	setup_smp();
 #endif
 
-	riscv_init_cbom_blocksize();
+	riscv_init_cbo_blocksizes();
 	riscv_fill_hwcap();
 	apply_boot_alternatives();
 #ifdef CONFIG_RISCV_DMA_NONCOHERENT
diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
index f096b9966cae..208e0d58bde3 100644
--- a/arch/riscv/mm/cacheflush.c
+++ b/arch/riscv/mm/cacheflush.c
@@ -91,6 +91,9 @@ void flush_icache_pte(pte_t pte)
 unsigned int riscv_cbom_block_size;
 EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
 
+unsigned int riscv_cboz_block_size;
+EXPORT_SYMBOL_GPL(riscv_cboz_block_size);
+
 static void cbo_get_block_size(struct device_node *node,
 			       const char *name, u32 *blksz,
 			       unsigned long *first_hartid)
@@ -113,19 +116,23 @@ static void cbo_get_block_size(struct device_node *node,
 	}
 }
 
-void riscv_init_cbom_blocksize(void)
+void riscv_init_cbo_blocksizes(void)
 {
+	unsigned long cbom_hartid, cboz_hartid;
+	u32 cbom_blksz = 0, cboz_blksz = 0;
 	struct device_node *node;
-	unsigned long cbom_hartid;
-	u32 probed_block_size;
 
-	probed_block_size = 0;
 	for_each_of_cpu_node(node) {
-		/* set block-size for cbom extension if available */
+		/* set block-size for cbom and/or cboz extension if available */
 		cbo_get_block_size(node, "riscv,cbom-block-size",
-				   &probed_block_size, &cbom_hartid);
+				   &cbom_blksz, &cbom_hartid);
+		cbo_get_block_size(node, "riscv,cboz-block-size",
+				   &cboz_blksz, &cboz_hartid);
 	}
 
-	if (probed_block_size)
-		riscv_cbom_block_size = probed_block_size;
+	if (cbom_blksz)
+		riscv_cbom_block_size = cbom_blksz;
+
+	if (cboz_blksz)
+		riscv_cboz_block_size = cboz_blksz;
 }
-- 
2.37.3


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

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

* [PATCH 3/9] RISC-V: insn-def: Define cbo.zero
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
  2022-10-27 13:02 ` [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop Andrew Jones
  2022-10-27 13:02 ` [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-27 15:37   ` Heiko Stübner
  2022-10-30 21:08   ` Conor Dooley
  2022-10-27 13:02 ` [PATCH 4/9] RISC-V: Use Zicboz in clear_page when available Andrew Jones
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

CBO instructions use the I-type of instruction format where
the immediate is used to identify the CBO instruction type.
Add I-type instruction encoding support to insn-def and also
add cbo.zero.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/asm/insn-def.h | 50 +++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
index 16044affa57c..f13054716556 100644
--- a/arch/riscv/include/asm/insn-def.h
+++ b/arch/riscv/include/asm/insn-def.h
@@ -12,6 +12,12 @@
 #define INSN_R_RD_SHIFT			 7
 #define INSN_R_OPCODE_SHIFT		 0
 
+#define INSN_I_SIMM12_SHIFT		20
+#define INSN_I_RS1_SHIFT		15
+#define INSN_I_FUNC3_SHIFT		12
+#define INSN_I_RD_SHIFT			 7
+#define INSN_I_OPCODE_SHIFT		 0
+
 #ifdef __ASSEMBLY__
 
 #ifdef CONFIG_AS_HAS_INSN
@@ -20,6 +26,10 @@
 	.insn	r \opcode, \func3, \func7, \rd, \rs1, \rs2
 	.endm
 
+	.macro insn_i, opcode, func3, rd, rs1, simm12
+	.insn	i \opcode, \func3, \rd, \rs1, \simm12
+	.endm
+
 #else
 
 #include <asm/gpr-num.h>
@@ -33,9 +43,18 @@
 		 (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
 	.endm
 
+	.macro insn_i, opcode, func3, rd, rs1, simm12
+	.4byte	((\opcode << INSN_I_OPCODE_SHIFT) |		\
+		 (\func3 << INSN_I_FUNC3_SHIFT) |		\
+		 (.L__gpr_num_\rd << INSN_I_RD_SHIFT) |		\
+		 (.L__gpr_num_\rs1 << INSN_I_RS1_SHIFT) |	\
+		 (\simm12 << INSN_I_SIMM12_SHIFT))
+	.endm
+
 #endif
 
 #define __INSN_R(...)	insn_r __VA_ARGS__
+#define __INSN_I(...)	insn_i __VA_ARGS__
 
 #else /* ! __ASSEMBLY__ */
 
@@ -44,6 +63,9 @@
 #define __INSN_R(opcode, func3, func7, rd, rs1, rs2)	\
 	".insn	r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
 
+#define __INSN_I(opcode, func3, rd, rs1, simm12)	\
+	".insn	i " opcode ", " func3 ", " rd ", " rs1 ", " simm12 "\n"
+
 #else
 
 #include <linux/stringify.h>
@@ -60,14 +82,32 @@
 "		 (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
 "	.endm\n"
 
+#define DEFINE_INSN_I							\
+	__DEFINE_ASM_GPR_NUMS						\
+"	.macro insn_i, opcode, func3, rd, rs1, simm12\n"		\
+"	.4byte	((\\opcode << " __stringify(INSN_I_OPCODE_SHIFT) ") |"	\
+"		 (\\func3 << " __stringify(INSN_I_FUNC3_SHIFT) ") |"	\
+"		 (.L__gpr_num_\\rd << " __stringify(INSN_I_RD_SHIFT) ") |"   \
+"		 (.L__gpr_num_\\rs1 << " __stringify(INSN_I_RS1_SHIFT) ") |" \
+"		 (\\simm12 << " __stringify(INSN_I_SIMM12_SHIFT) "))\n"	\
+"	.endm\n"
+
 #define UNDEFINE_INSN_R							\
 "	.purgem insn_r\n"
 
+#define UNDEFINE_INSN_I							\
+"	.purgem insn_i\n"
+
 #define __INSN_R(opcode, func3, func7, rd, rs1, rs2)			\
 	DEFINE_INSN_R							\
 	"insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
 	UNDEFINE_INSN_R
 
+#define __INSN_I(opcode, func3, rd, rs1, simm12)			\
+	DEFINE_INSN_I							\
+	"insn_i " opcode ", " func3 ", " rd ", " rs1 ", " simm12 "\n" \
+	UNDEFINE_INSN_I
+
 #endif
 
 #endif /* ! __ASSEMBLY__ */
@@ -76,9 +116,14 @@
 	__INSN_R(RV_##opcode, RV_##func3, RV_##func7,		\
 		 RV_##rd, RV_##rs1, RV_##rs2)
 
+#define INSN_I(opcode, func3, rd, rs1, simm12)			\
+	__INSN_I(RV_##opcode, RV_##func3, RV_##rd,		\
+		 RV_##rs1, RV_##simm12)
+
 #define RV_OPCODE(v)		__ASM_STR(v)
 #define RV_FUNC3(v)		__ASM_STR(v)
 #define RV_FUNC7(v)		__ASM_STR(v)
+#define RV_SIMM12(v)		__ASM_STR(v)
 #define RV_RD(v)		__ASM_STR(v)
 #define RV_RS1(v)		__ASM_STR(v)
 #define RV_RS2(v)		__ASM_STR(v)
@@ -87,6 +132,7 @@
 #define RV___RS1(v)		__RV_REG(v)
 #define RV___RS2(v)		__RV_REG(v)
 
+#define RV_OPCODE_MISC_MEM	RV_OPCODE(15)
 #define RV_OPCODE_SYSTEM	RV_OPCODE(115)
 
 #define HFENCE_VVMA(vaddr, asid)				\
@@ -134,4 +180,8 @@
 	INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(51),		\
 	       __RD(0), RS1(gaddr), RS2(vmid))
 
+#define CBO_ZERO(base)						\
+	INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0),		\
+	       RS1(base), SIMM12(4))
+
 #endif /* __ASM_INSN_DEF_H */
-- 
2.37.3


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

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

* [PATCH 4/9] RISC-V: Use Zicboz in clear_page when available
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (2 preceding siblings ...)
  2022-10-27 13:02 ` [PATCH 3/9] RISC-V: insn-def: Define cbo.zero Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-27 13:02 ` [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size Andrew Jones
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

Using memset() to zero a 4K page takes 563 total instructions
where 20 are branches. clear_page() with Zicboz takes 198 total
instructions where 64 are branches. We could reduce the number
branches by unrolling, but since the cboz block size isn't fixed,
cbo.zero doesn't take an offset, and even PAGE_SIZE doesn't have
to be 4K forever, we'd end up implementing a Duff device where
each unrolled block would not only contain a cbo.zero instruction,
but also an add to update the base address. So, for now, it seems
the simple tight loop approach is better. At least we don't have
to worry as much about potential icache misses as unrolled loops
do. Of course as hardware becomes available we can experiment
with unrolling too.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/asm/page.h |  6 +++++-
 arch/riscv/lib/Makefile       |  1 +
 arch/riscv/lib/clear_page.S   | 28 ++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/lib/clear_page.S

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index ac70b0fd9a9a..a86d6d8a9ca0 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -49,10 +49,14 @@
 
 #ifndef __ASSEMBLY__
 
+#ifdef CONFIG_RISCV_ISA_ZICBOZ
+void clear_page(void *page);
+#else
 #define clear_page(pgaddr)			memset((pgaddr), 0, PAGE_SIZE)
+#endif
 #define copy_page(to, from)			memcpy((to), (from), PAGE_SIZE)
 
-#define clear_user_page(pgaddr, vaddr, page)	memset((pgaddr), 0, PAGE_SIZE)
+#define clear_user_page(pgaddr, vaddr, page)	clear_page(pgaddr)
 #define copy_user_page(vto, vfrom, vaddr, topg) \
 			memcpy((vto), (vfrom), PAGE_SIZE)
 
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index 25d5c9664e57..9ee5e2ab5143 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -5,5 +5,6 @@ lib-y			+= memset.o
 lib-y			+= memmove.o
 lib-$(CONFIG_MMU)	+= uaccess.o
 lib-$(CONFIG_64BIT)	+= tishift.o
+lib-$(CONFIG_RISCV_ISA_ZICBOZ)	+= clear_page.o
 
 obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
diff --git a/arch/riscv/lib/clear_page.S b/arch/riscv/lib/clear_page.S
new file mode 100644
index 000000000000..cafa24a918d6
--- /dev/null
+++ b/arch/riscv/lib/clear_page.S
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <linux/linkage.h>
+#include <asm/asm.h>
+#include <asm/alternative-macros.h>
+#include <asm/hwcap.h>
+#include <asm/insn-def.h>
+#include <asm/page.h>
+
+/* void clear_page(void *page) */
+ENTRY(__clear_page)
+WEAK(clear_page)
+	li	a2, PAGE_SIZE
+	ALTERNATIVE("j .Lno_zicboz", "nop",
+		    0, RISCV_ISA_EXT_ZICBOZ, CONFIG_RISCV_ISA_ZICBOZ)
+	la	a3, riscv_cboz_block_size
+	lw	a1, 0(a3)
+	add	a2, a0, a2
+.Lzero_loop:
+	CBO_ZERO(a0)
+	add	a0, a0, a1
+	bltu	a0, a2, .Lzero_loop
+	ret
+.Lno_zicboz:
+	li	a1, 0
+	la	a3, __memset
+	jr	a3
+END(__clear_page)
-- 
2.37.3


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

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

* [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (3 preceding siblings ...)
  2022-10-27 13:02 ` [PATCH 4/9] RISC-V: Use Zicboz in clear_page when available Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-30 21:23   ` Conor Dooley
  2022-11-27  5:37   ` Anup Patel
  2022-10-27 13:02 ` [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest Andrew Jones
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

We're about to allow guests to use the Zicboz extension. KVM
userspace needs to know the cache block size in order to
properly advertise it to the guest. Provide a virtual config
register for userspace to get it with the GET_ONE_REG API, but
setting it cannot be supported, so disallow SET_ONE_REG.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/uapi/asm/kvm.h | 1 +
 arch/riscv/kvm/vcpu.c             | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index 8985ff234c01..4bbf55cb2b70 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -49,6 +49,7 @@ struct kvm_sregs {
 struct kvm_riscv_config {
 	unsigned long isa;
 	unsigned long zicbom_block_size;
+	unsigned long zicboz_block_size;
 };
 
 /* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 71ebbc4821f0..18a739070b51 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -270,6 +270,11 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
 			return -EINVAL;
 		reg_val = riscv_cbom_block_size;
 		break;
+	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
+		if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ))
+			return -EINVAL;
+		reg_val = riscv_cboz_block_size;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -329,6 +334,8 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
 		break;
 	case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
 		return -EOPNOTSUPP;
+	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
+		return -EOPNOTSUPP;
 	default:
 		return -EINVAL;
 	}
-- 
2.37.3


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

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

* [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (4 preceding siblings ...)
  2022-10-27 13:02 ` [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-30 21:23   ` Conor Dooley
  2022-11-27  5:38   ` Anup Patel
  2022-10-27 13:02 ` [PATCH 7/9] RISC-V: lib: Improve memset assembler formatting Andrew Jones
                   ` (5 subsequent siblings)
  11 siblings, 2 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

Guests may use the cbo.zero instruction when the CPU has the Zicboz
extension and the hypervisor sets henvcfg.CBZE.

Add Zicboz support for KVM guests which may be enabled and
disabled from KVM userspace using the ISA extension ONE_REG API.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/uapi/asm/kvm.h | 1 +
 arch/riscv/kvm/vcpu.c             | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index 4bbf55cb2b70..8dc21ceee7aa 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -103,6 +103,7 @@ enum KVM_RISCV_ISA_EXT_ID {
 	KVM_RISCV_ISA_EXT_SVINVAL,
 	KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
 	KVM_RISCV_ISA_EXT_ZICBOM,
+	KVM_RISCV_ISA_EXT_ZICBOZ,
 	KVM_RISCV_ISA_EXT_MAX,
 };
 
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 18a739070b51..7758faec590a 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -62,6 +62,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
 	KVM_ISA_EXT_ARR(SVPBMT),
 	KVM_ISA_EXT_ARR(ZIHINTPAUSE),
 	KVM_ISA_EXT_ARR(ZICBOM),
+	KVM_ISA_EXT_ARR(ZICBOZ),
 };
 
 static unsigned long kvm_riscv_vcpu_base2isa_ext(unsigned long base_ext)
@@ -821,6 +822,9 @@ static void kvm_riscv_vcpu_update_config(const unsigned long *isa)
 	if (riscv_isa_extension_available(isa, ZICBOM))
 		henvcfg |= (ENVCFG_CBIE | ENVCFG_CBCFE);
 
+	if (riscv_isa_extension_available(isa, ZICBOZ))
+		henvcfg |= ENVCFG_CBZE;
+
 	csr_write(CSR_HENVCFG, henvcfg);
 #ifdef CONFIG_32BIT
 	csr_write(CSR_HENVCFGH, henvcfg >> 32);
-- 
2.37.3


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

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

* [PATCH 7/9] RISC-V: lib: Improve memset assembler formatting
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (5 preceding siblings ...)
  2022-10-27 13:02 ` [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-30 21:27   ` Conor Dooley
  2022-10-27 13:02 ` [PATCH 8/9] RISC-V: lib: Use named labels in memset Andrew Jones
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

Aligning the first operand of each instructions with a tab is a
typical style which improves readability. Apply it to memset.S.
While there, we also make a small grammar change to a comment.

No functional change intended.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/lib/memset.S | 143 ++++++++++++++++++++--------------------
 1 file changed, 72 insertions(+), 71 deletions(-)

diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
index 34c5360c6705..e613c5c27998 100644
--- a/arch/riscv/lib/memset.S
+++ b/arch/riscv/lib/memset.S
@@ -3,111 +3,112 @@
  * Copyright (C) 2013 Regents of the University of California
  */
 
-
 #include <linux/linkage.h>
 #include <asm/asm.h>
 
 /* void *memset(void *, int, size_t) */
 ENTRY(__memset)
 WEAK(memset)
-	move t0, a0  /* Preserve return value */
+	move	t0, a0			/* Preserve return value */
 
 	/* Defer to byte-oriented fill for small sizes */
-	sltiu a3, a2, 16
-	bnez a3, 4f
+	sltiu	a3, a2, 16
+	bnez	a3, 4f
 
 	/*
 	 * Round to nearest XLEN-aligned address
-	 * greater than or equal to start address
+	 * greater than or equal to the start address.
 	 */
-	addi a3, t0, SZREG-1
-	andi a3, a3, ~(SZREG-1)
-	beq a3, t0, 2f  /* Skip if already aligned */
+	addi	a3, t0, SZREG-1
+	andi	a3, a3, ~(SZREG-1)
+	beq	a3, t0, 2f		/* Skip if already aligned */
+
 	/* Handle initial misalignment */
-	sub a4, a3, t0
+	sub	a4, a3, t0
 1:
-	sb a1, 0(t0)
-	addi t0, t0, 1
-	bltu t0, a3, 1b
-	sub a2, a2, a4  /* Update count */
+	sb	a1, 0(t0)
+	addi	t0, t0, 1
+	bltu	t0, a3, 1b
+	sub	a2, a2, a4		/* Update count */
 
 2: /* Duff's device with 32 XLEN stores per iteration */
 	/* Broadcast value into all bytes */
-	andi a1, a1, 0xff
-	slli a3, a1, 8
-	or a1, a3, a1
-	slli a3, a1, 16
-	or a1, a3, a1
+	andi	a1, a1, 0xff
+	slli	a3, a1, 8
+	or	a1, a3, a1
+	slli	a3, a1, 16
+	or	a1, a3, a1
 #ifdef CONFIG_64BIT
-	slli a3, a1, 32
-	or a1, a3, a1
+	slli	a3, a1, 32
+	or	a1, a3, a1
 #endif
 
 	/* Calculate end address */
-	andi a4, a2, ~(SZREG-1)
-	add a3, t0, a4
+	andi	a4, a2, ~(SZREG-1)
+	add	a3, t0, a4
 
-	andi a4, a4, 31*SZREG  /* Calculate remainder */
-	beqz a4, 3f            /* Shortcut if no remainder */
-	neg a4, a4
-	addi a4, a4, 32*SZREG  /* Calculate initial offset */
+	andi	a4, a4, 31*SZREG	/* Calculate remainder */
+	beqz	a4, 3f			/* Shortcut if no remainder */
+	neg	a4, a4
+	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
 
 	/* Adjust start address with offset */
-	sub t0, t0, a4
+	sub	t0, t0, a4
 
 	/* Jump into loop body */
 	/* Assumes 32-bit instruction lengths */
-	la a5, 3f
+	la	a5, 3f
 #ifdef CONFIG_64BIT
-	srli a4, a4, 1
+	srli	a4, a4, 1
 #endif
-	add a5, a5, a4
-	jr a5
+	add	a5, a5, a4
+	jr	a5
 3:
-	REG_S a1,        0(t0)
-	REG_S a1,    SZREG(t0)
-	REG_S a1,  2*SZREG(t0)
-	REG_S a1,  3*SZREG(t0)
-	REG_S a1,  4*SZREG(t0)
-	REG_S a1,  5*SZREG(t0)
-	REG_S a1,  6*SZREG(t0)
-	REG_S a1,  7*SZREG(t0)
-	REG_S a1,  8*SZREG(t0)
-	REG_S a1,  9*SZREG(t0)
-	REG_S a1, 10*SZREG(t0)
-	REG_S a1, 11*SZREG(t0)
-	REG_S a1, 12*SZREG(t0)
-	REG_S a1, 13*SZREG(t0)
-	REG_S a1, 14*SZREG(t0)
-	REG_S a1, 15*SZREG(t0)
-	REG_S a1, 16*SZREG(t0)
-	REG_S a1, 17*SZREG(t0)
-	REG_S a1, 18*SZREG(t0)
-	REG_S a1, 19*SZREG(t0)
-	REG_S a1, 20*SZREG(t0)
-	REG_S a1, 21*SZREG(t0)
-	REG_S a1, 22*SZREG(t0)
-	REG_S a1, 23*SZREG(t0)
-	REG_S a1, 24*SZREG(t0)
-	REG_S a1, 25*SZREG(t0)
-	REG_S a1, 26*SZREG(t0)
-	REG_S a1, 27*SZREG(t0)
-	REG_S a1, 28*SZREG(t0)
-	REG_S a1, 29*SZREG(t0)
-	REG_S a1, 30*SZREG(t0)
-	REG_S a1, 31*SZREG(t0)
-	addi t0, t0, 32*SZREG
-	bltu t0, a3, 3b
-	andi a2, a2, SZREG-1  /* Update count */
+	REG_S	a1,        0(t0)
+	REG_S	a1,    SZREG(t0)
+	REG_S	a1,  2*SZREG(t0)
+	REG_S	a1,  3*SZREG(t0)
+	REG_S	a1,  4*SZREG(t0)
+	REG_S	a1,  5*SZREG(t0)
+	REG_S	a1,  6*SZREG(t0)
+	REG_S	a1,  7*SZREG(t0)
+	REG_S	a1,  8*SZREG(t0)
+	REG_S	a1,  9*SZREG(t0)
+	REG_S	a1, 10*SZREG(t0)
+	REG_S	a1, 11*SZREG(t0)
+	REG_S	a1, 12*SZREG(t0)
+	REG_S	a1, 13*SZREG(t0)
+	REG_S	a1, 14*SZREG(t0)
+	REG_S	a1, 15*SZREG(t0)
+	REG_S	a1, 16*SZREG(t0)
+	REG_S	a1, 17*SZREG(t0)
+	REG_S	a1, 18*SZREG(t0)
+	REG_S	a1, 19*SZREG(t0)
+	REG_S	a1, 20*SZREG(t0)
+	REG_S	a1, 21*SZREG(t0)
+	REG_S	a1, 22*SZREG(t0)
+	REG_S	a1, 23*SZREG(t0)
+	REG_S	a1, 24*SZREG(t0)
+	REG_S	a1, 25*SZREG(t0)
+	REG_S	a1, 26*SZREG(t0)
+	REG_S	a1, 27*SZREG(t0)
+	REG_S	a1, 28*SZREG(t0)
+	REG_S	a1, 29*SZREG(t0)
+	REG_S	a1, 30*SZREG(t0)
+	REG_S	a1, 31*SZREG(t0)
+
+	addi	t0, t0, 32*SZREG
+	bltu	t0, a3, 3b
+	andi	a2, a2, SZREG-1		/* Update count */
 
 4:
 	/* Handle trailing misalignment */
-	beqz a2, 6f
-	add a3, t0, a2
+	beqz	a2, 6f
+	add	a3, t0, a2
 5:
-	sb a1, 0(t0)
-	addi t0, t0, 1
-	bltu t0, a3, 5b
+	sb	a1, 0(t0)
+	addi	t0, t0, 1
+	bltu	t0, a3, 5b
 6:
 	ret
 END(__memset)
-- 
2.37.3


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

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

* [PATCH 8/9] RISC-V: lib: Use named labels in memset
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (6 preceding siblings ...)
  2022-10-27 13:02 ` [PATCH 7/9] RISC-V: lib: Improve memset assembler formatting Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-30 22:15   ` Conor Dooley
  2022-10-27 13:02 ` [PATCH 9/9] RISC-V: Use Zicboz in memset when available Andrew Jones
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

In a coming patch we'll be adding more branch targets. Let's
change the numeric labels to named labels to make it easier
to read and integrate with.

No functional change intended.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
index e613c5c27998..74e4c7feec00 100644
--- a/arch/riscv/lib/memset.S
+++ b/arch/riscv/lib/memset.S
@@ -13,7 +13,7 @@ WEAK(memset)
 
 	/* Defer to byte-oriented fill for small sizes */
 	sltiu	a3, a2, 16
-	bnez	a3, 4f
+	bnez	a3, .Lfinish
 
 	/*
 	 * Round to nearest XLEN-aligned address
@@ -21,17 +21,18 @@ WEAK(memset)
 	 */
 	addi	a3, t0, SZREG-1
 	andi	a3, a3, ~(SZREG-1)
-	beq	a3, t0, 2f		/* Skip if already aligned */
+	beq	a3, t0, .Ldo_duff	/* Skip if already aligned */
 
 	/* Handle initial misalignment */
 	sub	a4, a3, t0
-1:
+.Lmisaligned1:
 	sb	a1, 0(t0)
 	addi	t0, t0, 1
-	bltu	t0, a3, 1b
+	bltu	t0, a3, .Lmisaligned1
 	sub	a2, a2, a4		/* Update count */
 
-2: /* Duff's device with 32 XLEN stores per iteration */
+.Ldo_duff:
+	/* Duff's device with 32 XLEN stores per iteration */
 	/* Broadcast value into all bytes */
 	andi	a1, a1, 0xff
 	slli	a3, a1, 8
@@ -48,7 +49,7 @@ WEAK(memset)
 	add	a3, t0, a4
 
 	andi	a4, a4, 31*SZREG	/* Calculate remainder */
-	beqz	a4, 3f			/* Shortcut if no remainder */
+	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
 	neg	a4, a4
 	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
 
@@ -57,13 +58,13 @@ WEAK(memset)
 
 	/* Jump into loop body */
 	/* Assumes 32-bit instruction lengths */
-	la	a5, 3f
+	la	a5, .Lduff_loop
 #ifdef CONFIG_64BIT
 	srli	a4, a4, 1
 #endif
 	add	a5, a5, a4
 	jr	a5
-3:
+.Lduff_loop:
 	REG_S	a1,        0(t0)
 	REG_S	a1,    SZREG(t0)
 	REG_S	a1,  2*SZREG(t0)
@@ -98,17 +99,17 @@ WEAK(memset)
 	REG_S	a1, 31*SZREG(t0)
 
 	addi	t0, t0, 32*SZREG
-	bltu	t0, a3, 3b
+	bltu	t0, a3, .Lduff_loop
 	andi	a2, a2, SZREG-1		/* Update count */
 
-4:
+.Lfinish:
 	/* Handle trailing misalignment */
-	beqz	a2, 6f
+	beqz	a2, .Ldone
 	add	a3, t0, a2
-5:
+.Lmisaligned2:
 	sb	a1, 0(t0)
 	addi	t0, t0, 1
-	bltu	t0, a3, 5b
-6:
+	bltu	t0, a3, .Lmisaligned2
+.Ldone:
 	ret
 END(__memset)
-- 
2.37.3


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

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

* [PATCH 9/9] RISC-V: Use Zicboz in memset when available
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (7 preceding siblings ...)
  2022-10-27 13:02 ` [PATCH 8/9] RISC-V: lib: Use named labels in memset Andrew Jones
@ 2022-10-27 13:02 ` Andrew Jones
  2022-10-30 22:35   ` Conor Dooley
  2022-11-03  2:43   ` Palmer Dabbelt
  2022-10-29  9:59 ` [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 13:02 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

RISC-V has an optimized memset() which does byte by byte writes up to
the first sizeof(long) aligned address, then uses Duff's device until
the last sizeof(long) aligned address, and finally byte by byte to
the end. When memset is used to zero memory and the Zicboz extension
is available, then we can extend that by doing the optimized memset
up to the first Zicboz block size aligned address, then use the
Zicboz zero instruction for each block to the last block size aligned
address, and finally the optimized memset to the end.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
index 74e4c7feec00..786b85b5e9cc 100644
--- a/arch/riscv/lib/memset.S
+++ b/arch/riscv/lib/memset.S
@@ -5,6 +5,12 @@
 
 #include <linux/linkage.h>
 #include <asm/asm.h>
+#include <asm/alternative-macros.h>
+#include <asm/insn-def.h>
+#include <asm/hwcap.h>
+
+#define ALT_ZICBOZ(old, new)	ALTERNATIVE(old, new, 0, RISCV_ISA_EXT_ZICBOZ, \
+					    CONFIG_RISCV_ISA_ZICBOZ)
 
 /* void *memset(void *, int, size_t) */
 ENTRY(__memset)
@@ -15,6 +21,58 @@ WEAK(memset)
 	sltiu	a3, a2, 16
 	bnez	a3, .Lfinish
 
+#ifdef CONFIG_RISCV_ISA_ZICBOZ
+	ALT_ZICBOZ("j .Ldo_memset", "nop")
+	/*
+	 * t1 will be the Zicboz block size.
+	 * Zero means we're not using Zicboz, and we don't when a1 != 0
+	 */
+	li	t1, 0
+	bnez	a1, .Ldo_memset
+	la	a3, riscv_cboz_block_size
+	lw	t1, 0(a3)
+
+	/*
+	 * Round to nearest Zicboz block-aligned address
+	 * greater than or equal to the start address.
+	 */
+	addi	a3, t1, -1
+	not	t2, a3			/* t2 is Zicboz block size mask */
+	add	a3, t0, a3
+	and	t3, a3, t2		/* t3 is Zicboz block aligned start */
+
+	/* Did we go too far or not have at least one block? */
+	add	a3, a0, a2
+	and	a3, a3, t2
+	bgtu	a3, t3, .Ldo_zero
+	li	t1, 0
+	j	.Ldo_memset
+
+.Ldo_zero:
+	/* Use Duff for initial bytes if there are any */
+	bne	t3, t0, .Ldo_memset
+
+.Ldo_zero2:
+	/* Calculate end address */
+	and	a3, a2, t2
+	add	a3, t0, a3
+	sub	a4, a3, t0
+
+.Lzero_loop:
+	CBO_ZERO(t0)
+	add	t0, t0, t1
+	bltu	t0, a3, .Lzero_loop
+	li	t1, 0			/* We're done with Zicboz */
+
+	sub	a2, a2, a4		/* Update count */
+	sltiu	a3, a2, 16
+	bnez	a3, .Lfinish
+
+	/* t0 is Zicboz block size aligned, so it must be SZREG aligned */
+	j	.Ldo_duff3
+#endif
+
+.Ldo_memset:
 	/*
 	 * Round to nearest XLEN-aligned address
 	 * greater than or equal to the start address.
@@ -33,6 +91,18 @@ WEAK(memset)
 
 .Ldo_duff:
 	/* Duff's device with 32 XLEN stores per iteration */
+
+#ifdef CONFIG_RISCV_ISA_ZICBOZ
+	ALT_ZICBOZ("j .Ldo_duff2", "nop")
+	beqz	t1, .Ldo_duff2
+	/* a3, "end", is start of block aligned start. a1 is 0 */
+	move    a3, t3
+	sub	a4, a3, t0		/* a4 is SZREG aligned count */
+	move	t4, a4			/* Save count for later, see below. */
+	j	.Ldo_duff4
+#endif
+
+.Ldo_duff2:
 	/* Broadcast value into all bytes */
 	andi	a1, a1, 0xff
 	slli	a3, a1, 8
@@ -44,10 +114,12 @@ WEAK(memset)
 	or	a1, a3, a1
 #endif
 
+.Ldo_duff3:
 	/* Calculate end address */
 	andi	a4, a2, ~(SZREG-1)
 	add	a3, t0, a4
 
+.Ldo_duff4:
 	andi	a4, a4, 31*SZREG	/* Calculate remainder */
 	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
 	neg	a4, a4
@@ -100,6 +172,15 @@ WEAK(memset)
 
 	addi	t0, t0, 32*SZREG
 	bltu	t0, a3, .Lduff_loop
+
+#ifdef CONFIG_RISCV_ISA_ZICBOZ
+	ALT_ZICBOZ("j .Lcount_update", "nop")
+	beqz	t1, .Lcount_update
+	sub	a2, a2, t4		/* Difference was saved above */
+	j	.Ldo_zero2
+#endif
+
+.Lcount_update:
 	andi	a2, a2, SZREG-1		/* Update count */
 
 .Lfinish:
-- 
2.37.3


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

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

* Re: [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop
  2022-10-27 13:02 ` [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop Andrew Jones
@ 2022-10-27 14:58   ` Heiko Stübner
  2022-10-30 20:31   ` Conor Dooley
  1 sibling, 0 replies; 40+ messages in thread
From: Heiko Stübner @ 2022-10-27 14:58 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv, Andrew Jones
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Conor Dooley, Atish Patra, Jisheng Zhang

Am Donnerstag, 27. Oktober 2022, 15:02:39 CEST schrieb Andrew Jones:
> Refactor riscv_init_cbom_blocksize() to prepare for it to be used
> for both cbom block size and cboz block size.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>



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

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

* Re: [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing
  2022-10-27 13:02 ` [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing Andrew Jones
@ 2022-10-27 15:03   ` Heiko Stübner
  2022-10-27 15:42     ` Andrew Jones
  2022-10-30 20:47   ` Conor Dooley
  1 sibling, 1 reply; 40+ messages in thread
From: Heiko Stübner @ 2022-10-27 15:03 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv, Andrew Jones
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Conor Dooley, Atish Patra, Jisheng Zhang

Am Donnerstag, 27. Oktober 2022, 15:02:40 CEST schrieb Andrew Jones:
> Mostly follow the same pattern as Zicbom, but leave out the toolchain
> checks as we plan to use the insn-def framework for the cbo.zero
> instruction.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/Kconfig                  | 13 +++++++++++++
>  arch/riscv/include/asm/cacheflush.h |  3 ++-
>  arch/riscv/include/asm/hwcap.h      |  1 +
>  arch/riscv/kernel/cpu.c             |  1 +
>  arch/riscv/kernel/cpufeature.c      | 10 ++++++++++
>  arch/riscv/kernel/setup.c           |  2 +-
>  arch/riscv/mm/cacheflush.c          | 23 +++++++++++++++--------
>  7 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 6b48a3ae9843..c20e6fa0c0b1 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -433,6 +433,19 @@ config RISCV_ISA_ZICBOM
>  
>  	   If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_ZICBOZ
> +	bool "Zicboz extension support for faster zeroing of memory"
> +	depends on !XIP_KERNEL && MMU
> +	select RISCV_ALTERNATIVE
> +	default y
> +	help
> +	   Adds support to dynamically detect the presence of the ZICBOZ
> +	   extension (cbo.zero instruction) and enable its usage.
> +
> +	   The Zicboz extension is used for faster zeroing of memory.
> +
> +	   If you don't know what to do here, say Y.
> +
>  config FPU
>  	bool "FPU support"
>  	default y
> diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
> index f6fbe7042f1c..5b31568cf5e6 100644
> --- a/arch/riscv/include/asm/cacheflush.h
> +++ b/arch/riscv/include/asm/cacheflush.h
> @@ -43,7 +43,8 @@ void flush_icache_mm(struct mm_struct *mm, bool local);
>  #endif /* CONFIG_SMP */
>  
>  extern unsigned int riscv_cbom_block_size;
> -void riscv_init_cbom_blocksize(void);
> +extern unsigned int riscv_cboz_block_size;
> +void riscv_init_cbo_blocksizes(void);
>  
>  #ifdef CONFIG_RISCV_DMA_NONCOHERENT
>  void riscv_noncoherent_supported(void);
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5d6492bde446..eaa5a972ad2d 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -45,6 +45,7 @@
>  #define RISCV_ISA_EXT_ZIHINTPAUSE	29
>  #define RISCV_ISA_EXT_SSTC		30
>  #define RISCV_ISA_EXT_SVINVAL		31
> +#define RISCV_ISA_EXT_ZICBOZ		32
>  
>  #define RISCV_ISA_EXT_ID_MAX		RISCV_ISA_EXT_MAX
>  
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fa427bdcf773..bf969218f609 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -144,6 +144,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
>  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>  	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
>  	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> +	__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
>  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
>  };
>  
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 18b9ed4df1f4..e13b3391de76 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -78,6 +78,15 @@ static bool riscv_isa_extension_check(int id)
>  			return false;
>  		}
>  		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");
> +			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");
> +			return false;
> +		}
> +		return true;
>  	}
>  
>  	return true;
> @@ -225,6 +234,7 @@ void __init riscv_fill_hwcap(void)
>  				SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
>  				SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
>  				SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> +				SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
>  			}
>  #undef SET_ISA_EXT_MAP
>  		}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index a07917551027..26de0d8fd23d 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -296,7 +296,7 @@ void __init setup_arch(char **cmdline_p)
>  	setup_smp();
>  #endif
>  
> -	riscv_init_cbom_blocksize();
> +	riscv_init_cbo_blocksizes();
>  	riscv_fill_hwcap();
>  	apply_boot_alternatives();
>  #ifdef CONFIG_RISCV_DMA_NONCOHERENT
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index f096b9966cae..208e0d58bde3 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -91,6 +91,9 @@ void flush_icache_pte(pte_t pte)
>  unsigned int riscv_cbom_block_size;
>  EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
>  
> +unsigned int riscv_cboz_block_size;
> +EXPORT_SYMBOL_GPL(riscv_cboz_block_size);
> +
>  static void cbo_get_block_size(struct device_node *node,
>  			       const char *name, u32 *blksz,
>  			       unsigned long *first_hartid)
> @@ -113,19 +116,23 @@ static void cbo_get_block_size(struct device_node *node,
>  	}
>  }
>  
> -void riscv_init_cbom_blocksize(void)
> +void riscv_init_cbo_blocksizes(void)
>  {
> +	unsigned long cbom_hartid, cboz_hartid;
> +	u32 cbom_blksz = 0, cboz_blksz = 0;
>  	struct device_node *node;
> -	unsigned long cbom_hartid;
> -	u32 probed_block_size;
>  
> -	probed_block_size = 0;
>  	for_each_of_cpu_node(node) {
> -		/* set block-size for cbom extension if available */
> +		/* set block-size for cbom and/or cboz extension if available */
>  		cbo_get_block_size(node, "riscv,cbom-block-size",
> -				   &probed_block_size, &cbom_hartid);
> +				   &cbom_blksz, &cbom_hartid);
> +		cbo_get_block_size(node, "riscv,cboz-block-size",

This is missing an entry in
	Documentation/devicetree/bindings/riscv/cpus.yaml
for the added riscv,cboz-block-size property.

Otherwise
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> +				   &cboz_blksz, &cboz_hartid);
>  	}
>  
> -	if (probed_block_size)
> -		riscv_cbom_block_size = probed_block_size;
> +	if (cbom_blksz)
> +		riscv_cbom_block_size = cbom_blksz;
> +
> +	if (cboz_blksz)
> +		riscv_cboz_block_size = cboz_blksz;
>  }
> 





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

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

* Re: [PATCH 3/9] RISC-V: insn-def: Define cbo.zero
  2022-10-27 13:02 ` [PATCH 3/9] RISC-V: insn-def: Define cbo.zero Andrew Jones
@ 2022-10-27 15:37   ` Heiko Stübner
  2022-10-30 21:08   ` Conor Dooley
  1 sibling, 0 replies; 40+ messages in thread
From: Heiko Stübner @ 2022-10-27 15:37 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv, Andrew Jones
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Conor Dooley, Atish Patra, Jisheng Zhang

Am Donnerstag, 27. Oktober 2022, 15:02:41 CEST schrieb Andrew Jones:
> CBO instructions use the I-type of instruction format where
> the immediate is used to identify the CBO instruction type.
> Add I-type instruction encoding support to insn-def and also
> add cbo.zero.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>




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

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

* Re: [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing
  2022-10-27 15:03   ` Heiko Stübner
@ 2022-10-27 15:42     ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-27 15:42 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Conor Dooley, Atish Patra, Jisheng Zhang

On Thu, Oct 27, 2022 at 05:03:58PM +0200, Heiko Stübner wrote:
> Am Donnerstag, 27. Oktober 2022, 15:02:40 CEST schrieb Andrew Jones:
...
> > +		cbo_get_block_size(node, "riscv,cboz-block-size",
> 
> This is missing an entry in
> 	Documentation/devicetree/bindings/riscv/cpus.yaml
> for the added riscv,cboz-block-size property.

Thanks for pointing that out. I'll add in v2.

> 
> Otherwise
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Thanks,
drew

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

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

* Re: [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (8 preceding siblings ...)
  2022-10-27 13:02 ` [PATCH 9/9] RISC-V: Use Zicboz in memset when available Andrew Jones
@ 2022-10-29  9:59 ` Andrew Jones
  2022-10-30 20:23   ` Conor Dooley
  2022-11-01 10:37 ` Andrew Jones
  2022-12-20 12:55 ` Conor Dooley
  11 siblings, 1 reply; 40+ messages in thread
From: Andrew Jones @ 2022-10-29  9:59 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:38PM +0200, Andrew Jones wrote:
> When the Zicboz extension is available we can more rapidly zero naturally
> aligned Zicboz block sized chunks of memory. As pages are always page
> aligned and are larger than any Zicboz block size will be, then
> clear_page() appears to be a good candidate for the extension. While cycle
> count and energy consumption should also be considered, we can be pretty
> certain that implementing clear_page() with the Zicboz extension is a win
> by comparing the new dynamic instruction count with its current count[1].
> Doing so we see that the new count is less than half the old count (see
> patch4's commit message for more details). Another candidate for the
> extension is memset(), but, since memset() isn't just used for zeroing
> memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
> it's not as obvious if adding support for Zicboz will be an overall win.
> In order to make a determination, I've done some analysis and wrote my
> conclusions in the bullets below.
> 
> * When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
>   doesn't change, so that's fine.
> 
> * The overhead added to memset() when the Zicboz extension isn't present,
>   but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
>   which I believe is fine.
> 
> * The overhead added to a memset() invocation which is not zeroing memory
>   is 7 instructions, where 3 are branches. This seems fine and,
>   furthermore, memset() is almost always invoked to zero memory (99% [2]).
> 
> * When memset() is invoked to zero memory, the proposed Zicboz extended
>   memset() always has a lower dynamic instruction count than the current
>   memset() as long as the input address is Zicboz block aligned and the
>   length is >= the block size.
> 
> * When memset() is invoked to zero memory, the proposed Zicboz extended
>   memset() is always worse for unaligned or too small inputs than the
>   current memset(), but it's only at most a few dozen instructions worse.
>   I think this is probably fine, especially considering the large majority
>   of zeroing invocations are 64 bytes or larger and are aligned to a
>   power-of-2 boundary, 64-byte or larger (77% [2]).
> 
> [1] I ported the functions under test to userspace and linked them with
>     a test program. Then, I ran them under gdb with a script[3] which
>     counted instructions by single stepping.
> 
> [2] I wrote bpftrace scripts[4] to count memset() invocations to see the
>     frequency of it being used to zero memory and have block size aligned
>     input addresses with block size or larger lengths. The workload was
>     just random desktop stuff including streaming video and compiling.
>     While I did run this on my x86 notebook, I still expect the data to
>     be representative on RISC-V. Note, x86 has clear_page() so the
>     memset() data regarding alignment and size weren't over inflated by
>     page zeroing invocations. Grepping also shows the large majority of
>     memset() calls are to zero memory (93%).
> 
> [3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
> [4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe
> 
> These patches are based on the following pending series
> 
> 1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
>    20221024091309.406906-1-ajones@ventanamicro.com
> 
> 2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
>    20221006070818.3616-1-jszhang@kernel.org
>    Also including the additional patch proposed here
>    20221013162038.ehseju2neic2xu5z@kamzik
> 
> The patches are also available here
> https://github.com/jones-drew/linux/commits/riscv/zicboz
> 
> To test over QEMU this branch may be used to enable Zicboz
> https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz
> 
> To test running a KVM guest with Zicboz this kvmtool branch may be used
> https://github.com/jones-drew/kvmtool/commits/riscv/zicboz
> 
> Thanks,
> drew
> 
> Andrew Jones (9):
>   RISC-V: Factor out body of riscv_init_cbom_blocksize loop
>   RISC-V: Add Zicboz detection and block size parsing
>   RISC-V: insn-def: Define cbo.zero
>   RISC-V: Use Zicboz in clear_page when available
>   RISC-V: KVM: Provide UAPI for Zicboz block size
>   RISC-V: KVM: Expose Zicboz to the guest
>   RISC-V: lib: Improve memset assembler formatting
>   RISC-V: lib: Use named labels in memset
>   RISC-V: Use Zicboz in memset when available
> 
>  arch/riscv/Kconfig                  |  13 ++
>  arch/riscv/include/asm/cacheflush.h |   3 +-
>  arch/riscv/include/asm/hwcap.h      |   1 +
>  arch/riscv/include/asm/insn-def.h   |  50 ++++++
>  arch/riscv/include/asm/page.h       |   6 +-
>  arch/riscv/include/uapi/asm/kvm.h   |   2 +
>  arch/riscv/kernel/cpu.c             |   1 +
>  arch/riscv/kernel/cpufeature.c      |  10 ++
>  arch/riscv/kernel/setup.c           |   2 +-
>  arch/riscv/kvm/vcpu.c               |  11 ++
>  arch/riscv/lib/Makefile             |   1 +
>  arch/riscv/lib/clear_page.S         |  28 ++++
>  arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
>  arch/riscv/mm/cacheflush.c          |  64 +++++---
>  14 files changed, 325 insertions(+), 108 deletions(-)
>  create mode 100644 arch/riscv/lib/clear_page.S
> 
> -- 
> 2.37.3
>

FYI, I just tried this with clang. It compiles but doesn't boot when
Zicboz is present (it does boot when Zicboz is disabled in QEMU). I'm
suspicious of the ALTERNATIVE() stuff, but I'll debug more on Monday.
Also, I see building with LLVM=1 doesn't work, but that appears to be
an issue introduced with "[PATCH 0/8] riscv: improve boot time isa
extensions handling" which this series is based on.

drew

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

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

* Re: [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
  2022-10-29  9:59 ` [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
@ 2022-10-30 20:23   ` Conor Dooley
  2022-10-31  8:39     ` Andrew Jones
  0 siblings, 1 reply; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 20:23 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sat, Oct 29, 2022 at 11:59:53AM +0200, Andrew Jones wrote:
> On Thu, Oct 27, 2022 at 03:02:38PM +0200, Andrew Jones wrote:
> > When the Zicboz extension is available we can more rapidly zero naturally
> > aligned Zicboz block sized chunks of memory. As pages are always page
> > aligned and are larger than any Zicboz block size will be, then
> > clear_page() appears to be a good candidate for the extension. While cycle
> > count and energy consumption should also be considered, we can be pretty
> > certain that implementing clear_page() with the Zicboz extension is a win
> > by comparing the new dynamic instruction count with its current count[1].
> > Doing so we see that the new count is less than half the old count (see
> > patch4's commit message for more details). Another candidate for the
> > extension is memset(), but, since memset() isn't just used for zeroing
> > memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
> > it's not as obvious if adding support for Zicboz will be an overall win.
> > In order to make a determination, I've done some analysis and wrote my
> > conclusions in the bullets below.
> > 
> > * When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
> >   doesn't change, so that's fine.
> > 
> > * The overhead added to memset() when the Zicboz extension isn't present,
> >   but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
> >   which I believe is fine.
> > 
> > * The overhead added to a memset() invocation which is not zeroing memory
> >   is 7 instructions, where 3 are branches. This seems fine and,
> >   furthermore, memset() is almost always invoked to zero memory (99% [2]).
> > 
> > * When memset() is invoked to zero memory, the proposed Zicboz extended
> >   memset() always has a lower dynamic instruction count than the current
> >   memset() as long as the input address is Zicboz block aligned and the
> >   length is >= the block size.
> > 
> > * When memset() is invoked to zero memory, the proposed Zicboz extended
> >   memset() is always worse for unaligned or too small inputs than the
> >   current memset(), but it's only at most a few dozen instructions worse.
> >   I think this is probably fine, especially considering the large majority
> >   of zeroing invocations are 64 bytes or larger and are aligned to a
> >   power-of-2 boundary, 64-byte or larger (77% [2]).
> > 
> > [1] I ported the functions under test to userspace and linked them with
> >     a test program. Then, I ran them under gdb with a script[3] which
> >     counted instructions by single stepping.
> > 
> > [2] I wrote bpftrace scripts[4] to count memset() invocations to see the
> >     frequency of it being used to zero memory and have block size aligned
> >     input addresses with block size or larger lengths. The workload was
> >     just random desktop stuff including streaming video and compiling.
> >     While I did run this on my x86 notebook, I still expect the data to
> >     be representative on RISC-V. Note, x86 has clear_page() so the
> >     memset() data regarding alignment and size weren't over inflated by
> >     page zeroing invocations. Grepping also shows the large majority of
> >     memset() calls are to zero memory (93%).
> > 
> > [3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
> > [4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe
> > 
> > These patches are based on the following pending series
> > 
> > 1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
> >    20221024091309.406906-1-ajones@ventanamicro.com
> > 
> > 2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
> >    20221006070818.3616-1-jszhang@kernel.org
> >    Also including the additional patch proposed here
> >    20221013162038.ehseju2neic2xu5z@kamzik
> > 
> > The patches are also available here
> > https://github.com/jones-drew/linux/commits/riscv/zicboz
> > 
> > To test over QEMU this branch may be used to enable Zicboz
> > https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz
> > 
> > To test running a KVM guest with Zicboz this kvmtool branch may be used
> > https://github.com/jones-drew/kvmtool/commits/riscv/zicboz
> > 
> > Thanks,
> > drew
> > 
> > Andrew Jones (9):
> >   RISC-V: Factor out body of riscv_init_cbom_blocksize loop
> >   RISC-V: Add Zicboz detection and block size parsing
> >   RISC-V: insn-def: Define cbo.zero
> >   RISC-V: Use Zicboz in clear_page when available
> >   RISC-V: KVM: Provide UAPI for Zicboz block size
> >   RISC-V: KVM: Expose Zicboz to the guest
> >   RISC-V: lib: Improve memset assembler formatting
> >   RISC-V: lib: Use named labels in memset
> >   RISC-V: Use Zicboz in memset when available
> > 
> >  arch/riscv/Kconfig                  |  13 ++
> >  arch/riscv/include/asm/cacheflush.h |   3 +-
> >  arch/riscv/include/asm/hwcap.h      |   1 +
> >  arch/riscv/include/asm/insn-def.h   |  50 ++++++
> >  arch/riscv/include/asm/page.h       |   6 +-
> >  arch/riscv/include/uapi/asm/kvm.h   |   2 +
> >  arch/riscv/kernel/cpu.c             |   1 +
> >  arch/riscv/kernel/cpufeature.c      |  10 ++
> >  arch/riscv/kernel/setup.c           |   2 +-
> >  arch/riscv/kvm/vcpu.c               |  11 ++
> >  arch/riscv/lib/Makefile             |   1 +
> >  arch/riscv/lib/clear_page.S         |  28 ++++
> >  arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
> >  arch/riscv/mm/cacheflush.c          |  64 +++++---
> >  14 files changed, 325 insertions(+), 108 deletions(-)
> >  create mode 100644 arch/riscv/lib/clear_page.S
> > 
> > -- 
> > 2.37.3
> >
> 
> FYI, I just tried this with clang. It compiles but doesn't boot when
> Zicboz is present (it does boot when Zicboz is disabled in QEMU). I'm
> suspicious of the ALTERNATIVE() stuff, but I'll debug more on Monday.
> Also, I see building with LLVM=1 doesn't work, but that appears to be
> an issue introduced with "[PATCH 0/8] riscv: improve boot time isa
> extensions handling" which this series is based on.

Yeah, I was hitting the LLVM failures when I gave it a go the other day,
thanks for passing it on to Jisheng.. I meant to do that and forgot.
FWIW, I did find a build error with gcc too:

riscv64-unknown-linux-gnu-ld: arch/riscv/purgatory/purgatory.ro: in function `.L3␂1':
ctype.c:(.text+0x215a): undefined reference to `riscv_cboz_block_size'

defconfig should be:
https://gist.github.com/ConchuOD/e14293a28b04dca2125bdcc0d97d366a
& toolchain stuff:
CONFIG_CC_VERSION_TEXT="riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0"
CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=110100
CONFIG_AS_IS_GNU=y
CONFIG_AS_VERSION=23700
CONFIG_LD_IS_BFD=y
CONFIG_LD_VERSION=23700

My toolchain predates zicbo* being introduced & I set the config option.
I didn't do any specific digging as to the cause.

Thanks,
Conor.


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

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

* Re: [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop
  2022-10-27 13:02 ` [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop Andrew Jones
  2022-10-27 14:58   ` Heiko Stübner
@ 2022-10-30 20:31   ` Conor Dooley
  2022-10-31  8:11     ` Andrew Jones
  1 sibling, 1 reply; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 20:31 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:39PM +0200, Andrew Jones wrote:
> Refactor riscv_init_cbom_blocksize() to prepare for it to be used
> for both cbom block size and cboz block size.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/mm/cacheflush.c | 45 +++++++++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index 57b40a350420..f096b9966cae 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -91,34 +91,39 @@ void flush_icache_pte(pte_t pte)
>  unsigned int riscv_cbom_block_size;
>  EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
>  
> +static void cbo_get_block_size(struct device_node *node,
> +			       const char *name, u32 *blksz,

Is there a reason you called this "blksz" when we are using the spelt
out "block_size" everywhere else in this code? My OCD would appreciate
the consistency :s

Other than that:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> +			       unsigned long *first_hartid)
> +{
> +	unsigned long hartid;
> +	u32 val;
> +
> +	if (riscv_of_processor_hartid(node, &hartid))
> +		return;
> +
> +	if (of_property_read_u32(node, name, &val))
> +		return;
> +
> +	if (!*blksz) {
> +		*blksz = val;
> +		*first_hartid = hartid;
> +	} else if (*blksz != val) {
> +		pr_warn("%s mismatched between harts %lu and %lu\n",
> +			name, *first_hartid, hartid);
> +	}
> +}
> +
>  void riscv_init_cbom_blocksize(void)
>  {
>  	struct device_node *node;
>  	unsigned long cbom_hartid;
> -	u32 val, probed_block_size;
> -	int ret;
> +	u32 probed_block_size;
>  
>  	probed_block_size = 0;
>  	for_each_of_cpu_node(node) {
> -		unsigned long hartid;
> -
> -		ret = riscv_of_processor_hartid(node, &hartid);
> -		if (ret)
> -			continue;
> -
>  		/* set block-size for cbom extension if available */
> -		ret = of_property_read_u32(node, "riscv,cbom-block-size", &val);
> -		if (ret)
> -			continue;
> -
> -		if (!probed_block_size) {
> -			probed_block_size = val;
> -			cbom_hartid = hartid;
> -		} else {
> -			if (probed_block_size != val)
> -				pr_warn("cbom-block-size mismatched between harts %lu and %lu\n",
> -					cbom_hartid, hartid);
> -		}
> +		cbo_get_block_size(node, "riscv,cbom-block-size",
> +				   &probed_block_size, &cbom_hartid);
>  	}
>  
>  	if (probed_block_size)
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing
  2022-10-27 13:02 ` [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing Andrew Jones
  2022-10-27 15:03   ` Heiko Stübner
@ 2022-10-30 20:47   ` Conor Dooley
  2022-10-31  8:12     ` Andrew Jones
  2022-11-13 22:24     ` Conor Dooley
  1 sibling, 2 replies; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 20:47 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:40PM +0200, Andrew Jones wrote:
> Mostly follow the same pattern as Zicbom, but leave out the toolchain
> checks as we plan to use the insn-def framework for the cbo.zero
> instruction.

nit: I'd prefer to see a commit message that stands on it's own without
relying on someone knowing how zicbom has been wired up.

> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/Kconfig                  | 13 +++++++++++++
>  arch/riscv/include/asm/cacheflush.h |  3 ++-
>  arch/riscv/include/asm/hwcap.h      |  1 +
>  arch/riscv/kernel/cpu.c             |  1 +
>  arch/riscv/kernel/cpufeature.c      | 10 ++++++++++
>  arch/riscv/kernel/setup.c           |  2 +-
>  arch/riscv/mm/cacheflush.c          | 23 +++++++++++++++--------
>  7 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 6b48a3ae9843..c20e6fa0c0b1 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -433,6 +433,19 @@ config RISCV_ISA_ZICBOM
>  
>  	   If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_ZICBOZ
> +	bool "Zicboz extension support for faster zeroing of memory"
> +	depends on !XIP_KERNEL && MMU
> +	select RISCV_ALTERNATIVE
> +	default y
> +	help
> +	   Adds support to dynamically detect the presence of the ZICBOZ
> +	   extension (cbo.zero instruction) and enable its usage.
> +
> +	   The Zicboz extension is used for faster zeroing of memory.
> +
> +	   If you don't know what to do here, say Y.
> +
>  config FPU
>  	bool "FPU support"
>  	default y
> diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
> index f6fbe7042f1c..5b31568cf5e6 100644
> --- a/arch/riscv/include/asm/cacheflush.h
> +++ b/arch/riscv/include/asm/cacheflush.h
> @@ -43,7 +43,8 @@ void flush_icache_mm(struct mm_struct *mm, bool local);
>  #endif /* CONFIG_SMP */
>  
>  extern unsigned int riscv_cbom_block_size;
> -void riscv_init_cbom_blocksize(void);
> +extern unsigned int riscv_cboz_block_size;
> +void riscv_init_cbo_blocksizes(void);
>  
>  #ifdef CONFIG_RISCV_DMA_NONCOHERENT
>  void riscv_noncoherent_supported(void);
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5d6492bde446..eaa5a972ad2d 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -45,6 +45,7 @@
>  #define RISCV_ISA_EXT_ZIHINTPAUSE	29
>  #define RISCV_ISA_EXT_SSTC		30
>  #define RISCV_ISA_EXT_SVINVAL		31
> +#define RISCV_ISA_EXT_ZICBOZ		32
>  
>  #define RISCV_ISA_EXT_ID_MAX		RISCV_ISA_EXT_MAX
>  
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fa427bdcf773..bf969218f609 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -144,6 +144,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
>  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>  	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
>  	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> +	__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
>  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
>  };
>  
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 18b9ed4df1f4..e13b3391de76 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -78,6 +78,15 @@ static bool riscv_isa_extension_check(int id)
>  			return false;
>  		}
>  		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");
> +			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");
> +			return false;
> +		}
> +		return true;
>  	}
>  
>  	return true;
> @@ -225,6 +234,7 @@ void __init riscv_fill_hwcap(void)
>  				SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
>  				SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
>  				SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> +				SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
>  			}
>  #undef SET_ISA_EXT_MAP
>  		}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index a07917551027..26de0d8fd23d 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -296,7 +296,7 @@ void __init setup_arch(char **cmdline_p)
>  	setup_smp();
>  #endif
>  
> -	riscv_init_cbom_blocksize();
> +	riscv_init_cbo_blocksizes();
>  	riscv_fill_hwcap();
>  	apply_boot_alternatives();
>  #ifdef CONFIG_RISCV_DMA_NONCOHERENT
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index f096b9966cae..208e0d58bde3 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -91,6 +91,9 @@ void flush_icache_pte(pte_t pte)
>  unsigned int riscv_cbom_block_size;
>  EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
>  
> +unsigned int riscv_cboz_block_size;
> +EXPORT_SYMBOL_GPL(riscv_cboz_block_size);
> +
>  static void cbo_get_block_size(struct device_node *node,
>  			       const char *name, u32 *blksz,
>  			       unsigned long *first_hartid)
> @@ -113,19 +116,23 @@ static void cbo_get_block_size(struct device_node *node,
>  	}
>  }
>  
> -void riscv_init_cbom_blocksize(void)
> +void riscv_init_cbo_blocksizes(void)
>  {
> +	unsigned long cbom_hartid, cboz_hartid;
> +	u32 cbom_blksz = 0, cboz_blksz = 0;
>  	struct device_node *node;
> -	unsigned long cbom_hartid;
> -	u32 probed_block_size;
>  
> -	probed_block_size = 0;
>  	for_each_of_cpu_node(node) {
> -		/* set block-size for cbom extension if available */
> +		/* set block-size for cbom and/or cboz extension if available */
>  		cbo_get_block_size(node, "riscv,cbom-block-size",
> -				   &probed_block_size, &cbom_hartid);
> +				   &cbom_blksz, &cbom_hartid);
> +		cbo_get_block_size(node, "riscv,cboz-block-size",
> +				   &cboz_blksz, &cboz_hartid);
>  	}
>  
> -	if (probed_block_size)
> -		riscv_cbom_block_size = probed_block_size;
> +	if (cbom_blksz)
> +		riscv_cbom_block_size = cbom_blksz;

As in patch 1, can you stick with s/blksz/block_size/ please.
My poor OCD!!! Other than those minor things:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> +
> +	if (cboz_blksz)
> +		riscv_cboz_block_size = cboz_blksz;
>  }
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 3/9] RISC-V: insn-def: Define cbo.zero
  2022-10-27 13:02 ` [PATCH 3/9] RISC-V: insn-def: Define cbo.zero Andrew Jones
  2022-10-27 15:37   ` Heiko Stübner
@ 2022-10-30 21:08   ` Conor Dooley
  2022-10-31  8:18     ` Andrew Jones
  1 sibling, 1 reply; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 21:08 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:41PM +0200, Andrew Jones wrote:
> CBO instructions use the I-type of instruction format where
> the immediate is used to identify the CBO instruction type.
> Add I-type instruction encoding support to insn-def and also
> add cbo.zero.

Cool, I like this a lot more than putting cc-option & linker version
number checks into Kbuild stuff. I guess if this gets applied, I'll
send one ripping my checks in Kconfig out and replace it with one of
these?

I mostly just cross-checked the numbers etc here and things look grand.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

One minor comment below.

> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/include/asm/insn-def.h | 50 +++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index 16044affa57c..f13054716556 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -12,6 +12,12 @@
>  #define INSN_R_RD_SHIFT			 7
>  #define INSN_R_OPCODE_SHIFT		 0
>  
> +#define INSN_I_SIMM12_SHIFT		20
> +#define INSN_I_RS1_SHIFT		15
> +#define INSN_I_FUNC3_SHIFT		12
> +#define INSN_I_RD_SHIFT			 7
> +#define INSN_I_OPCODE_SHIFT		 0
> +
>  #ifdef __ASSEMBLY__
>  
>  #ifdef CONFIG_AS_HAS_INSN
> @@ -20,6 +26,10 @@
>  	.insn	r \opcode, \func3, \func7, \rd, \rs1, \rs2
>  	.endm
>  
> +	.macro insn_i, opcode, func3, rd, rs1, simm12
> +	.insn	i \opcode, \func3, \rd, \rs1, \simm12
> +	.endm
> +
>  #else
>  
>  #include <asm/gpr-num.h>
> @@ -33,9 +43,18 @@
>  		 (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT))
>  	.endm
>  
> +	.macro insn_i, opcode, func3, rd, rs1, simm12
> +	.4byte	((\opcode << INSN_I_OPCODE_SHIFT) |		\
> +		 (\func3 << INSN_I_FUNC3_SHIFT) |		\
> +		 (.L__gpr_num_\rd << INSN_I_RD_SHIFT) |		\
> +		 (.L__gpr_num_\rs1 << INSN_I_RS1_SHIFT) |	\
> +		 (\simm12 << INSN_I_SIMM12_SHIFT))
> +	.endm
> +
>  #endif
>  
>  #define __INSN_R(...)	insn_r __VA_ARGS__
> +#define __INSN_I(...)	insn_i __VA_ARGS__
>  
>  #else /* ! __ASSEMBLY__ */
>  
> @@ -44,6 +63,9 @@
>  #define __INSN_R(opcode, func3, func7, rd, rs1, rs2)	\
>  	".insn	r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n"
>  
> +#define __INSN_I(opcode, func3, rd, rs1, simm12)	\
> +	".insn	i " opcode ", " func3 ", " rd ", " rs1 ", " simm12 "\n"
> +
>  #else
>  
>  #include <linux/stringify.h>
> @@ -60,14 +82,32 @@
>  "		 (.L__gpr_num_\\rs2 << " __stringify(INSN_R_RS2_SHIFT) "))\n" \
>  "	.endm\n"
>  
> +#define DEFINE_INSN_I							\
> +	__DEFINE_ASM_GPR_NUMS						\
> +"	.macro insn_i, opcode, func3, rd, rs1, simm12\n"		\
> +"	.4byte	((\\opcode << " __stringify(INSN_I_OPCODE_SHIFT) ") |"	\
> +"		 (\\func3 << " __stringify(INSN_I_FUNC3_SHIFT) ") |"	\
> +"		 (.L__gpr_num_\\rd << " __stringify(INSN_I_RD_SHIFT) ") |"   \
> +"		 (.L__gpr_num_\\rs1 << " __stringify(INSN_I_RS1_SHIFT) ") |" \

It'd be nice if these macros had aligned \s. I know the file is pretty
inconsistent in that regard and I'm not asking you to change those but I
think it'd be nice to do so for stuff that's just being added now.

Thanks,
Conor.

> +"		 (\\simm12 << " __stringify(INSN_I_SIMM12_SHIFT) "))\n"	\
> +"	.endm\n"
> +
>  #define UNDEFINE_INSN_R							\
>  "	.purgem insn_r\n"
>  
> +#define UNDEFINE_INSN_I							\
> +"	.purgem insn_i\n"
> +
>  #define __INSN_R(opcode, func3, func7, rd, rs1, rs2)			\
>  	DEFINE_INSN_R							\
>  	"insn_r " opcode ", " func3 ", " func7 ", " rd ", " rs1 ", " rs2 "\n" \
>  	UNDEFINE_INSN_R
>  
> +#define __INSN_I(opcode, func3, rd, rs1, simm12)			\
> +	DEFINE_INSN_I							\
> +	"insn_i " opcode ", " func3 ", " rd ", " rs1 ", " simm12 "\n" \
> +	UNDEFINE_INSN_I
> +
>  #endif
>  
>  #endif /* ! __ASSEMBLY__ */
> @@ -76,9 +116,14 @@
>  	__INSN_R(RV_##opcode, RV_##func3, RV_##func7,		\
>  		 RV_##rd, RV_##rs1, RV_##rs2)
>  
> +#define INSN_I(opcode, func3, rd, rs1, simm12)			\
> +	__INSN_I(RV_##opcode, RV_##func3, RV_##rd,		\
> +		 RV_##rs1, RV_##simm12)
> +
>  #define RV_OPCODE(v)		__ASM_STR(v)
>  #define RV_FUNC3(v)		__ASM_STR(v)
>  #define RV_FUNC7(v)		__ASM_STR(v)
> +#define RV_SIMM12(v)		__ASM_STR(v)
>  #define RV_RD(v)		__ASM_STR(v)
>  #define RV_RS1(v)		__ASM_STR(v)
>  #define RV_RS2(v)		__ASM_STR(v)
> @@ -87,6 +132,7 @@
>  #define RV___RS1(v)		__RV_REG(v)
>  #define RV___RS2(v)		__RV_REG(v)
>  
> +#define RV_OPCODE_MISC_MEM	RV_OPCODE(15)
>  #define RV_OPCODE_SYSTEM	RV_OPCODE(115)
>  
>  #define HFENCE_VVMA(vaddr, asid)				\
> @@ -134,4 +180,8 @@
>  	INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(51),		\
>  	       __RD(0), RS1(gaddr), RS2(vmid))
>  
> +#define CBO_ZERO(base)						\
> +	INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0),		\
> +	       RS1(base), SIMM12(4))
> +
>  #endif /* __ASM_INSN_DEF_H */
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size
  2022-10-27 13:02 ` [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size Andrew Jones
@ 2022-10-30 21:23   ` Conor Dooley
  2022-11-27  5:37   ` Anup Patel
  1 sibling, 0 replies; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 21:23 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:43PM +0200, Andrew Jones wrote:
> We're about to allow guests to use the Zicboz extension. KVM
> userspace needs to know the cache block size in order to
> properly advertise it to the guest. Provide a virtual config
> register for userspace to get it with the GET_ONE_REG API, but
> setting it cannot be supported, so disallow SET_ONE_REG.

This all looks pretty formulaic..
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/include/uapi/asm/kvm.h | 1 +
>  arch/riscv/kvm/vcpu.c             | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 8985ff234c01..4bbf55cb2b70 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -49,6 +49,7 @@ struct kvm_sregs {
>  struct kvm_riscv_config {
>  	unsigned long isa;
>  	unsigned long zicbom_block_size;
> +	unsigned long zicboz_block_size;
>  };
>  
>  /* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 71ebbc4821f0..18a739070b51 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -270,6 +270,11 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
>  			return -EINVAL;
>  		reg_val = riscv_cbom_block_size;
>  		break;
> +	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
> +		if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ))
> +			return -EINVAL;
> +		reg_val = riscv_cboz_block_size;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -329,6 +334,8 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
>  		return -EOPNOTSUPP;
> +	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
> +		return -EOPNOTSUPP;
>  	default:
>  		return -EINVAL;
>  	}
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest
  2022-10-27 13:02 ` [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest Andrew Jones
@ 2022-10-30 21:23   ` Conor Dooley
  2022-11-27  5:38   ` Anup Patel
  1 sibling, 0 replies; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 21:23 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:44PM +0200, Andrew Jones wrote:
> Guests may use the cbo.zero instruction when the CPU has the Zicboz
> extension and the hypervisor sets henvcfg.CBZE.
> 
> Add Zicboz support for KVM guests which may be enabled and
> disabled from KVM userspace using the ISA extension ONE_REG API.

Ditto re: formulaic.. Doubt it's needed here, but:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/include/uapi/asm/kvm.h | 1 +
>  arch/riscv/kvm/vcpu.c             | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 4bbf55cb2b70..8dc21ceee7aa 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -103,6 +103,7 @@ enum KVM_RISCV_ISA_EXT_ID {
>  	KVM_RISCV_ISA_EXT_SVINVAL,
>  	KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
>  	KVM_RISCV_ISA_EXT_ZICBOM,
> +	KVM_RISCV_ISA_EXT_ZICBOZ,
>  	KVM_RISCV_ISA_EXT_MAX,
>  };
>  
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 18a739070b51..7758faec590a 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -62,6 +62,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
>  	KVM_ISA_EXT_ARR(SVPBMT),
>  	KVM_ISA_EXT_ARR(ZIHINTPAUSE),
>  	KVM_ISA_EXT_ARR(ZICBOM),
> +	KVM_ISA_EXT_ARR(ZICBOZ),
>  };
>  
>  static unsigned long kvm_riscv_vcpu_base2isa_ext(unsigned long base_ext)
> @@ -821,6 +822,9 @@ static void kvm_riscv_vcpu_update_config(const unsigned long *isa)
>  	if (riscv_isa_extension_available(isa, ZICBOM))
>  		henvcfg |= (ENVCFG_CBIE | ENVCFG_CBCFE);
>  
> +	if (riscv_isa_extension_available(isa, ZICBOZ))
> +		henvcfg |= ENVCFG_CBZE;
> +
>  	csr_write(CSR_HENVCFG, henvcfg);
>  #ifdef CONFIG_32BIT
>  	csr_write(CSR_HENVCFGH, henvcfg >> 32);
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 7/9] RISC-V: lib: Improve memset assembler formatting
  2022-10-27 13:02 ` [PATCH 7/9] RISC-V: lib: Improve memset assembler formatting Andrew Jones
@ 2022-10-30 21:27   ` Conor Dooley
  0 siblings, 0 replies; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 21:27 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:45PM +0200, Andrew Jones wrote:
> Aligning the first operand of each instructions with a tab is a
> typical style which improves readability. Apply it to memset.S.
> While there, we also make a small grammar change to a comment.
> 
> No functional change intended.

Cool, nice cleanup :)
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.

> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/lib/memset.S | 143 ++++++++++++++++++++--------------------
>  1 file changed, 72 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> index 34c5360c6705..e613c5c27998 100644
> --- a/arch/riscv/lib/memset.S
> +++ b/arch/riscv/lib/memset.S
> @@ -3,111 +3,112 @@
>   * Copyright (C) 2013 Regents of the University of California
>   */
>  
> -
>  #include <linux/linkage.h>
>  #include <asm/asm.h>
>  
>  /* void *memset(void *, int, size_t) */
>  ENTRY(__memset)
>  WEAK(memset)
> -	move t0, a0  /* Preserve return value */
> +	move	t0, a0			/* Preserve return value */
>  
>  	/* Defer to byte-oriented fill for small sizes */
> -	sltiu a3, a2, 16
> -	bnez a3, 4f
> +	sltiu	a3, a2, 16
> +	bnez	a3, 4f
>  
>  	/*
>  	 * Round to nearest XLEN-aligned address
> -	 * greater than or equal to start address
> +	 * greater than or equal to the start address.
>  	 */
> -	addi a3, t0, SZREG-1
> -	andi a3, a3, ~(SZREG-1)
> -	beq a3, t0, 2f  /* Skip if already aligned */
> +	addi	a3, t0, SZREG-1
> +	andi	a3, a3, ~(SZREG-1)
> +	beq	a3, t0, 2f		/* Skip if already aligned */
> +
>  	/* Handle initial misalignment */
> -	sub a4, a3, t0
> +	sub	a4, a3, t0
>  1:
> -	sb a1, 0(t0)
> -	addi t0, t0, 1
> -	bltu t0, a3, 1b
> -	sub a2, a2, a4  /* Update count */
> +	sb	a1, 0(t0)
> +	addi	t0, t0, 1
> +	bltu	t0, a3, 1b
> +	sub	a2, a2, a4		/* Update count */
>  
>  2: /* Duff's device with 32 XLEN stores per iteration */
>  	/* Broadcast value into all bytes */
> -	andi a1, a1, 0xff
> -	slli a3, a1, 8
> -	or a1, a3, a1
> -	slli a3, a1, 16
> -	or a1, a3, a1
> +	andi	a1, a1, 0xff
> +	slli	a3, a1, 8
> +	or	a1, a3, a1
> +	slli	a3, a1, 16
> +	or	a1, a3, a1
>  #ifdef CONFIG_64BIT
> -	slli a3, a1, 32
> -	or a1, a3, a1
> +	slli	a3, a1, 32
> +	or	a1, a3, a1
>  #endif
>  
>  	/* Calculate end address */
> -	andi a4, a2, ~(SZREG-1)
> -	add a3, t0, a4
> +	andi	a4, a2, ~(SZREG-1)
> +	add	a3, t0, a4
>  
> -	andi a4, a4, 31*SZREG  /* Calculate remainder */
> -	beqz a4, 3f            /* Shortcut if no remainder */
> -	neg a4, a4
> -	addi a4, a4, 32*SZREG  /* Calculate initial offset */
> +	andi	a4, a4, 31*SZREG	/* Calculate remainder */
> +	beqz	a4, 3f			/* Shortcut if no remainder */
> +	neg	a4, a4
> +	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
>  
>  	/* Adjust start address with offset */
> -	sub t0, t0, a4
> +	sub	t0, t0, a4
>  
>  	/* Jump into loop body */
>  	/* Assumes 32-bit instruction lengths */
> -	la a5, 3f
> +	la	a5, 3f
>  #ifdef CONFIG_64BIT
> -	srli a4, a4, 1
> +	srli	a4, a4, 1
>  #endif
> -	add a5, a5, a4
> -	jr a5
> +	add	a5, a5, a4
> +	jr	a5
>  3:
> -	REG_S a1,        0(t0)
> -	REG_S a1,    SZREG(t0)
> -	REG_S a1,  2*SZREG(t0)
> -	REG_S a1,  3*SZREG(t0)
> -	REG_S a1,  4*SZREG(t0)
> -	REG_S a1,  5*SZREG(t0)
> -	REG_S a1,  6*SZREG(t0)
> -	REG_S a1,  7*SZREG(t0)
> -	REG_S a1,  8*SZREG(t0)
> -	REG_S a1,  9*SZREG(t0)
> -	REG_S a1, 10*SZREG(t0)
> -	REG_S a1, 11*SZREG(t0)
> -	REG_S a1, 12*SZREG(t0)
> -	REG_S a1, 13*SZREG(t0)
> -	REG_S a1, 14*SZREG(t0)
> -	REG_S a1, 15*SZREG(t0)
> -	REG_S a1, 16*SZREG(t0)
> -	REG_S a1, 17*SZREG(t0)
> -	REG_S a1, 18*SZREG(t0)
> -	REG_S a1, 19*SZREG(t0)
> -	REG_S a1, 20*SZREG(t0)
> -	REG_S a1, 21*SZREG(t0)
> -	REG_S a1, 22*SZREG(t0)
> -	REG_S a1, 23*SZREG(t0)
> -	REG_S a1, 24*SZREG(t0)
> -	REG_S a1, 25*SZREG(t0)
> -	REG_S a1, 26*SZREG(t0)
> -	REG_S a1, 27*SZREG(t0)
> -	REG_S a1, 28*SZREG(t0)
> -	REG_S a1, 29*SZREG(t0)
> -	REG_S a1, 30*SZREG(t0)
> -	REG_S a1, 31*SZREG(t0)
> -	addi t0, t0, 32*SZREG
> -	bltu t0, a3, 3b
> -	andi a2, a2, SZREG-1  /* Update count */
> +	REG_S	a1,        0(t0)
> +	REG_S	a1,    SZREG(t0)
> +	REG_S	a1,  2*SZREG(t0)
> +	REG_S	a1,  3*SZREG(t0)
> +	REG_S	a1,  4*SZREG(t0)
> +	REG_S	a1,  5*SZREG(t0)
> +	REG_S	a1,  6*SZREG(t0)
> +	REG_S	a1,  7*SZREG(t0)
> +	REG_S	a1,  8*SZREG(t0)
> +	REG_S	a1,  9*SZREG(t0)
> +	REG_S	a1, 10*SZREG(t0)
> +	REG_S	a1, 11*SZREG(t0)
> +	REG_S	a1, 12*SZREG(t0)
> +	REG_S	a1, 13*SZREG(t0)
> +	REG_S	a1, 14*SZREG(t0)
> +	REG_S	a1, 15*SZREG(t0)
> +	REG_S	a1, 16*SZREG(t0)
> +	REG_S	a1, 17*SZREG(t0)
> +	REG_S	a1, 18*SZREG(t0)
> +	REG_S	a1, 19*SZREG(t0)
> +	REG_S	a1, 20*SZREG(t0)
> +	REG_S	a1, 21*SZREG(t0)
> +	REG_S	a1, 22*SZREG(t0)
> +	REG_S	a1, 23*SZREG(t0)
> +	REG_S	a1, 24*SZREG(t0)
> +	REG_S	a1, 25*SZREG(t0)
> +	REG_S	a1, 26*SZREG(t0)
> +	REG_S	a1, 27*SZREG(t0)
> +	REG_S	a1, 28*SZREG(t0)
> +	REG_S	a1, 29*SZREG(t0)
> +	REG_S	a1, 30*SZREG(t0)
> +	REG_S	a1, 31*SZREG(t0)
> +
> +	addi	t0, t0, 32*SZREG
> +	bltu	t0, a3, 3b
> +	andi	a2, a2, SZREG-1		/* Update count */
>  
>  4:
>  	/* Handle trailing misalignment */
> -	beqz a2, 6f
> -	add a3, t0, a2
> +	beqz	a2, 6f
> +	add	a3, t0, a2
>  5:
> -	sb a1, 0(t0)
> -	addi t0, t0, 1
> -	bltu t0, a3, 5b
> +	sb	a1, 0(t0)
> +	addi	t0, t0, 1
> +	bltu	t0, a3, 5b
>  6:
>  	ret
>  END(__memset)
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 8/9] RISC-V: lib: Use named labels in memset
  2022-10-27 13:02 ` [PATCH 8/9] RISC-V: lib: Use named labels in memset Andrew Jones
@ 2022-10-30 22:15   ` Conor Dooley
  2022-10-31  8:24     ` Andrew Jones
  0 siblings, 1 reply; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 22:15 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:46PM +0200, Andrew Jones wrote:
> In a coming patch we'll be adding more branch targets. Let's

rather minor nit: I don't think "In a coming patch" will be
particularly meaningful if anyone does some history diving.

> change the numeric labels to named labels to make it easier
> to read and integrate with.
> 
> No functional change intended.

It looks fine to me..
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> index e613c5c27998..74e4c7feec00 100644
> --- a/arch/riscv/lib/memset.S
> +++ b/arch/riscv/lib/memset.S
> @@ -13,7 +13,7 @@ WEAK(memset)
>  
>  	/* Defer to byte-oriented fill for small sizes */
>  	sltiu	a3, a2, 16
> -	bnez	a3, 4f
> +	bnez	a3, .Lfinish
>  
>  	/*
>  	 * Round to nearest XLEN-aligned address
> @@ -21,17 +21,18 @@ WEAK(memset)
>  	 */
>  	addi	a3, t0, SZREG-1
>  	andi	a3, a3, ~(SZREG-1)
> -	beq	a3, t0, 2f		/* Skip if already aligned */
> +	beq	a3, t0, .Ldo_duff	/* Skip if already aligned */
>  
>  	/* Handle initial misalignment */
>  	sub	a4, a3, t0
> -1:
> +.Lmisaligned1:
>  	sb	a1, 0(t0)
>  	addi	t0, t0, 1
> -	bltu	t0, a3, 1b
> +	bltu	t0, a3, .Lmisaligned1
>  	sub	a2, a2, a4		/* Update count */
>  
> -2: /* Duff's device with 32 XLEN stores per iteration */
> +.Ldo_duff:
> +	/* Duff's device with 32 XLEN stores per iteration */
>  	/* Broadcast value into all bytes */
>  	andi	a1, a1, 0xff
>  	slli	a3, a1, 8
> @@ -48,7 +49,7 @@ WEAK(memset)
>  	add	a3, t0, a4
>  
>  	andi	a4, a4, 31*SZREG	/* Calculate remainder */
> -	beqz	a4, 3f			/* Shortcut if no remainder */
> +	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
>  	neg	a4, a4
>  	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
>  
> @@ -57,13 +58,13 @@ WEAK(memset)
>  
>  	/* Jump into loop body */
>  	/* Assumes 32-bit instruction lengths */
> -	la	a5, 3f
> +	la	a5, .Lduff_loop
>  #ifdef CONFIG_64BIT
>  	srli	a4, a4, 1
>  #endif
>  	add	a5, a5, a4
>  	jr	a5
> -3:
> +.Lduff_loop:
>  	REG_S	a1,        0(t0)
>  	REG_S	a1,    SZREG(t0)
>  	REG_S	a1,  2*SZREG(t0)
> @@ -98,17 +99,17 @@ WEAK(memset)
>  	REG_S	a1, 31*SZREG(t0)
>  
>  	addi	t0, t0, 32*SZREG
> -	bltu	t0, a3, 3b
> +	bltu	t0, a3, .Lduff_loop
>  	andi	a2, a2, SZREG-1		/* Update count */
>  
> -4:
> +.Lfinish:
>  	/* Handle trailing misalignment */
> -	beqz	a2, 6f
> +	beqz	a2, .Ldone
>  	add	a3, t0, a2
> -5:
> +.Lmisaligned2:
>  	sb	a1, 0(t0)
>  	addi	t0, t0, 1
> -	bltu	t0, a3, 5b
> -6:
> +	bltu	t0, a3, .Lmisaligned2
> +.Ldone:
>  	ret
>  END(__memset)
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 9/9] RISC-V: Use Zicboz in memset when available
  2022-10-27 13:02 ` [PATCH 9/9] RISC-V: Use Zicboz in memset when available Andrew Jones
@ 2022-10-30 22:35   ` Conor Dooley
  2022-10-31  8:30     ` Andrew Jones
  2022-11-03  2:43   ` Palmer Dabbelt
  1 sibling, 1 reply; 40+ messages in thread
From: Conor Dooley @ 2022-10-30 22:35 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:47PM +0200, Andrew Jones wrote:
> RISC-V has an optimized memset() which does byte by byte writes up to
> the first sizeof(long) aligned address, then uses Duff's device until
> the last sizeof(long) aligned address, and finally byte by byte to
> the end. When memset is used to zero memory and the Zicboz extension
> is available, then we can extend that by doing the optimized memset
> up to the first Zicboz block size aligned address, then use the
> Zicboz zero instruction for each block to the last block size aligned
> address, and finally the optimized memset to the end.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> index 74e4c7feec00..786b85b5e9cc 100644
> --- a/arch/riscv/lib/memset.S
> +++ b/arch/riscv/lib/memset.S
> @@ -5,6 +5,12 @@
>  
>  #include <linux/linkage.h>
>  #include <asm/asm.h>
> +#include <asm/alternative-macros.h>
> +#include <asm/insn-def.h>
> +#include <asm/hwcap.h>
> +
> +#define ALT_ZICBOZ(old, new)	ALTERNATIVE(old, new, 0, RISCV_ISA_EXT_ZICBOZ, \
> +					    CONFIG_RISCV_ISA_ZICBOZ)
>  
>  /* void *memset(void *, int, size_t) */
>  ENTRY(__memset)
> @@ -15,6 +21,58 @@ WEAK(memset)
>  	sltiu	a3, a2, 16
>  	bnez	a3, .Lfinish
>  
> +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> +	ALT_ZICBOZ("j .Ldo_memset", "nop")
> +	/*
> +	 * t1 will be the Zicboz block size.
> +	 * Zero means we're not using Zicboz, and we don't when a1 != 0
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
I find this second half a little hard to parse. Do you mean "we don't
use zicboz when a1 != 0"? IOW, is my rewording of this comment accurate?
"A block size of zero means we're not using Zicboz. We also do not use
Zicboz when a1 is non zero".

> +	 */
> +	li	t1, 0
> +	bnez	a1, .Ldo_memset
> +	la	a3, riscv_cboz_block_size
> +	lw	t1, 0(a3)
> +
> +	/*
> +	 * Round to nearest Zicboz block-aligned address
> +	 * greater than or equal to the start address.
> +	 */
> +	addi	a3, t1, -1
> +	not	t2, a3			/* t2 is Zicboz block size mask */
> +	add	a3, t0, a3
> +	and	t3, a3, t2		/* t3 is Zicboz block aligned start */
> +
> +	/* Did we go too far or not have at least one block? */

This one is a little hard too, I think it's because you're switching
from "did" to "have". Maybe this is only an issue for me because this
stuff is beyond me in terms of reviewing, so I relying on the comments a
lot - although I suppose that makes me the target audience in a way.

I think it'd make more sense to me as "Did we go too far, or did we not
find any blocks".

Thanks,
Conor.

> +	add	a3, a0, a2
> +	and	a3, a3, t2
> +	bgtu	a3, t3, .Ldo_zero
> +	li	t1, 0
> +	j	.Ldo_memset
> +
> +.Ldo_zero:
> +	/* Use Duff for initial bytes if there are any */
> +	bne	t3, t0, .Ldo_memset
> +
> +.Ldo_zero2:
> +	/* Calculate end address */
> +	and	a3, a2, t2
> +	add	a3, t0, a3
> +	sub	a4, a3, t0
> +
> +.Lzero_loop:
> +	CBO_ZERO(t0)
> +	add	t0, t0, t1
> +	bltu	t0, a3, .Lzero_loop
> +	li	t1, 0			/* We're done with Zicboz */
> +
> +	sub	a2, a2, a4		/* Update count */
> +	sltiu	a3, a2, 16
> +	bnez	a3, .Lfinish
> +
> +	/* t0 is Zicboz block size aligned, so it must be SZREG aligned */
> +	j	.Ldo_duff3
> +#endif
> +
> +.Ldo_memset:
>  	/*
>  	 * Round to nearest XLEN-aligned address
>  	 * greater than or equal to the start address.
> @@ -33,6 +91,18 @@ WEAK(memset)
>  
>  .Ldo_duff:
>  	/* Duff's device with 32 XLEN stores per iteration */
> +
> +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> +	ALT_ZICBOZ("j .Ldo_duff2", "nop")
> +	beqz	t1, .Ldo_duff2
> +	/* a3, "end", is start of block aligned start. a1 is 0 */
> +	move    a3, t3
> +	sub	a4, a3, t0		/* a4 is SZREG aligned count */
> +	move	t4, a4			/* Save count for later, see below. */
> +	j	.Ldo_duff4
> +#endif
> +
> +.Ldo_duff2:
>  	/* Broadcast value into all bytes */
>  	andi	a1, a1, 0xff
>  	slli	a3, a1, 8
> @@ -44,10 +114,12 @@ WEAK(memset)
>  	or	a1, a3, a1
>  #endif
>  
> +.Ldo_duff3:
>  	/* Calculate end address */
>  	andi	a4, a2, ~(SZREG-1)
>  	add	a3, t0, a4
>  
> +.Ldo_duff4:
>  	andi	a4, a4, 31*SZREG	/* Calculate remainder */
>  	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
>  	neg	a4, a4
> @@ -100,6 +172,15 @@ WEAK(memset)
>  
>  	addi	t0, t0, 32*SZREG
>  	bltu	t0, a3, .Lduff_loop
> +
> +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> +	ALT_ZICBOZ("j .Lcount_update", "nop")
> +	beqz	t1, .Lcount_update
> +	sub	a2, a2, t4		/* Difference was saved above */
> +	j	.Ldo_zero2
> +#endif
> +
> +.Lcount_update:
>  	andi	a2, a2, SZREG-1		/* Update count */
>  
>  .Lfinish:
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> 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] 40+ messages in thread

* Re: [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop
  2022-10-30 20:31   ` Conor Dooley
@ 2022-10-31  8:11     ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-31  8:11 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Oct 30, 2022 at 08:31:46PM +0000, Conor Dooley wrote:
> On Thu, Oct 27, 2022 at 03:02:39PM +0200, Andrew Jones wrote:
> > Refactor riscv_init_cbom_blocksize() to prepare for it to be used
> > for both cbom block size and cboz block size.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/mm/cacheflush.c | 45 +++++++++++++++++++++-----------------
> >  1 file changed, 25 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> > index 57b40a350420..f096b9966cae 100644
> > --- a/arch/riscv/mm/cacheflush.c
> > +++ b/arch/riscv/mm/cacheflush.c
> > @@ -91,34 +91,39 @@ void flush_icache_pte(pte_t pte)
> >  unsigned int riscv_cbom_block_size;
> >  EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
> >  
> > +static void cbo_get_block_size(struct device_node *node,
> > +			       const char *name, u32 *blksz,
> 
> Is there a reason you called this "blksz" when we are using the spelt
> out "block_size" everywhere else in this code? My OCD would appreciate
> the consistency :s

I just tend to prefer shorter names for local, short-lived variables
as it sometimes helps avoid line wrapping, but I'm fine with naming it
block_size as well and can do the rename for v2 if you'd like.

> 
> Other than that:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
drew

> 
> > +			       unsigned long *first_hartid)
> > +{
> > +	unsigned long hartid;
> > +	u32 val;
> > +
> > +	if (riscv_of_processor_hartid(node, &hartid))
> > +		return;
> > +
> > +	if (of_property_read_u32(node, name, &val))
> > +		return;
> > +
> > +	if (!*blksz) {
> > +		*blksz = val;
> > +		*first_hartid = hartid;
> > +	} else if (*blksz != val) {
> > +		pr_warn("%s mismatched between harts %lu and %lu\n",
> > +			name, *first_hartid, hartid);
> > +	}
> > +}
> > +
> >  void riscv_init_cbom_blocksize(void)
> >  {
> >  	struct device_node *node;
> >  	unsigned long cbom_hartid;
> > -	u32 val, probed_block_size;
> > -	int ret;
> > +	u32 probed_block_size;
> >  
> >  	probed_block_size = 0;
> >  	for_each_of_cpu_node(node) {
> > -		unsigned long hartid;
> > -
> > -		ret = riscv_of_processor_hartid(node, &hartid);
> > -		if (ret)
> > -			continue;
> > -
> >  		/* set block-size for cbom extension if available */
> > -		ret = of_property_read_u32(node, "riscv,cbom-block-size", &val);
> > -		if (ret)
> > -			continue;
> > -
> > -		if (!probed_block_size) {
> > -			probed_block_size = val;
> > -			cbom_hartid = hartid;
> > -		} else {
> > -			if (probed_block_size != val)
> > -				pr_warn("cbom-block-size mismatched between harts %lu and %lu\n",
> > -					cbom_hartid, hartid);
> > -		}
> > +		cbo_get_block_size(node, "riscv,cbom-block-size",
> > +				   &probed_block_size, &cbom_hartid);
> >  	}
> >  
> >  	if (probed_block_size)
> > -- 
> > 2.37.3
> > 
> > 
> > _______________________________________________
> > 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] 40+ messages in thread

* Re: [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing
  2022-10-30 20:47   ` Conor Dooley
@ 2022-10-31  8:12     ` Andrew Jones
  2022-11-13 22:24     ` Conor Dooley
  1 sibling, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-31  8:12 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Oct 30, 2022 at 08:47:46PM +0000, Conor Dooley wrote:
> On Thu, Oct 27, 2022 at 03:02:40PM +0200, Andrew Jones wrote:
> > Mostly follow the same pattern as Zicbom, but leave out the toolchain
> > checks as we plan to use the insn-def framework for the cbo.zero
> > instruction.
> 
> nit: I'd prefer to see a commit message that stands on it's own without
> relying on someone knowing how zicbom has been wired up.

Sure, I'll rewrite this for v2.

> 
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/Kconfig                  | 13 +++++++++++++
> >  arch/riscv/include/asm/cacheflush.h |  3 ++-
> >  arch/riscv/include/asm/hwcap.h      |  1 +
> >  arch/riscv/kernel/cpu.c             |  1 +
> >  arch/riscv/kernel/cpufeature.c      | 10 ++++++++++
> >  arch/riscv/kernel/setup.c           |  2 +-
> >  arch/riscv/mm/cacheflush.c          | 23 +++++++++++++++--------
> >  7 files changed, 43 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 6b48a3ae9843..c20e6fa0c0b1 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -433,6 +433,19 @@ config RISCV_ISA_ZICBOM
> >  
> >  	   If you don't know what to do here, say Y.
> >  
> > +config RISCV_ISA_ZICBOZ
> > +	bool "Zicboz extension support for faster zeroing of memory"
> > +	depends on !XIP_KERNEL && MMU
> > +	select RISCV_ALTERNATIVE
> > +	default y
> > +	help
> > +	   Adds support to dynamically detect the presence of the ZICBOZ
> > +	   extension (cbo.zero instruction) and enable its usage.
> > +
> > +	   The Zicboz extension is used for faster zeroing of memory.
> > +
> > +	   If you don't know what to do here, say Y.
> > +
> >  config FPU
> >  	bool "FPU support"
> >  	default y
> > diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
> > index f6fbe7042f1c..5b31568cf5e6 100644
> > --- a/arch/riscv/include/asm/cacheflush.h
> > +++ b/arch/riscv/include/asm/cacheflush.h
> > @@ -43,7 +43,8 @@ void flush_icache_mm(struct mm_struct *mm, bool local);
> >  #endif /* CONFIG_SMP */
> >  
> >  extern unsigned int riscv_cbom_block_size;
> > -void riscv_init_cbom_blocksize(void);
> > +extern unsigned int riscv_cboz_block_size;
> > +void riscv_init_cbo_blocksizes(void);
> >  
> >  #ifdef CONFIG_RISCV_DMA_NONCOHERENT
> >  void riscv_noncoherent_supported(void);
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 5d6492bde446..eaa5a972ad2d 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -45,6 +45,7 @@
> >  #define RISCV_ISA_EXT_ZIHINTPAUSE	29
> >  #define RISCV_ISA_EXT_SSTC		30
> >  #define RISCV_ISA_EXT_SVINVAL		31
> > +#define RISCV_ISA_EXT_ZICBOZ		32
> >  
> >  #define RISCV_ISA_EXT_ID_MAX		RISCV_ISA_EXT_MAX
> >  
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fa427bdcf773..bf969218f609 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -144,6 +144,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> >  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >  	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> >  	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> > +	__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> >  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> >  };
> >  
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 18b9ed4df1f4..e13b3391de76 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -78,6 +78,15 @@ static bool riscv_isa_extension_check(int id)
> >  			return false;
> >  		}
> >  		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");
> > +			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");
> > +			return false;
> > +		}
> > +		return true;
> >  	}
> >  
> >  	return true;
> > @@ -225,6 +234,7 @@ void __init riscv_fill_hwcap(void)
> >  				SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> >  				SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> >  				SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> > +				SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> >  			}
> >  #undef SET_ISA_EXT_MAP
> >  		}
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index a07917551027..26de0d8fd23d 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -296,7 +296,7 @@ void __init setup_arch(char **cmdline_p)
> >  	setup_smp();
> >  #endif
> >  
> > -	riscv_init_cbom_blocksize();
> > +	riscv_init_cbo_blocksizes();
> >  	riscv_fill_hwcap();
> >  	apply_boot_alternatives();
> >  #ifdef CONFIG_RISCV_DMA_NONCOHERENT
> > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> > index f096b9966cae..208e0d58bde3 100644
> > --- a/arch/riscv/mm/cacheflush.c
> > +++ b/arch/riscv/mm/cacheflush.c
> > @@ -91,6 +91,9 @@ void flush_icache_pte(pte_t pte)
> >  unsigned int riscv_cbom_block_size;
> >  EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
> >  
> > +unsigned int riscv_cboz_block_size;
> > +EXPORT_SYMBOL_GPL(riscv_cboz_block_size);
> > +
> >  static void cbo_get_block_size(struct device_node *node,
> >  			       const char *name, u32 *blksz,
> >  			       unsigned long *first_hartid)
> > @@ -113,19 +116,23 @@ static void cbo_get_block_size(struct device_node *node,
> >  	}
> >  }
> >  
> > -void riscv_init_cbom_blocksize(void)
> > +void riscv_init_cbo_blocksizes(void)
> >  {
> > +	unsigned long cbom_hartid, cboz_hartid;
> > +	u32 cbom_blksz = 0, cboz_blksz = 0;
> >  	struct device_node *node;
> > -	unsigned long cbom_hartid;
> > -	u32 probed_block_size;
> >  
> > -	probed_block_size = 0;
> >  	for_each_of_cpu_node(node) {
> > -		/* set block-size for cbom extension if available */
> > +		/* set block-size for cbom and/or cboz extension if available */
> >  		cbo_get_block_size(node, "riscv,cbom-block-size",
> > -				   &probed_block_size, &cbom_hartid);
> > +				   &cbom_blksz, &cbom_hartid);
> > +		cbo_get_block_size(node, "riscv,cboz-block-size",
> > +				   &cboz_blksz, &cboz_hartid);
> >  	}
> >  
> > -	if (probed_block_size)
> > -		riscv_cbom_block_size = probed_block_size;
> > +	if (cbom_blksz)
> > +		riscv_cbom_block_size = cbom_blksz;
> 
> As in patch 1, can you stick with s/blksz/block_size/ please.

Will do.

> My poor OCD!!! Other than those minor things:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
drew

> 
> > +
> > +	if (cboz_blksz)
> > +		riscv_cboz_block_size = cboz_blksz;
> >  }
> > -- 
> > 2.37.3
> > 
> > 
> > _______________________________________________
> > 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] 40+ messages in thread

* Re: [PATCH 3/9] RISC-V: insn-def: Define cbo.zero
  2022-10-30 21:08   ` Conor Dooley
@ 2022-10-31  8:18     ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-31  8:18 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Oct 30, 2022 at 09:08:35PM +0000, Conor Dooley wrote:
...
> > +#define DEFINE_INSN_I							\
> > +	__DEFINE_ASM_GPR_NUMS						\
> > +"	.macro insn_i, opcode, func3, rd, rs1, simm12\n"		\
> > +"	.4byte	((\\opcode << " __stringify(INSN_I_OPCODE_SHIFT) ") |"	\
> > +"		 (\\func3 << " __stringify(INSN_I_FUNC3_SHIFT) ") |"	\
> > +"		 (.L__gpr_num_\\rd << " __stringify(INSN_I_RD_SHIFT) ") |"   \
> > +"		 (.L__gpr_num_\\rs1 << " __stringify(INSN_I_RS1_SHIFT) ") |" \
> 
> It'd be nice if these macros had aligned \s. I know the file is pretty
> inconsistent in that regard and I'm not asking you to change those but I
> think it'd be nice to do so for stuff that's just being added now.

My approach has been to tab them out without crossing the 80 char "limit"
and then when the next tab would go past 80, I use minimum spaces instead.
I realize checkpatch has changed to 100, though, so maybe I should apply
the same approach but with 20 extra characters, which will hopefully mean
tab always works.

Thanks,
drew

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

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

* Re: [PATCH 8/9] RISC-V: lib: Use named labels in memset
  2022-10-30 22:15   ` Conor Dooley
@ 2022-10-31  8:24     ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-31  8:24 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Oct 30, 2022 at 10:15:43PM +0000, Conor Dooley wrote:
> On Thu, Oct 27, 2022 at 03:02:46PM +0200, Andrew Jones wrote:
> > In a coming patch we'll be adding more branch targets. Let's
> 
> rather minor nit: I don't think "In a coming patch" will be
> particularly meaningful if anyone does some history diving.

Yes, I struggle with these. I think "next patch" is typically frowned upon
because things get reordered and then 'next' is no longer appropriate, but
it is good to convey somehow that while this patch doesn't provide much
use on its own it does serve a preparation purpose. Maybe I should just
write "Improve readability and prepare for extensions which will add
more labels..."

I'll do something like that for v2.

> 
> > change the numeric labels to named labels to make it easier
> > to read and integrate with.
> > 
> > No functional change intended.
> 
> It looks fine to me..
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
drew

> 
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> > index e613c5c27998..74e4c7feec00 100644
> > --- a/arch/riscv/lib/memset.S
> > +++ b/arch/riscv/lib/memset.S
> > @@ -13,7 +13,7 @@ WEAK(memset)
> >  
> >  	/* Defer to byte-oriented fill for small sizes */
> >  	sltiu	a3, a2, 16
> > -	bnez	a3, 4f
> > +	bnez	a3, .Lfinish
> >  
> >  	/*
> >  	 * Round to nearest XLEN-aligned address
> > @@ -21,17 +21,18 @@ WEAK(memset)
> >  	 */
> >  	addi	a3, t0, SZREG-1
> >  	andi	a3, a3, ~(SZREG-1)
> > -	beq	a3, t0, 2f		/* Skip if already aligned */
> > +	beq	a3, t0, .Ldo_duff	/* Skip if already aligned */
> >  
> >  	/* Handle initial misalignment */
> >  	sub	a4, a3, t0
> > -1:
> > +.Lmisaligned1:
> >  	sb	a1, 0(t0)
> >  	addi	t0, t0, 1
> > -	bltu	t0, a3, 1b
> > +	bltu	t0, a3, .Lmisaligned1
> >  	sub	a2, a2, a4		/* Update count */
> >  
> > -2: /* Duff's device with 32 XLEN stores per iteration */
> > +.Ldo_duff:
> > +	/* Duff's device with 32 XLEN stores per iteration */
> >  	/* Broadcast value into all bytes */
> >  	andi	a1, a1, 0xff
> >  	slli	a3, a1, 8
> > @@ -48,7 +49,7 @@ WEAK(memset)
> >  	add	a3, t0, a4
> >  
> >  	andi	a4, a4, 31*SZREG	/* Calculate remainder */
> > -	beqz	a4, 3f			/* Shortcut if no remainder */
> > +	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
> >  	neg	a4, a4
> >  	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
> >  
> > @@ -57,13 +58,13 @@ WEAK(memset)
> >  
> >  	/* Jump into loop body */
> >  	/* Assumes 32-bit instruction lengths */
> > -	la	a5, 3f
> > +	la	a5, .Lduff_loop
> >  #ifdef CONFIG_64BIT
> >  	srli	a4, a4, 1
> >  #endif
> >  	add	a5, a5, a4
> >  	jr	a5
> > -3:
> > +.Lduff_loop:
> >  	REG_S	a1,        0(t0)
> >  	REG_S	a1,    SZREG(t0)
> >  	REG_S	a1,  2*SZREG(t0)
> > @@ -98,17 +99,17 @@ WEAK(memset)
> >  	REG_S	a1, 31*SZREG(t0)
> >  
> >  	addi	t0, t0, 32*SZREG
> > -	bltu	t0, a3, 3b
> > +	bltu	t0, a3, .Lduff_loop
> >  	andi	a2, a2, SZREG-1		/* Update count */
> >  
> > -4:
> > +.Lfinish:
> >  	/* Handle trailing misalignment */
> > -	beqz	a2, 6f
> > +	beqz	a2, .Ldone
> >  	add	a3, t0, a2
> > -5:
> > +.Lmisaligned2:
> >  	sb	a1, 0(t0)
> >  	addi	t0, t0, 1
> > -	bltu	t0, a3, 5b
> > -6:
> > +	bltu	t0, a3, .Lmisaligned2
> > +.Ldone:
> >  	ret
> >  END(__memset)
> > -- 
> > 2.37.3
> > 
> > 
> > _______________________________________________
> > 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] 40+ messages in thread

* Re: [PATCH 9/9] RISC-V: Use Zicboz in memset when available
  2022-10-30 22:35   ` Conor Dooley
@ 2022-10-31  8:30     ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-31  8:30 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Oct 30, 2022 at 10:35:47PM +0000, Conor Dooley wrote:
> On Thu, Oct 27, 2022 at 03:02:47PM +0200, Andrew Jones wrote:
> > RISC-V has an optimized memset() which does byte by byte writes up to
> > the first sizeof(long) aligned address, then uses Duff's device until
> > the last sizeof(long) aligned address, and finally byte by byte to
> > the end. When memset is used to zero memory and the Zicboz extension
> > is available, then we can extend that by doing the optimized memset
> > up to the first Zicboz block size aligned address, then use the
> > Zicboz zero instruction for each block to the last block size aligned
> > address, and finally the optimized memset to the end.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> > 
> > diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> > index 74e4c7feec00..786b85b5e9cc 100644
> > --- a/arch/riscv/lib/memset.S
> > +++ b/arch/riscv/lib/memset.S
> > @@ -5,6 +5,12 @@
> >  
> >  #include <linux/linkage.h>
> >  #include <asm/asm.h>
> > +#include <asm/alternative-macros.h>
> > +#include <asm/insn-def.h>
> > +#include <asm/hwcap.h>
> > +
> > +#define ALT_ZICBOZ(old, new)	ALTERNATIVE(old, new, 0, RISCV_ISA_EXT_ZICBOZ, \
> > +					    CONFIG_RISCV_ISA_ZICBOZ)
> >  
> >  /* void *memset(void *, int, size_t) */
> >  ENTRY(__memset)
> > @@ -15,6 +21,58 @@ WEAK(memset)
> >  	sltiu	a3, a2, 16
> >  	bnez	a3, .Lfinish
> >  
> > +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> > +	ALT_ZICBOZ("j .Ldo_memset", "nop")
> > +	/*
> > +	 * t1 will be the Zicboz block size.
> > +	 * Zero means we're not using Zicboz, and we don't when a1 != 0
>                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> I find this second half a little hard to parse. Do you mean "we don't
> use zicboz when a1 != 0"? IOW, is my rewording of this comment accurate?
> "A block size of zero means we're not using Zicboz. We also do not use
> Zicboz when a1 is non zero".

Yup. I'll use your words in v2.

> 
> > +	 */
> > +	li	t1, 0
> > +	bnez	a1, .Ldo_memset
> > +	la	a3, riscv_cboz_block_size
> > +	lw	t1, 0(a3)
> > +
> > +	/*
> > +	 * Round to nearest Zicboz block-aligned address
> > +	 * greater than or equal to the start address.
> > +	 */
> > +	addi	a3, t1, -1
> > +	not	t2, a3			/* t2 is Zicboz block size mask */
> > +	add	a3, t0, a3
> > +	and	t3, a3, t2		/* t3 is Zicboz block aligned start */
> > +
> > +	/* Did we go too far or not have at least one block? */
> 
> This one is a little hard too, I think it's because you're switching
> from "did" to "have". Maybe this is only an issue for me because this
> stuff is beyond me in terms of reviewing, so I relying on the comments a
> lot - although I suppose that makes me the target audience in a way.
> 
> I think it'd make more sense to me as "Did we go too far, or did we not
> find any blocks".

OK, I'll also take those words for v2.

Thanks,
drew

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

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

* Re: [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
  2022-10-30 20:23   ` Conor Dooley
@ 2022-10-31  8:39     ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-10-31  8:39 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Oct 30, 2022 at 08:23:27PM +0000, Conor Dooley wrote:
> On Sat, Oct 29, 2022 at 11:59:53AM +0200, Andrew Jones wrote:
> > On Thu, Oct 27, 2022 at 03:02:38PM +0200, Andrew Jones wrote:
> > > When the Zicboz extension is available we can more rapidly zero naturally
> > > aligned Zicboz block sized chunks of memory. As pages are always page
> > > aligned and are larger than any Zicboz block size will be, then
> > > clear_page() appears to be a good candidate for the extension. While cycle
> > > count and energy consumption should also be considered, we can be pretty
> > > certain that implementing clear_page() with the Zicboz extension is a win
> > > by comparing the new dynamic instruction count with its current count[1].
> > > Doing so we see that the new count is less than half the old count (see
> > > patch4's commit message for more details). Another candidate for the
> > > extension is memset(), but, since memset() isn't just used for zeroing
> > > memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
> > > it's not as obvious if adding support for Zicboz will be an overall win.
> > > In order to make a determination, I've done some analysis and wrote my
> > > conclusions in the bullets below.
> > > 
> > > * When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
> > >   doesn't change, so that's fine.
> > > 
> > > * The overhead added to memset() when the Zicboz extension isn't present,
> > >   but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
> > >   which I believe is fine.
> > > 
> > > * The overhead added to a memset() invocation which is not zeroing memory
> > >   is 7 instructions, where 3 are branches. This seems fine and,
> > >   furthermore, memset() is almost always invoked to zero memory (99% [2]).
> > > 
> > > * When memset() is invoked to zero memory, the proposed Zicboz extended
> > >   memset() always has a lower dynamic instruction count than the current
> > >   memset() as long as the input address is Zicboz block aligned and the
> > >   length is >= the block size.
> > > 
> > > * When memset() is invoked to zero memory, the proposed Zicboz extended
> > >   memset() is always worse for unaligned or too small inputs than the
> > >   current memset(), but it's only at most a few dozen instructions worse.
> > >   I think this is probably fine, especially considering the large majority
> > >   of zeroing invocations are 64 bytes or larger and are aligned to a
> > >   power-of-2 boundary, 64-byte or larger (77% [2]).
> > > 
> > > [1] I ported the functions under test to userspace and linked them with
> > >     a test program. Then, I ran them under gdb with a script[3] which
> > >     counted instructions by single stepping.
> > > 
> > > [2] I wrote bpftrace scripts[4] to count memset() invocations to see the
> > >     frequency of it being used to zero memory and have block size aligned
> > >     input addresses with block size or larger lengths. The workload was
> > >     just random desktop stuff including streaming video and compiling.
> > >     While I did run this on my x86 notebook, I still expect the data to
> > >     be representative on RISC-V. Note, x86 has clear_page() so the
> > >     memset() data regarding alignment and size weren't over inflated by
> > >     page zeroing invocations. Grepping also shows the large majority of
> > >     memset() calls are to zero memory (93%).
> > > 
> > > [3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
> > > [4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe
> > > 
> > > These patches are based on the following pending series
> > > 
> > > 1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
> > >    20221024091309.406906-1-ajones@ventanamicro.com
> > > 
> > > 2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
> > >    20221006070818.3616-1-jszhang@kernel.org
> > >    Also including the additional patch proposed here
> > >    20221013162038.ehseju2neic2xu5z@kamzik
> > > 
> > > The patches are also available here
> > > https://github.com/jones-drew/linux/commits/riscv/zicboz
> > > 
> > > To test over QEMU this branch may be used to enable Zicboz
> > > https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz
> > > 
> > > To test running a KVM guest with Zicboz this kvmtool branch may be used
> > > https://github.com/jones-drew/kvmtool/commits/riscv/zicboz
> > > 
> > > Thanks,
> > > drew
> > > 
> > > Andrew Jones (9):
> > >   RISC-V: Factor out body of riscv_init_cbom_blocksize loop
> > >   RISC-V: Add Zicboz detection and block size parsing
> > >   RISC-V: insn-def: Define cbo.zero
> > >   RISC-V: Use Zicboz in clear_page when available
> > >   RISC-V: KVM: Provide UAPI for Zicboz block size
> > >   RISC-V: KVM: Expose Zicboz to the guest
> > >   RISC-V: lib: Improve memset assembler formatting
> > >   RISC-V: lib: Use named labels in memset
> > >   RISC-V: Use Zicboz in memset when available
> > > 
> > >  arch/riscv/Kconfig                  |  13 ++
> > >  arch/riscv/include/asm/cacheflush.h |   3 +-
> > >  arch/riscv/include/asm/hwcap.h      |   1 +
> > >  arch/riscv/include/asm/insn-def.h   |  50 ++++++
> > >  arch/riscv/include/asm/page.h       |   6 +-
> > >  arch/riscv/include/uapi/asm/kvm.h   |   2 +
> > >  arch/riscv/kernel/cpu.c             |   1 +
> > >  arch/riscv/kernel/cpufeature.c      |  10 ++
> > >  arch/riscv/kernel/setup.c           |   2 +-
> > >  arch/riscv/kvm/vcpu.c               |  11 ++
> > >  arch/riscv/lib/Makefile             |   1 +
> > >  arch/riscv/lib/clear_page.S         |  28 ++++
> > >  arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
> > >  arch/riscv/mm/cacheflush.c          |  64 +++++---
> > >  14 files changed, 325 insertions(+), 108 deletions(-)
> > >  create mode 100644 arch/riscv/lib/clear_page.S
> > > 
> > > -- 
> > > 2.37.3
> > >
> > 
> > FYI, I just tried this with clang. It compiles but doesn't boot when
> > Zicboz is present (it does boot when Zicboz is disabled in QEMU). I'm
> > suspicious of the ALTERNATIVE() stuff, but I'll debug more on Monday.
> > Also, I see building with LLVM=1 doesn't work, but that appears to be
> > an issue introduced with "[PATCH 0/8] riscv: improve boot time isa
> > extensions handling" which this series is based on.
> 
> Yeah, I was hitting the LLVM failures when I gave it a go the other day,
> thanks for passing it on to Jisheng.. I meant to do that and forgot.
> FWIW, I did find a build error with gcc too:
> 
> riscv64-unknown-linux-gnu-ld: arch/riscv/purgatory/purgatory.ro: in function `.L3␂1':
> ctype.c:(.text+0x215a): undefined reference to `riscv_cboz_block_size'

Oh, I didn't see that with my toolchain
 - gcc 12.1.0 (g1ea978e3066a)
 - binutils 2.39

I'll see if I can reproduce it.

Thanks,
drew

> 
> defconfig should be:
> https://gist.github.com/ConchuOD/e14293a28b04dca2125bdcc0d97d366a
> & toolchain stuff:
> CONFIG_CC_VERSION_TEXT="riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0"
> CONFIG_CC_IS_GCC=y
> CONFIG_GCC_VERSION=110100
> CONFIG_AS_IS_GNU=y
> CONFIG_AS_VERSION=23700
> CONFIG_LD_IS_BFD=y
> CONFIG_LD_VERSION=23700
> 
> My toolchain predates zicbo* being introduced & I set the config option.
> I didn't do any specific digging as to the cause.
> 
> Thanks,
> Conor.
> 

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

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

* Re: [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (9 preceding siblings ...)
  2022-10-29  9:59 ` [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
@ 2022-11-01 10:37 ` Andrew Jones
  2022-11-01 10:53   ` Andrew Jones
  2022-12-20 12:55 ` Conor Dooley
  11 siblings, 1 reply; 40+ messages in thread
From: Andrew Jones @ 2022-11-01 10:37 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

On Thu, Oct 27, 2022 at 03:02:38PM +0200, Andrew Jones wrote:
> When the Zicboz extension is available we can more rapidly zero naturally
> aligned Zicboz block sized chunks of memory. As pages are always page
> aligned and are larger than any Zicboz block size will be, then
> clear_page() appears to be a good candidate for the extension. While cycle
> count and energy consumption should also be considered, we can be pretty
> certain that implementing clear_page() with the Zicboz extension is a win
> by comparing the new dynamic instruction count with its current count[1].
> Doing so we see that the new count is less than half the old count (see
> patch4's commit message for more details). Another candidate for the
> extension is memset(), but, since memset() isn't just used for zeroing
> memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
> it's not as obvious if adding support for Zicboz will be an overall win.
> In order to make a determination, I've done some analysis and wrote my
> conclusions in the bullets below.
> 
> * When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
>   doesn't change, so that's fine.
> 
> * The overhead added to memset() when the Zicboz extension isn't present,
>   but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
>   which I believe is fine.
> 
> * The overhead added to a memset() invocation which is not zeroing memory
>   is 7 instructions, where 3 are branches. This seems fine and,
>   furthermore, memset() is almost always invoked to zero memory (99% [2]).
> 
> * When memset() is invoked to zero memory, the proposed Zicboz extended
>   memset() always has a lower dynamic instruction count than the current
>   memset() as long as the input address is Zicboz block aligned and the
>   length is >= the block size.
> 
> * When memset() is invoked to zero memory, the proposed Zicboz extended
>   memset() is always worse for unaligned or too small inputs than the
>   current memset(), but it's only at most a few dozen instructions worse.
>   I think this is probably fine, especially considering the large majority
>   of zeroing invocations are 64 bytes or larger and are aligned to a
>   power-of-2 boundary, 64-byte or larger (77% [2]).

FYI, I spotted a bug in the bpf script used for this result, so I fixed it
and reran it. The new result is 67%, which is no longer a large majority,
but still a solid majority.

> 
> [1] I ported the functions under test to userspace and linked them with
>     a test program. Then, I ran them under gdb with a script[3] which
>     counted instructions by single stepping.
> 
> [2] I wrote bpftrace scripts[4] to count memset() invocations to see the
>     frequency of it being used to zero memory and have block size aligned
>     input addresses with block size or larger lengths. The workload was
>     just random desktop stuff including streaming video and compiling.
>     While I did run this on my x86 notebook, I still expect the data to
>     be representative on RISC-V. Note, x86 has clear_page() so the
>     memset() data regarding alignment and size weren't over inflated by
>     page zeroing invocations. Grepping also shows the large majority of
>     memset() calls are to zero memory (93%).
> 
> [3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
> [4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe
> 
> These patches are based on the following pending series
> 
> 1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
>    20221024091309.406906-1-ajones@ventanamicro.com
> 
> 2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
>    20221006070818.3616-1-jszhang@kernel.org
>    Also including the additional patch proposed here
>    20221013162038.ehseju2neic2xu5z@kamzik
> 
> The patches are also available here
> https://github.com/jones-drew/linux/commits/riscv/zicboz
> 
> To test over QEMU this branch may be used to enable Zicboz
> https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz
> 
> To test running a KVM guest with Zicboz this kvmtool branch may be used
> https://github.com/jones-drew/kvmtool/commits/riscv/zicboz
> 
> Thanks,
> drew
> 
> Andrew Jones (9):
>   RISC-V: Factor out body of riscv_init_cbom_blocksize loop
>   RISC-V: Add Zicboz detection and block size parsing
>   RISC-V: insn-def: Define cbo.zero
>   RISC-V: Use Zicboz in clear_page when available
>   RISC-V: KVM: Provide UAPI for Zicboz block size
>   RISC-V: KVM: Expose Zicboz to the guest
>   RISC-V: lib: Improve memset assembler formatting
>   RISC-V: lib: Use named labels in memset
>   RISC-V: Use Zicboz in memset when available
> 
>  arch/riscv/Kconfig                  |  13 ++
>  arch/riscv/include/asm/cacheflush.h |   3 +-
>  arch/riscv/include/asm/hwcap.h      |   1 +
>  arch/riscv/include/asm/insn-def.h   |  50 ++++++
>  arch/riscv/include/asm/page.h       |   6 +-
>  arch/riscv/include/uapi/asm/kvm.h   |   2 +
>  arch/riscv/kernel/cpu.c             |   1 +
>  arch/riscv/kernel/cpufeature.c      |  10 ++
>  arch/riscv/kernel/setup.c           |   2 +-
>  arch/riscv/kvm/vcpu.c               |  11 ++
>  arch/riscv/lib/Makefile             |   1 +
>  arch/riscv/lib/clear_page.S         |  28 ++++
>  arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
>  arch/riscv/mm/cacheflush.c          |  64 +++++---
>  14 files changed, 325 insertions(+), 108 deletions(-)
>  create mode 100644 arch/riscv/lib/clear_page.S
> 
> -- 
> 2.37.3
> 

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

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

* Re: [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
  2022-11-01 10:37 ` Andrew Jones
@ 2022-11-01 10:53   ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-11-01 10:53 UTC (permalink / raw)
  To: linux-riscv, kvm-riscv
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Anup Patel,
	Heiko Stuebner, Conor Dooley, Atish Patra, Jisheng Zhang

On Tue, Nov 01, 2022 at 11:37:11AM +0100, Andrew Jones wrote:
> On Thu, Oct 27, 2022 at 03:02:38PM +0200, Andrew Jones wrote:
> > When the Zicboz extension is available we can more rapidly zero naturally
> > aligned Zicboz block sized chunks of memory. As pages are always page
> > aligned and are larger than any Zicboz block size will be, then
> > clear_page() appears to be a good candidate for the extension. While cycle
> > count and energy consumption should also be considered, we can be pretty
> > certain that implementing clear_page() with the Zicboz extension is a win
> > by comparing the new dynamic instruction count with its current count[1].
> > Doing so we see that the new count is less than half the old count (see
> > patch4's commit message for more details). Another candidate for the
> > extension is memset(), but, since memset() isn't just used for zeroing
> > memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
> > it's not as obvious if adding support for Zicboz will be an overall win.
> > In order to make a determination, I've done some analysis and wrote my
> > conclusions in the bullets below.
> > 
> > * When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
> >   doesn't change, so that's fine.
> > 
> > * The overhead added to memset() when the Zicboz extension isn't present,
> >   but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
> >   which I believe is fine.
> > 
> > * The overhead added to a memset() invocation which is not zeroing memory
> >   is 7 instructions, where 3 are branches. This seems fine and,
> >   furthermore, memset() is almost always invoked to zero memory (99% [2]).
> > 
> > * When memset() is invoked to zero memory, the proposed Zicboz extended
> >   memset() always has a lower dynamic instruction count than the current
> >   memset() as long as the input address is Zicboz block aligned and the
> >   length is >= the block size.
> > 
> > * When memset() is invoked to zero memory, the proposed Zicboz extended
> >   memset() is always worse for unaligned or too small inputs than the
> >   current memset(), but it's only at most a few dozen instructions worse.
> >   I think this is probably fine, especially considering the large majority
> >   of zeroing invocations are 64 bytes or larger and are aligned to a
> >   power-of-2 boundary, 64-byte or larger (77% [2]).
> 
> FYI, I spotted a bug in the bpf script used for this result, so I fixed it
> and reran it. The new result is 67%, which is no longer a large majority,
> but still a solid majority.

Additionally, I've now seen on a mostly idle system that at least half the
objects getting zeroed were too small (< 64 bytes). Considering the memset
profile is workload dependent, it's difficult to say with certainty that
we should patch it. More experiments are likely necessary.

> 
> > 
> > [1] I ported the functions under test to userspace and linked them with
> >     a test program. Then, I ran them under gdb with a script[3] which
> >     counted instructions by single stepping.
> > 
> > [2] I wrote bpftrace scripts[4] to count memset() invocations to see the
> >     frequency of it being used to zero memory and have block size aligned
> >     input addresses with block size or larger lengths. The workload was
> >     just random desktop stuff including streaming video and compiling.
> >     While I did run this on my x86 notebook, I still expect the data to
> >     be representative on RISC-V. Note, x86 has clear_page() so the
> >     memset() data regarding alignment and size weren't over inflated by
> >     page zeroing invocations. Grepping also shows the large majority of
> >     memset() calls are to zero memory (93%).
> > 
> > [3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
> > [4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe
> > 
> > These patches are based on the following pending series
> > 
> > 1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
> >    20221024091309.406906-1-ajones@ventanamicro.com
> > 
> > 2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
> >    20221006070818.3616-1-jszhang@kernel.org
> >    Also including the additional patch proposed here
> >    20221013162038.ehseju2neic2xu5z@kamzik
> > 
> > The patches are also available here
> > https://github.com/jones-drew/linux/commits/riscv/zicboz
> > 
> > To test over QEMU this branch may be used to enable Zicboz
> > https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz
> > 
> > To test running a KVM guest with Zicboz this kvmtool branch may be used
> > https://github.com/jones-drew/kvmtool/commits/riscv/zicboz
> > 
> > Thanks,
> > drew
> > 
> > Andrew Jones (9):
> >   RISC-V: Factor out body of riscv_init_cbom_blocksize loop
> >   RISC-V: Add Zicboz detection and block size parsing
> >   RISC-V: insn-def: Define cbo.zero
> >   RISC-V: Use Zicboz in clear_page when available
> >   RISC-V: KVM: Provide UAPI for Zicboz block size
> >   RISC-V: KVM: Expose Zicboz to the guest
> >   RISC-V: lib: Improve memset assembler formatting
> >   RISC-V: lib: Use named labels in memset
> >   RISC-V: Use Zicboz in memset when available
> > 
> >  arch/riscv/Kconfig                  |  13 ++
> >  arch/riscv/include/asm/cacheflush.h |   3 +-
> >  arch/riscv/include/asm/hwcap.h      |   1 +
> >  arch/riscv/include/asm/insn-def.h   |  50 ++++++
> >  arch/riscv/include/asm/page.h       |   6 +-
> >  arch/riscv/include/uapi/asm/kvm.h   |   2 +
> >  arch/riscv/kernel/cpu.c             |   1 +
> >  arch/riscv/kernel/cpufeature.c      |  10 ++
> >  arch/riscv/kernel/setup.c           |   2 +-
> >  arch/riscv/kvm/vcpu.c               |  11 ++
> >  arch/riscv/lib/Makefile             |   1 +
> >  arch/riscv/lib/clear_page.S         |  28 ++++
> >  arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
> >  arch/riscv/mm/cacheflush.c          |  64 +++++---
> >  14 files changed, 325 insertions(+), 108 deletions(-)
> >  create mode 100644 arch/riscv/lib/clear_page.S
> > 
> > -- 
> > 2.37.3
> > 

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

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

* Re: [PATCH 9/9] RISC-V: Use Zicboz in memset when available
  2022-10-27 13:02 ` [PATCH 9/9] RISC-V: Use Zicboz in memset when available Andrew Jones
  2022-10-30 22:35   ` Conor Dooley
@ 2022-11-03  2:43   ` Palmer Dabbelt
  2022-11-03 10:21     ` Andrew Jones
  1 sibling, 1 reply; 40+ messages in thread
From: Palmer Dabbelt @ 2022-11-03  2:43 UTC (permalink / raw)
  To: ajones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, aou, apatel, heiko,
	Conor Dooley, Atish Patra, jszhang

On Thu, 27 Oct 2022 06:02:47 PDT (-0700), ajones@ventanamicro.com wrote:
> RISC-V has an optimized memset() which does byte by byte writes up to
> the first sizeof(long) aligned address, then uses Duff's device until
> the last sizeof(long) aligned address, and finally byte by byte to
> the end. When memset is used to zero memory and the Zicboz extension
> is available, then we can extend that by doing the optimized memset
> up to the first Zicboz block size aligned address, then use the
> Zicboz zero instruction for each block to the last block size aligned
> address, and finally the optimized memset to the end.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>
> diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> index 74e4c7feec00..786b85b5e9cc 100644
> --- a/arch/riscv/lib/memset.S
> +++ b/arch/riscv/lib/memset.S
> @@ -5,6 +5,12 @@
>
>  #include <linux/linkage.h>
>  #include <asm/asm.h>
> +#include <asm/alternative-macros.h>
> +#include <asm/insn-def.h>
> +#include <asm/hwcap.h>
> +
> +#define ALT_ZICBOZ(old, new)	ALTERNATIVE(old, new, 0, RISCV_ISA_EXT_ZICBOZ, \
> +					    CONFIG_RISCV_ISA_ZICBOZ)
>
>  /* void *memset(void *, int, size_t) */
>  ENTRY(__memset)
> @@ -15,6 +21,58 @@ WEAK(memset)
>  	sltiu	a3, a2, 16
>  	bnez	a3, .Lfinish
>
> +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> +	ALT_ZICBOZ("j .Ldo_memset", "nop")

This at least deserves a comment: the jump is PC-relative, so it'll only 
work if alternative processing happens in a way that ensures these PC 
offsets don't change.  I think this might actually work if all that 
section stuff avoids touching the PC, but that'd need to be written 
down if we're going to depend on it.

That said, this is really just a static_branch implemented differently.  
Can we just use one?

> +	/*
> +	 * t1 will be the Zicboz block size.
> +	 * Zero means we're not using Zicboz, and we don't when a1 != 0
> +	 */
> +	li	t1, 0
> +	bnez	a1, .Ldo_memset
> +	la	a3, riscv_cboz_block_size
> +	lw	t1, 0(a3)
> +
> +	/*
> +	 * Round to nearest Zicboz block-aligned address
> +	 * greater than or equal to the start address.
> +	 */
> +	addi	a3, t1, -1
> +	not	t2, a3			/* t2 is Zicboz block size mask */
> +	add	a3, t0, a3
> +	and	t3, a3, t2		/* t3 is Zicboz block aligned start */
> +
> +	/* Did we go too far or not have at least one block? */
> +	add	a3, a0, a2
> +	and	a3, a3, t2
> +	bgtu	a3, t3, .Ldo_zero
> +	li	t1, 0
> +	j	.Ldo_memset
> +
> +.Ldo_zero:
> +	/* Use Duff for initial bytes if there are any */
> +	bne	t3, t0, .Ldo_memset
> +
> +.Ldo_zero2:
> +	/* Calculate end address */
> +	and	a3, a2, t2
> +	add	a3, t0, a3
> +	sub	a4, a3, t0
> +
> +.Lzero_loop:
> +	CBO_ZERO(t0)
> +	add	t0, t0, t1
> +	bltu	t0, a3, .Lzero_loop
> +	li	t1, 0			/* We're done with Zicboz */
> +
> +	sub	a2, a2, a4		/* Update count */
> +	sltiu	a3, a2, 16
> +	bnez	a3, .Lfinish
> +
> +	/* t0 is Zicboz block size aligned, so it must be SZREG aligned */
> +	j	.Ldo_duff3
> +#endif
> +
> +.Ldo_memset:
>  	/*
>  	 * Round to nearest XLEN-aligned address
>  	 * greater than or equal to the start address.
> @@ -33,6 +91,18 @@ WEAK(memset)
>
>  .Ldo_duff:
>  	/* Duff's device with 32 XLEN stores per iteration */
> +
> +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> +	ALT_ZICBOZ("j .Ldo_duff2", "nop")
> +	beqz	t1, .Ldo_duff2
> +	/* a3, "end", is start of block aligned start. a1 is 0 */
> +	move    a3, t3
> +	sub	a4, a3, t0		/* a4 is SZREG aligned count */
> +	move	t4, a4			/* Save count for later, see below. */
> +	j	.Ldo_duff4
> +#endif
> +
> +.Ldo_duff2:
>  	/* Broadcast value into all bytes */
>  	andi	a1, a1, 0xff
>  	slli	a3, a1, 8
> @@ -44,10 +114,12 @@ WEAK(memset)
>  	or	a1, a3, a1
>  #endif
>
> +.Ldo_duff3:
>  	/* Calculate end address */
>  	andi	a4, a2, ~(SZREG-1)
>  	add	a3, t0, a4
>
> +.Ldo_duff4:
>  	andi	a4, a4, 31*SZREG	/* Calculate remainder */
>  	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
>  	neg	a4, a4
> @@ -100,6 +172,15 @@ WEAK(memset)
>
>  	addi	t0, t0, 32*SZREG
>  	bltu	t0, a3, .Lduff_loop
> +
> +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> +	ALT_ZICBOZ("j .Lcount_update", "nop")
> +	beqz	t1, .Lcount_update
> +	sub	a2, a2, t4		/* Difference was saved above */
> +	j	.Ldo_zero2
> +#endif
> +
> +.Lcount_update:
>  	andi	a2, a2, SZREG-1		/* Update count */
>
>  .Lfinish:

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

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

* Re: [PATCH 9/9] RISC-V: Use Zicboz in memset when available
  2022-11-03  2:43   ` Palmer Dabbelt
@ 2022-11-03 10:21     ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-11-03 10:21 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, aou, apatel, heiko,
	Conor Dooley, Atish Patra, jszhang

On Wed, Nov 02, 2022 at 07:43:03PM -0700, Palmer Dabbelt wrote:
> On Thu, 27 Oct 2022 06:02:47 PDT (-0700), ajones@ventanamicro.com wrote:
> > RISC-V has an optimized memset() which does byte by byte writes up to
> > the first sizeof(long) aligned address, then uses Duff's device until
> > the last sizeof(long) aligned address, and finally byte by byte to
> > the end. When memset is used to zero memory and the Zicboz extension
> > is available, then we can extend that by doing the optimized memset
> > up to the first Zicboz block size aligned address, then use the
> > Zicboz zero instruction for each block to the last block size aligned
> > address, and finally the optimized memset to the end.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> > 
> > diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> > index 74e4c7feec00..786b85b5e9cc 100644
> > --- a/arch/riscv/lib/memset.S
> > +++ b/arch/riscv/lib/memset.S
> > @@ -5,6 +5,12 @@
> > 
> >  #include <linux/linkage.h>
> >  #include <asm/asm.h>
> > +#include <asm/alternative-macros.h>
> > +#include <asm/insn-def.h>
> > +#include <asm/hwcap.h>
> > +
> > +#define ALT_ZICBOZ(old, new)	ALTERNATIVE(old, new, 0, RISCV_ISA_EXT_ZICBOZ, \
> > +					    CONFIG_RISCV_ISA_ZICBOZ)
> > 
> >  /* void *memset(void *, int, size_t) */
> >  ENTRY(__memset)
> > @@ -15,6 +21,58 @@ WEAK(memset)
> >  	sltiu	a3, a2, 16
> >  	bnez	a3, .Lfinish
> > 
> > +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> > +	ALT_ZICBOZ("j .Ldo_memset", "nop")
> 
> This at least deserves a comment: the jump is PC-relative, so it'll only
> work if alternative processing happens in a way that ensures these PC
> offsets don't change.  I think this might actually work if all that section
> stuff avoids touching the PC, but that'd need to be written down if we're
> going to depend on it.

I believe the "old" instructions can be anything, so PC-relative jumps
should always work. The "new" instructions cannot contain any branch
targets outside its content though. I agree we should better document
the constraints in arch/riscv/include/asm/alternative-macros.h as
my beliefs come from some trial-and-error and also from reading the
constraints in arm64's implementation, as it appears riscv's
implementation was derived from there. I can try to do an ALTERNATIVE
documenting patch independently of this series.

> 
> That said, this is really just a static_branch implemented differently.  Can
> we just use one?

I don't think we can use static branches in assembly.

Thanks,
drew

> 
> > +	/*
> > +	 * t1 will be the Zicboz block size.
> > +	 * Zero means we're not using Zicboz, and we don't when a1 != 0
> > +	 */
> > +	li	t1, 0
> > +	bnez	a1, .Ldo_memset
> > +	la	a3, riscv_cboz_block_size
> > +	lw	t1, 0(a3)
> > +
> > +	/*
> > +	 * Round to nearest Zicboz block-aligned address
> > +	 * greater than or equal to the start address.
> > +	 */
> > +	addi	a3, t1, -1
> > +	not	t2, a3			/* t2 is Zicboz block size mask */
> > +	add	a3, t0, a3
> > +	and	t3, a3, t2		/* t3 is Zicboz block aligned start */
> > +
> > +	/* Did we go too far or not have at least one block? */
> > +	add	a3, a0, a2
> > +	and	a3, a3, t2
> > +	bgtu	a3, t3, .Ldo_zero
> > +	li	t1, 0
> > +	j	.Ldo_memset
> > +
> > +.Ldo_zero:
> > +	/* Use Duff for initial bytes if there are any */
> > +	bne	t3, t0, .Ldo_memset
> > +
> > +.Ldo_zero2:
> > +	/* Calculate end address */
> > +	and	a3, a2, t2
> > +	add	a3, t0, a3
> > +	sub	a4, a3, t0
> > +
> > +.Lzero_loop:
> > +	CBO_ZERO(t0)
> > +	add	t0, t0, t1
> > +	bltu	t0, a3, .Lzero_loop
> > +	li	t1, 0			/* We're done with Zicboz */
> > +
> > +	sub	a2, a2, a4		/* Update count */
> > +	sltiu	a3, a2, 16
> > +	bnez	a3, .Lfinish
> > +
> > +	/* t0 is Zicboz block size aligned, so it must be SZREG aligned */
> > +	j	.Ldo_duff3
> > +#endif
> > +
> > +.Ldo_memset:
> >  	/*
> >  	 * Round to nearest XLEN-aligned address
> >  	 * greater than or equal to the start address.
> > @@ -33,6 +91,18 @@ WEAK(memset)
> > 
> >  .Ldo_duff:
> >  	/* Duff's device with 32 XLEN stores per iteration */
> > +
> > +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> > +	ALT_ZICBOZ("j .Ldo_duff2", "nop")
> > +	beqz	t1, .Ldo_duff2
> > +	/* a3, "end", is start of block aligned start. a1 is 0 */
> > +	move    a3, t3
> > +	sub	a4, a3, t0		/* a4 is SZREG aligned count */
> > +	move	t4, a4			/* Save count for later, see below. */
> > +	j	.Ldo_duff4
> > +#endif
> > +
> > +.Ldo_duff2:
> >  	/* Broadcast value into all bytes */
> >  	andi	a1, a1, 0xff
> >  	slli	a3, a1, 8
> > @@ -44,10 +114,12 @@ WEAK(memset)
> >  	or	a1, a3, a1
> >  #endif
> > 
> > +.Ldo_duff3:
> >  	/* Calculate end address */
> >  	andi	a4, a2, ~(SZREG-1)
> >  	add	a3, t0, a4
> > 
> > +.Ldo_duff4:
> >  	andi	a4, a4, 31*SZREG	/* Calculate remainder */
> >  	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
> >  	neg	a4, a4
> > @@ -100,6 +172,15 @@ WEAK(memset)
> > 
> >  	addi	t0, t0, 32*SZREG
> >  	bltu	t0, a3, .Lduff_loop
> > +
> > +#ifdef CONFIG_RISCV_ISA_ZICBOZ
> > +	ALT_ZICBOZ("j .Lcount_update", "nop")
> > +	beqz	t1, .Lcount_update
> > +	sub	a2, a2, t4		/* Difference was saved above */
> > +	j	.Ldo_zero2
> > +#endif
> > +
> > +.Lcount_update:
> >  	andi	a2, a2, SZREG-1		/* Update count */
> > 
> >  .Lfinish:

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

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

* Re: [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing
  2022-10-30 20:47   ` Conor Dooley
  2022-10-31  8:12     ` Andrew Jones
@ 2022-11-13 22:24     ` Conor Dooley
  2022-11-14  8:29       ` Andrew Jones
  1 sibling, 1 reply; 40+ messages in thread
From: Conor Dooley @ 2022-11-13 22:24 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Oct 30, 2022 at 08:47:46PM +0000, Conor Dooley wrote:
> On Thu, Oct 27, 2022 at 03:02:40PM +0200, Andrew Jones wrote:

> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fa427bdcf773..bf969218f609 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -144,6 +144,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> >  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >  	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> >  	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> > +	__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),

Forgot to mention it at the time, but given Palmer submitted a resort of
this in canonical order recently - seems like zicboz should sort before
zihintpause?

> >  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),


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

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

* Re: [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing
  2022-11-13 22:24     ` Conor Dooley
@ 2022-11-14  8:29       ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-11-14  8:29 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Sun, Nov 13, 2022 at 10:24:30PM +0000, Conor Dooley wrote:
> On Sun, Oct 30, 2022 at 08:47:46PM +0000, Conor Dooley wrote:
> > On Thu, Oct 27, 2022 at 03:02:40PM +0200, Andrew Jones wrote:
> 
> > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > index fa427bdcf773..bf969218f609 100644
> > > --- a/arch/riscv/kernel/cpu.c
> > > +++ b/arch/riscv/kernel/cpu.c
> > > @@ -144,6 +144,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> > >  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> > >  	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> > >  	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> > > +	__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> 
> Forgot to mention it at the time, but given Palmer submitted a resort of
> this in canonical order recently - seems like zicboz should sort before
> zihintpause?

Will do for v2. Sorry for the delay on v2. I need to finish some other
stuff before I can get back to this.

Thanks,
drew

> 
> > >  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> 

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

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

* Re: [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size
  2022-10-27 13:02 ` [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size Andrew Jones
  2022-10-30 21:23   ` Conor Dooley
@ 2022-11-27  5:37   ` Anup Patel
  1 sibling, 0 replies; 40+ messages in thread
From: Anup Patel @ 2022-11-27  5:37 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 6:34 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> We're about to allow guests to use the Zicboz extension. KVM
> userspace needs to know the cache block size in order to
> properly advertise it to the guest. Provide a virtual config
> register for userspace to get it with the GET_ONE_REG API, but
> setting it cannot be supported, so disallow SET_ONE_REG.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/include/uapi/asm/kvm.h | 1 +
>  arch/riscv/kvm/vcpu.c             | 7 +++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 8985ff234c01..4bbf55cb2b70 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -49,6 +49,7 @@ struct kvm_sregs {
>  struct kvm_riscv_config {
>         unsigned long isa;
>         unsigned long zicbom_block_size;
> +       unsigned long zicboz_block_size;
>  };
>
>  /* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 71ebbc4821f0..18a739070b51 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -270,6 +270,11 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
>                         return -EINVAL;
>                 reg_val = riscv_cbom_block_size;
>                 break;
> +       case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
> +               if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ))
> +                       return -EINVAL;
> +               reg_val = riscv_cboz_block_size;
> +               break;
>         default:
>                 return -EINVAL;
>         }
> @@ -329,6 +334,8 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
>                 break;
>         case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
>                 return -EOPNOTSUPP;
> +       case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
> +               return -EOPNOTSUPP;
>         default:
>                 return -EINVAL;
>         }
> --
> 2.37.3
>
>
> --
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

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

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

* Re: [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest
  2022-10-27 13:02 ` [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest Andrew Jones
  2022-10-30 21:23   ` Conor Dooley
@ 2022-11-27  5:38   ` Anup Patel
  1 sibling, 0 replies; 40+ messages in thread
From: Anup Patel @ 2022-11-27  5:38 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Conor Dooley, Atish Patra,
	Jisheng Zhang

On Thu, Oct 27, 2022 at 6:34 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Guests may use the cbo.zero instruction when the CPU has the Zicboz
> extension and the hypervisor sets henvcfg.CBZE.
>
> Add Zicboz support for KVM guests which may be enabled and
> disabled from KVM userspace using the ISA extension ONE_REG API.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/include/uapi/asm/kvm.h | 1 +
>  arch/riscv/kvm/vcpu.c             | 4 ++++
>  2 files changed, 5 insertions(+)
>
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 4bbf55cb2b70..8dc21ceee7aa 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -103,6 +103,7 @@ enum KVM_RISCV_ISA_EXT_ID {
>         KVM_RISCV_ISA_EXT_SVINVAL,
>         KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
>         KVM_RISCV_ISA_EXT_ZICBOM,
> +       KVM_RISCV_ISA_EXT_ZICBOZ,
>         KVM_RISCV_ISA_EXT_MAX,
>  };
>
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 18a739070b51..7758faec590a 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -62,6 +62,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
>         KVM_ISA_EXT_ARR(SVPBMT),
>         KVM_ISA_EXT_ARR(ZIHINTPAUSE),
>         KVM_ISA_EXT_ARR(ZICBOM),
> +       KVM_ISA_EXT_ARR(ZICBOZ),
>  };
>
>  static unsigned long kvm_riscv_vcpu_base2isa_ext(unsigned long base_ext)
> @@ -821,6 +822,9 @@ static void kvm_riscv_vcpu_update_config(const unsigned long *isa)
>         if (riscv_isa_extension_available(isa, ZICBOM))
>                 henvcfg |= (ENVCFG_CBIE | ENVCFG_CBCFE);
>
> +       if (riscv_isa_extension_available(isa, ZICBOZ))
> +               henvcfg |= ENVCFG_CBZE;
> +
>         csr_write(CSR_HENVCFG, henvcfg);
>  #ifdef CONFIG_32BIT
>         csr_write(CSR_HENVCFGH, henvcfg >> 32);
> --
> 2.37.3
>
>
> --
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

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

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

* Re: [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
  2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
                   ` (10 preceding siblings ...)
  2022-11-01 10:37 ` Andrew Jones
@ 2022-12-20 12:55 ` Conor Dooley
  2022-12-26 18:56   ` Andrew Jones
  11 siblings, 1 reply; 40+ messages in thread
From: Conor Dooley @ 2022-12-20 12:55 UTC (permalink / raw)
  To: Andrew Jones
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Atish Patra, Jisheng Zhang


[-- Attachment #1.1: Type: text/plain, Size: 5766 bytes --]

Hey Drew,

I assume you're not gonna respin this one before the xmas holidays etc,
but a v2 is on the cards, right?

Thanks,
Conor.

On Thu, Oct 27, 2022 at 03:02:38PM +0200, Andrew Jones wrote:
> When the Zicboz extension is available we can more rapidly zero naturally
> aligned Zicboz block sized chunks of memory. As pages are always page
> aligned and are larger than any Zicboz block size will be, then
> clear_page() appears to be a good candidate for the extension. While cycle
> count and energy consumption should also be considered, we can be pretty
> certain that implementing clear_page() with the Zicboz extension is a win
> by comparing the new dynamic instruction count with its current count[1].
> Doing so we see that the new count is less than half the old count (see
> patch4's commit message for more details). Another candidate for the
> extension is memset(), but, since memset() isn't just used for zeroing
> memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
> it's not as obvious if adding support for Zicboz will be an overall win.
> In order to make a determination, I've done some analysis and wrote my
> conclusions in the bullets below.
> 
> * When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
>   doesn't change, so that's fine.
> 
> * The overhead added to memset() when the Zicboz extension isn't present,
>   but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
>   which I believe is fine.
> 
> * The overhead added to a memset() invocation which is not zeroing memory
>   is 7 instructions, where 3 are branches. This seems fine and,
>   furthermore, memset() is almost always invoked to zero memory (99% [2]).
> 
> * When memset() is invoked to zero memory, the proposed Zicboz extended
>   memset() always has a lower dynamic instruction count than the current
>   memset() as long as the input address is Zicboz block aligned and the
>   length is >= the block size.
> 
> * When memset() is invoked to zero memory, the proposed Zicboz extended
>   memset() is always worse for unaligned or too small inputs than the
>   current memset(), but it's only at most a few dozen instructions worse.
>   I think this is probably fine, especially considering the large majority
>   of zeroing invocations are 64 bytes or larger and are aligned to a
>   power-of-2 boundary, 64-byte or larger (77% [2]).
> 
> [1] I ported the functions under test to userspace and linked them with
>     a test program. Then, I ran them under gdb with a script[3] which
>     counted instructions by single stepping.
> 
> [2] I wrote bpftrace scripts[4] to count memset() invocations to see the
>     frequency of it being used to zero memory and have block size aligned
>     input addresses with block size or larger lengths. The workload was
>     just random desktop stuff including streaming video and compiling.
>     While I did run this on my x86 notebook, I still expect the data to
>     be representative on RISC-V. Note, x86 has clear_page() so the
>     memset() data regarding alignment and size weren't over inflated by
>     page zeroing invocations. Grepping also shows the large majority of
>     memset() calls are to zero memory (93%).
> 
> [3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
> [4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe
> 
> These patches are based on the following pending series
> 
> 1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
>    20221024091309.406906-1-ajones@ventanamicro.com
> 
> 2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
>    20221006070818.3616-1-jszhang@kernel.org
>    Also including the additional patch proposed here
>    20221013162038.ehseju2neic2xu5z@kamzik
> 
> The patches are also available here
> https://github.com/jones-drew/linux/commits/riscv/zicboz
> 
> To test over QEMU this branch may be used to enable Zicboz
> https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz
> 
> To test running a KVM guest with Zicboz this kvmtool branch may be used
> https://github.com/jones-drew/kvmtool/commits/riscv/zicboz
> 
> Thanks,
> drew
> 
> Andrew Jones (9):
>   RISC-V: Factor out body of riscv_init_cbom_blocksize loop
>   RISC-V: Add Zicboz detection and block size parsing
>   RISC-V: insn-def: Define cbo.zero
>   RISC-V: Use Zicboz in clear_page when available
>   RISC-V: KVM: Provide UAPI for Zicboz block size
>   RISC-V: KVM: Expose Zicboz to the guest
>   RISC-V: lib: Improve memset assembler formatting
>   RISC-V: lib: Use named labels in memset
>   RISC-V: Use Zicboz in memset when available
> 
>  arch/riscv/Kconfig                  |  13 ++
>  arch/riscv/include/asm/cacheflush.h |   3 +-
>  arch/riscv/include/asm/hwcap.h      |   1 +
>  arch/riscv/include/asm/insn-def.h   |  50 ++++++
>  arch/riscv/include/asm/page.h       |   6 +-
>  arch/riscv/include/uapi/asm/kvm.h   |   2 +
>  arch/riscv/kernel/cpu.c             |   1 +
>  arch/riscv/kernel/cpufeature.c      |  10 ++
>  arch/riscv/kernel/setup.c           |   2 +-
>  arch/riscv/kvm/vcpu.c               |  11 ++
>  arch/riscv/lib/Makefile             |   1 +
>  arch/riscv/lib/clear_page.S         |  28 ++++
>  arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
>  arch/riscv/mm/cacheflush.c          |  64 +++++---
>  14 files changed, 325 insertions(+), 108 deletions(-)
>  create mode 100644 arch/riscv/lib/clear_page.S
> 
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

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

* Re: [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset
  2022-12-20 12:55 ` Conor Dooley
@ 2022-12-26 18:56   ` Andrew Jones
  0 siblings, 0 replies; 40+ messages in thread
From: Andrew Jones @ 2022-12-26 18:56 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, kvm-riscv, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Anup Patel, Heiko Stuebner, Atish Patra, Jisheng Zhang

On Tue, Dec 20, 2022 at 12:55:07PM +0000, Conor Dooley wrote:
> Hey Drew,
> 
> I assume you're not gonna respin this one before the xmas holidays etc,
> but a v2 is on the cards, right?

Hi Conor,

A v2 is definitely on my TODO. I'll try to get one out soon.

Thanks,
drew

> 
> Thanks,
> Conor.
> 
> On Thu, Oct 27, 2022 at 03:02:38PM +0200, Andrew Jones wrote:
> > When the Zicboz extension is available we can more rapidly zero naturally
> > aligned Zicboz block sized chunks of memory. As pages are always page
> > aligned and are larger than any Zicboz block size will be, then
> > clear_page() appears to be a good candidate for the extension. While cycle
> > count and energy consumption should also be considered, we can be pretty
> > certain that implementing clear_page() with the Zicboz extension is a win
> > by comparing the new dynamic instruction count with its current count[1].
> > Doing so we see that the new count is less than half the old count (see
> > patch4's commit message for more details). Another candidate for the
> > extension is memset(), but, since memset() isn't just used for zeroing
> > memory and it accepts arbitrarily aligned addresses and arbitrary sizes,
> > it's not as obvious if adding support for Zicboz will be an overall win.
> > In order to make a determination, I've done some analysis and wrote my
> > conclusions in the bullets below.
> > 
> > * When compiling the kernel without CONFIG_RISCV_ISA_ZICBOZ, memset()
> >   doesn't change, so that's fine.
> > 
> > * The overhead added to memset() when the Zicboz extension isn't present,
> >   but CONFIG_RISCV_ISA_ZICBOZ is selected, is 3 jumps to known targets,
> >   which I believe is fine.
> > 
> > * The overhead added to a memset() invocation which is not zeroing memory
> >   is 7 instructions, where 3 are branches. This seems fine and,
> >   furthermore, memset() is almost always invoked to zero memory (99% [2]).
> > 
> > * When memset() is invoked to zero memory, the proposed Zicboz extended
> >   memset() always has a lower dynamic instruction count than the current
> >   memset() as long as the input address is Zicboz block aligned and the
> >   length is >= the block size.
> > 
> > * When memset() is invoked to zero memory, the proposed Zicboz extended
> >   memset() is always worse for unaligned or too small inputs than the
> >   current memset(), but it's only at most a few dozen instructions worse.
> >   I think this is probably fine, especially considering the large majority
> >   of zeroing invocations are 64 bytes or larger and are aligned to a
> >   power-of-2 boundary, 64-byte or larger (77% [2]).
> > 
> > [1] I ported the functions under test to userspace and linked them with
> >     a test program. Then, I ran them under gdb with a script[3] which
> >     counted instructions by single stepping.
> > 
> > [2] I wrote bpftrace scripts[4] to count memset() invocations to see the
> >     frequency of it being used to zero memory and have block size aligned
> >     input addresses with block size or larger lengths. The workload was
> >     just random desktop stuff including streaming video and compiling.
> >     While I did run this on my x86 notebook, I still expect the data to
> >     be representative on RISC-V. Note, x86 has clear_page() so the
> >     memset() data regarding alignment and size weren't over inflated by
> >     page zeroing invocations. Grepping also shows the large majority of
> >     memset() calls are to zero memory (93%).
> > 
> > [3] https://gist.github.com/jones-drew/487791c956ceca8c18adc2847eec9c60
> > [4] https://gist.github.com/jones-drew/1e860692cf6fc0fb2a82a04c9ce720fe
> > 
> > These patches are based on the following pending series
> > 
> > 1. "[PATCH v2 0/3] RISC-V: Ensure Zicbom has a valid block size"
> >    20221024091309.406906-1-ajones@ventanamicro.com
> > 
> > 2. "[PATCH 0/8] riscv: improve boot time isa extensions handling"
> >    20221006070818.3616-1-jszhang@kernel.org
> >    Also including the additional patch proposed here
> >    20221013162038.ehseju2neic2xu5z@kamzik
> > 
> > The patches are also available here
> > https://github.com/jones-drew/linux/commits/riscv/zicboz
> > 
> > To test over QEMU this branch may be used to enable Zicboz
> > https://gitlab.com/jones-drew/qemu/-/commits/riscv/zicboz
> > 
> > To test running a KVM guest with Zicboz this kvmtool branch may be used
> > https://github.com/jones-drew/kvmtool/commits/riscv/zicboz
> > 
> > Thanks,
> > drew
> > 
> > Andrew Jones (9):
> >   RISC-V: Factor out body of riscv_init_cbom_blocksize loop
> >   RISC-V: Add Zicboz detection and block size parsing
> >   RISC-V: insn-def: Define cbo.zero
> >   RISC-V: Use Zicboz in clear_page when available
> >   RISC-V: KVM: Provide UAPI for Zicboz block size
> >   RISC-V: KVM: Expose Zicboz to the guest
> >   RISC-V: lib: Improve memset assembler formatting
> >   RISC-V: lib: Use named labels in memset
> >   RISC-V: Use Zicboz in memset when available
> > 
> >  arch/riscv/Kconfig                  |  13 ++
> >  arch/riscv/include/asm/cacheflush.h |   3 +-
> >  arch/riscv/include/asm/hwcap.h      |   1 +
> >  arch/riscv/include/asm/insn-def.h   |  50 ++++++
> >  arch/riscv/include/asm/page.h       |   6 +-
> >  arch/riscv/include/uapi/asm/kvm.h   |   2 +
> >  arch/riscv/kernel/cpu.c             |   1 +
> >  arch/riscv/kernel/cpufeature.c      |  10 ++
> >  arch/riscv/kernel/setup.c           |   2 +-
> >  arch/riscv/kvm/vcpu.c               |  11 ++
> >  arch/riscv/lib/Makefile             |   1 +
> >  arch/riscv/lib/clear_page.S         |  28 ++++
> >  arch/riscv/lib/memset.S             | 241 +++++++++++++++++++---------
> >  arch/riscv/mm/cacheflush.c          |  64 +++++---
> >  14 files changed, 325 insertions(+), 108 deletions(-)
> >  create mode 100644 arch/riscv/lib/clear_page.S
> > 
> > -- 
> > 2.37.3
> > 
> > 
> > _______________________________________________
> > 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] 40+ messages in thread

end of thread, other threads:[~2022-12-26 18:57 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-27 13:02 [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
2022-10-27 13:02 ` [PATCH 1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop Andrew Jones
2022-10-27 14:58   ` Heiko Stübner
2022-10-30 20:31   ` Conor Dooley
2022-10-31  8:11     ` Andrew Jones
2022-10-27 13:02 ` [PATCH 2/9] RISC-V: Add Zicboz detection and block size parsing Andrew Jones
2022-10-27 15:03   ` Heiko Stübner
2022-10-27 15:42     ` Andrew Jones
2022-10-30 20:47   ` Conor Dooley
2022-10-31  8:12     ` Andrew Jones
2022-11-13 22:24     ` Conor Dooley
2022-11-14  8:29       ` Andrew Jones
2022-10-27 13:02 ` [PATCH 3/9] RISC-V: insn-def: Define cbo.zero Andrew Jones
2022-10-27 15:37   ` Heiko Stübner
2022-10-30 21:08   ` Conor Dooley
2022-10-31  8:18     ` Andrew Jones
2022-10-27 13:02 ` [PATCH 4/9] RISC-V: Use Zicboz in clear_page when available Andrew Jones
2022-10-27 13:02 ` [PATCH 5/9] RISC-V: KVM: Provide UAPI for Zicboz block size Andrew Jones
2022-10-30 21:23   ` Conor Dooley
2022-11-27  5:37   ` Anup Patel
2022-10-27 13:02 ` [PATCH 6/9] RISC-V: KVM: Expose Zicboz to the guest Andrew Jones
2022-10-30 21:23   ` Conor Dooley
2022-11-27  5:38   ` Anup Patel
2022-10-27 13:02 ` [PATCH 7/9] RISC-V: lib: Improve memset assembler formatting Andrew Jones
2022-10-30 21:27   ` Conor Dooley
2022-10-27 13:02 ` [PATCH 8/9] RISC-V: lib: Use named labels in memset Andrew Jones
2022-10-30 22:15   ` Conor Dooley
2022-10-31  8:24     ` Andrew Jones
2022-10-27 13:02 ` [PATCH 9/9] RISC-V: Use Zicboz in memset when available Andrew Jones
2022-10-30 22:35   ` Conor Dooley
2022-10-31  8:30     ` Andrew Jones
2022-11-03  2:43   ` Palmer Dabbelt
2022-11-03 10:21     ` Andrew Jones
2022-10-29  9:59 ` [PATCH 0/9] RISC-V: Apply Zicboz to clear_page and memset Andrew Jones
2022-10-30 20:23   ` Conor Dooley
2022-10-31  8:39     ` Andrew Jones
2022-11-01 10:37 ` Andrew Jones
2022-11-01 10:53   ` Andrew Jones
2022-12-20 12:55 ` Conor Dooley
2022-12-26 18:56   ` 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).