RISC-V: Apply Zicboz to clear_page and memset

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 <[email protected]>

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 <[email protected]>

---
 arch/riscv/mm/cacheflush.c | 45 +++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 20 deletions(-)

arch/riscv/mm/cacheflush.c +0 -0

index 57b40a350420..f096b9966cae 100644

arch/riscv/mm/cacheflush.c +23 -20

+			       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)

Signature · 7 lines

2.37.3


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

Quoted 16 lines · 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 <[email protected]>

---
 arch/riscv/mm/cacheflush.c | 45 +++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 20 deletions(-)

arch/riscv/mm/cacheflush.c +0 -0

index 57b40a350420..f096b9966cae 100644
@@ -91,34 +91,39 @@ void flush_icache_pte(pte_t pte)
 unsigned int riscv_cbom_block_size;
 EXPORT_SYMBOL_GPL(riscv_cbom_block_size);
 

arch/riscv/mm/cacheflush.c void flush_icache_pte(pte_t pte) · line 95

Quoted 2 lines

arch/riscv/mm/cacheflush.c +2 -0

+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 <[email protected]>

Quoted 63 lines

arch/riscv/mm/cacheflush.c +23 -20

+			       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)

Signature · 7 lines

2.37.3


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

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 <[email protected]>

---
 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(-)

arch/riscv/Kconfig +13 -0

index 6b48a3ae9843..c20e6fa0c0b1 100644
@@ -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

arch/riscv/include/asm/cacheflush.h +2 -1

index f6fbe7042f1c..5b31568cf5e6 100644
@@ -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);

arch/riscv/include/asm/hwcap.h +1 -0

index 5d6492bde446..eaa5a972ad2d 100644
@@ -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
 

arch/riscv/kernel/cpu.c +1 -0

index fa427bdcf773..bf969218f609 100644
@@ -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),
 };
 

arch/riscv/kernel/cpufeature.c +10 -0

index 18b9ed4df1f4..e13b3391de76 100644
@@ -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
 		}

arch/riscv/kernel/setup.c +1 -1

index a07917551027..26de0d8fd23d 100644
@@ -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

arch/riscv/mm/cacheflush.c +6 -2

index f096b9966cae..208e0d58bde3 100644
@@ -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;

arch/riscv/mm/cacheflush.c +6 -2

+				   &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;
 }

Am Donnerstag, 27. Oktober 2022, 15:02:40 CEST schrieb Andrew Jones:

arch/riscv/mm/cacheflush.c static void cbo_get_block_size(struct device_node *node, · line 129

Quoted 1 line

arch/riscv/mm/cacheflush.c +1 -0

+		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 <[email protected]>

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 <[email protected]>

---
 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(-)

arch/riscv/Kconfig +13 -0

index 6b48a3ae9843..c20e6fa0c0b1 100644
@@ -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

arch/riscv/include/asm/cacheflush.h +2 -1

index f6fbe7042f1c..5b31568cf5e6 100644
@@ -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);

arch/riscv/include/asm/hwcap.h +1 -0

index 5d6492bde446..eaa5a972ad2d 100644
@@ -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
 

arch/riscv/kernel/cpu.c +1 -0

index fa427bdcf773..bf969218f609 100644
@@ -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),
 };
 

arch/riscv/kernel/cpufeature.c +10 -0

index 18b9ed4df1f4..e13b3391de76 100644
@@ -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
 		}

arch/riscv/kernel/setup.c +1 -1

index a07917551027..26de0d8fd23d 100644
@@ -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

arch/riscv/mm/cacheflush.c +7 -5

index f096b9966cae..208e0d58bde3 100644
@@ -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",

arch/riscv/mm/cacheflush.c +3 -0

+
+	if (cboz_blksz)
+		riscv_cboz_block_size = cboz_blksz;
 }

Signature · 7 lines

2.37.3


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

Quoted 3 lines · 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.

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.

Quoted 154 lines

Signed-off-by Andrew Jones <[email protected]>

---
 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(-)

arch/riscv/Kconfig +13 -0

index 6b48a3ae9843..c20e6fa0c0b1 100644
@@ -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

arch/riscv/include/asm/cacheflush.h +2 -1

index f6fbe7042f1c..5b31568cf5e6 100644
@@ -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);

arch/riscv/include/asm/hwcap.h +1 -0

index 5d6492bde446..eaa5a972ad2d 100644
@@ -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
 

arch/riscv/kernel/cpu.c +1 -0

index fa427bdcf773..bf969218f609 100644
@@ -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),
 };
 

arch/riscv/kernel/cpufeature.c +10 -0

index 18b9ed4df1f4..e13b3391de76 100644
@@ -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
 		}

arch/riscv/kernel/setup.c +1 -1

index a07917551027..26de0d8fd23d 100644
@@ -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

arch/riscv/mm/cacheflush.c +10 -6

index f096b9966cae..208e0d58bde3 100644
@@ -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);
 	}
 

arch/riscv/mm/cacheflush.c static void cbo_get_block_size(struct device_node *node, · line 134

Quoted 4 lines

arch/riscv/mm/cacheflush.c +2 -2

-	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 <[email protected]>

Quoted 13 lines

arch/riscv/mm/cacheflush.c +3 -0

+
+	if (cboz_blksz)
+		riscv_cboz_block_size = cboz_blksz;
 }

Signature · 7 lines

2.37.3


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

arch/riscv/kernel/cpu.c static struct riscv_isa_ext_data isa_ext_arr[] = { · line 147

Quoted 9 lines · Andrew Jones

arch/riscv/kernel/cpu.c +1 -0

index fa427bdcf773..bf969218f609 100644
@@ -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),

Quoted 1 line

arch/riscv/kernel/cpu.c +0 -0

 	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),

Quoted 11 lines · Conor Dooley

arch/riscv/kernel/cpu.c static struct riscv_isa_ext_data isa_ext_arr[] = { · line 147

Quoted 9 lines · Andrew Jones

arch/riscv/kernel/cpu.c +1 -0

index fa427bdcf773..bf969218f609 100644
@@ -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?

Quoted 2 lines

Quoted 2 lines

arch/riscv/kernel/cpu.c +0 -0

 	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),

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 <[email protected]>

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 <[email protected]>

---
 arch/riscv/include/asm/insn-def.h | 50 +++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

arch/riscv/include/asm/insn-def.h +22 -0

index 16044affa57c..f13054716556 100644
@@ -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>

arch/riscv/include/asm/insn-def.h +21 -0

+"		 (\\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 */

Signature · 7 lines

2.37.3


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

arch/riscv/include/asm/insn-def.h line 91

Quoted 7 lines

arch/riscv/include/asm/insn-def.h +7 -0

+#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.

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 <[email protected]>

---
 arch/riscv/include/uapi/asm/kvm.h | 1 +
 arch/riscv/kvm/vcpu.c             | 7 +++++++
 2 files changed, 8 insertions(+)

arch/riscv/include/uapi/asm/kvm.h +1 -0

index 8985ff234c01..4bbf55cb2b70 100644
@@ -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 */

arch/riscv/kvm/vcpu.c +7 -0

index 71ebbc4821f0..18a739070b51 100644
@@ -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;
 	}

Signature · 7 lines

2.37.3


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

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 <[email protected]>

---
 arch/riscv/include/uapi/asm/kvm.h | 1 +
 arch/riscv/kvm/vcpu.c             | 7 +++++++
 2 files changed, 8 insertions(+)

arch/riscv/include/uapi/asm/kvm.h +1 -0

index 8985ff234c01..4bbf55cb2b70 100644
@@ -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 */

arch/riscv/kvm/vcpu.c +7 -1

index 71ebbc4821f0..18a739070b51 100644
@@ -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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

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 <[email protected]>

---
 arch/riscv/include/uapi/asm/kvm.h | 1 +
 arch/riscv/kvm/vcpu.c             | 4 ++++
 2 files changed, 5 insertions(+)

arch/riscv/include/uapi/asm/kvm.h +1 -0

index 4bbf55cb2b70..8dc21ceee7aa 100644
@@ -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,
 };
 

arch/riscv/kvm/vcpu.c +4 -0

index 18a739070b51..7758faec590a 100644
@@ -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);

Signature · 7 lines

2.37.3


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

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 <[email protected]>

---
 arch/riscv/include/uapi/asm/kvm.h | 1 +
 arch/riscv/kvm/vcpu.c             | 4 ++++
 2 files changed, 5 insertions(+)

arch/riscv/include/uapi/asm/kvm.h +1 -0

index 4bbf55cb2b70..8dc21ceee7aa 100644
@@ -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,
 };
 

arch/riscv/kvm/vcpu.c +4 -1

index 18a739070b51..7758faec590a 100644
@@ -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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

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 <[email protected]>

---
 arch/riscv/lib/memset.S | 143 ++++++++++++++++++++--------------------
 1 file changed, 72 insertions(+), 71 deletions(-)

arch/riscv/lib/memset.S +72 -71

index 34c5360c6705..e613c5c27998 100644
@@ -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)

Signature · 7 lines

2.37.3


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

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 <[email protected]>

---
 arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

arch/riscv/lib/memset.S +15 -14

index e613c5c27998..74e4c7feec00 100644
@@ -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)

Signature · 7 lines

2.37.3


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

Quoted 1 line · Andrew Jones

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.

Quoted 5 lines

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 <[email protected]>

Quoted 100 lines

Signed-off-by Andrew Jones <[email protected]>

---
 arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

arch/riscv/lib/memset.S +15 -14

index e613c5c27998..74e4c7feec00 100644
@@ -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)

Signature · 7 lines

2.37.3


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

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 <[email protected]>

---
 arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

arch/riscv/lib/memset.S +6 -0

index 74e4c7feec00..786b85b5e9cc 100644
@@ -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)

arch/riscv/lib/memset.S +6 -0

+	 */
+	li	t1, 0
+	bnez	a1, .Ldo_memset
+	la	a3, riscv_cboz_block_size
+	lw	t1, 0(a3)
+

arch/riscv/lib/memset.S +54 -0

+	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:

Signature · 7 lines

2.37.3


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

Quoted 32 lines · Andrew Jones

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 <[email protected]>

---
 arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

arch/riscv/lib/memset.S +6 -0

index 74e4c7feec00..786b85b5e9cc 100644
@@ -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)

arch/riscv/lib/memset.S WEAK(memset) · line 28

Quoted 8 lines

arch/riscv/lib/memset.S +5 -0

@@ -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".

Quoted 7 lines

arch/riscv/lib/memset.S +6 -0

+	 */
+	li	t1, 0
+	bnez	a1, .Ldo_memset
+	la	a3, riscv_cboz_block_size
+	lw	t1, 0(a3)
+

arch/riscv/lib/memset.S WEAK(memset) · line 44

Quoted 10 lines

arch/riscv/lib/memset.S +10 -0

+	/*
+	 * 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".

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 <[email protected]>

---
 arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

arch/riscv/lib/memset.S +6 -0

index 74e4c7feec00..786b85b5e9cc 100644
@@ -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)

arch/riscv/lib/memset.S +73 -0

+	/*
+	 * 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:

Quoted 35 lines · ajones at ventanamicro.com

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 <[email protected]>

---
 arch/riscv/lib/memset.S | 81 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

arch/riscv/lib/memset.S +6 -0

index 74e4c7feec00..786b85b5e9cc 100644
@@ -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
 

arch/riscv/lib/memset.S WEAK(memset) · line 25

Quoted 2 lines

arch/riscv/lib/memset.S +2 -0

+#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?

Quoted 102 lines

arch/riscv/lib/memset.S +73 -0

+	/*
+	 * 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:

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 at ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling" 20221006070818.3616-1-jszhang at kernel.org
Also including the additional patch proposed here 20221013162038.ehseju2neic2xu5z at 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

Signature · 1 line

2.37.3

Quoted 88 lines · Andrew Jones

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 at ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling" 20221006070818.3616-1-jszhang at kernel.org
Also including the additional patch proposed here 20221013162038.ehseju2neic2xu5z at 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

Quoted 20 lines

 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

Signature · 1 line

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.

Quoted 110 lines · Andrew Jones

Quoted 88 lines · Andrew Jones

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 at ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling" 20221006070818.3616-1-jszhang at kernel.org
Also including the additional patch proposed here 20221013162038.ehseju2neic2xu5z at 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

Quoted 20 lines

 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

Signature · 1 line

2.37.3

Quoted 6 lines

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.

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 at ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling" 20221006070818.3616-1-jszhang at kernel.org
Also including the additional patch proposed here 20221013162038.ehseju2neic2xu5z at 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

Signature · 1 line

2.37.3

Quoted 31 lines · Andrew Jones

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.

Quoted 6 lines

* 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.

Quoted 72 lines

[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 at ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling" 20221006070818.3616-1-jszhang at kernel.org
Also including the additional patch proposed here 20221013162038.ehseju2neic2xu5z at 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

Signature · 1 line

2.37.3

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 at ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling" 20221006070818.3616-1-jszhang at kernel.org
Also including the additional patch proposed here 20221013162038.ehseju2neic2xu5z at 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

Signature · 7 lines

2.37.3


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

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.

Quoted 114 lines · Andrew Jones

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 at ventanamicro.com

2. "[PATCH 0/8] riscv: improve boot time isa extensions handling" 20221006070818.3616-1-jszhang at kernel.org
Also including the additional patch proposed here 20221013162038.ehseju2neic2xu5z at 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

Signature · 7 lines

2.37.3


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