[PATCH 1/3] RISC-V: Add Zicclsm to cpufeature and hwprobe

[PATCH 1/3] RISC-V: Add Zicclsm to cpufeature and hwprobe

[Jesse Taube]

> Zicclsm Misaligned loads and stores to main memory regions with both
> the cacheability and coherence PMAs must be supported.
> Note:
> This introduces a new extension name for this feature.
> This requires misaligned support for all regular load and store
> instructions (including scalar and vector) but not AMOs or other
> specialized forms of memory access. Even though mandated, misaligned
> loads and stores might execute extremely slowly. Standard software
> distributions should assume their existence only for correctness,
> not for performance.

Detecing zicclsm allows the kernel to report if the
hardware supports misaligned accesses even if support wasn't probed.

This is useful for usermode to know if vector misaligned accesses are
supported.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/include/asm/hwcap.h        | 1 +
 arch/riscv/include/uapi/asm/hwprobe.h | 1 +
 arch/riscv/kernel/cpufeature.c        | 1 +
 arch/riscv/kernel/sys_hwprobe.c       | 1 +
 4 files changed, 4 insertions(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..8c0d0b555a8e 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,7 @@
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
 #define RISCV_ISA_EXT_XANDESPMU		74
+#define RISCV_ISA_EXT_ZICCLSM		75
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 2902f68dc913..060212331a03 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -59,6 +59,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
 #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
 #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
+#define		RISCV_HWPROBE_EXT_ZICCLSM	(1ULL << 36)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
 #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 3ed2359eae35..863c708f2f2e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -305,6 +305,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
 	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
 	__RISCV_ISA_EXT_DATA(xandespmu, RISCV_ISA_EXT_XANDESPMU),
+	__RISCV_ISA_EXT_DATA(zicclsm, RISCV_ISA_EXT_ZICCLSM),
 };
 
 const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index 8cae41a502dd..b286b73e763e 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -125,6 +125,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 			EXT_KEY(ZVKT);
 			EXT_KEY(ZVFH);
 			EXT_KEY(ZVFHMIN);
+			EXT_KEY(ZICCLSM);
 		}
 
 		if (has_fpu()) {
-- 
2.43.0

[PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Jesse Taube]

Run a unaligned vector access to test if the system supports
vector unaligned access. Add the result to a new key in hwprobe.
This is useful for usermode to know if vector misaligned accesses are
supported and if they are faster or slower than equivalent byte accesses.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/include/asm/cpufeature.h        |  2 +
 arch/riscv/include/asm/hwprobe.h           |  2 +-
 arch/riscv/include/asm/vector.h            |  1 +
 arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
 arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
 arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
 arch/riscv/kernel/unaligned_access_speed.c |  4 ++
 arch/riscv/kernel/vector.c                 |  2 +-
 8 files changed, 129 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
index 347805446151..a012c8490a27 100644
--- a/arch/riscv/include/asm/cpufeature.h
+++ b/arch/riscv/include/asm/cpufeature.h
@@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
 
 #if defined(CONFIG_RISCV_MISALIGNED)
 bool check_unaligned_access_emulated_all_cpus(void);
+bool check_vector_unaligned_access_all_cpus(void);
 void unaligned_emulation_finish(void);
 bool unaligned_ctl_available(void);
 DECLARE_PER_CPU(long, misaligned_access_speed);
+DECLARE_PER_CPU(long, vector_misaligned_access);
 #else
 static inline bool unaligned_ctl_available(void)
 {
diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
index 630507dff5ea..150a9877b0af 100644
--- a/arch/riscv/include/asm/hwprobe.h
+++ b/arch/riscv/include/asm/hwprobe.h
@@ -8,7 +8,7 @@
 
 #include <uapi/asm/hwprobe.h>
 
-#define RISCV_HWPROBE_MAX_KEY 6
+#define RISCV_HWPROBE_MAX_KEY 7
 
 static inline bool riscv_hwprobe_key_is_valid(__s64 key)
 {
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 731dcd0ed4de..776af9b37e23 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -21,6 +21,7 @@
 
 extern unsigned long riscv_v_vsize;
 int riscv_v_setup_vsize(void);
+bool insn_is_vector(u32 insn_buf);
 bool riscv_v_first_use_handler(struct pt_regs *regs);
 void kernel_vector_begin(void);
 void kernel_vector_end(void);
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 060212331a03..ebacff86f134 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -68,6 +68,12 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
 #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
+#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
+#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
+#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
+#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
+#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
+#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
 /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
 
 /* Flags */
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index b286b73e763e..ce641cc6e47a 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
 }
 #endif
 
+#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
+static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
+{
+	int cpu;
+	u64 perf = -1ULL;
+
+	for_each_cpu(cpu, cpus) {
+		int this_perf = per_cpu(vector_misaligned_access, cpu);
+
+		if (perf == -1ULL)
+			perf = this_perf;
+
+		if (perf != this_perf) {
+			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
+			break;
+		}
+	}
+
+	if (perf == -1ULL)
+		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
+
+	return perf;
+}
+#else
+static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
+{
+	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
+}
+#endif
+
 static void hwprobe_one_pair(struct riscv_hwprobe *pair,
 			     const struct cpumask *cpus)
 {
@@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
 		pair->value = hwprobe_misaligned(cpus);
 		break;
 
+	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
+		pair->value = hwprobe_vec_misaligned(cpus);
+		break;
+
 	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
 		pair->value = 0;
 		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
index 2adb7c3e4dd5..8f26c3d92230 100644
--- a/arch/riscv/kernel/traps_misaligned.c
+++ b/arch/riscv/kernel/traps_misaligned.c
@@ -16,6 +16,7 @@
 #include <asm/entry-common.h>
 #include <asm/hwprobe.h>
 #include <asm/cpufeature.h>
+#include <asm/vector.h>
 
 #define INSN_MATCH_LB			0x3
 #define INSN_MASK_LB			0x707f
@@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
 
 	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
 
-#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
-	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
-#endif
-
 	if (!unaligned_enabled)
 		return -1;
 
@@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
 	if (get_insn(regs, epc, &insn))
 		return -1;
 
+#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
+	if (insn_is_vector(insn) &&
+	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
+		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
+		regs->epc = epc + INSN_LEN(insn);
+		return 0;
+	}
+
+	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
+#endif
+
 	regs->epc = 0;
 
 	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
@@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
 	return misaligned_emu_detected;
 }
 
+#ifdef CONFIG_RISCV_ISA_V
+static void check_vector_unaligned_access(struct work_struct *unused)
+{
+	int cpu = smp_processor_id();
+	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
+	unsigned long tmp_var;
+
+	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
+		return;
+
+	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
+
+	local_irq_enable();
+	kernel_vector_begin();
+	__asm__ __volatile__ (
+		".balign 4\n\t"
+		".option push\n\t"
+		".option arch, +v\n\t"
+		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
+		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
+		".option pop\n\t"
+		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");
+	kernel_vector_end();
+
+	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
+		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
+}
+
+bool check_vector_unaligned_access_all_cpus(void)
+{
+	int cpu;
+	bool ret = true;
+
+	for_each_online_cpu(cpu)
+		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
+			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
+		else
+			ret = false;
+
+
+	if (ret)
+		return true;
+	ret = true;
+
+	schedule_on_each_cpu(check_vector_unaligned_access);
+
+	for_each_online_cpu(cpu)
+		if (per_cpu(vector_misaligned_access, cpu)
+		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
+			return false;
+
+	return ret;
+}
+#else
+bool check_vector_unaligned_access_all_cpus(void)
+{
+	int cpu;
+
+	for_each_online_cpu(cpu)
+		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
+			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
+		else
+			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
+
+	return false;
+}
+#endif
+
 bool check_unaligned_access_emulated_all_cpus(void)
 {
 	int cpu;
diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
index a9a6bcb02acf..92a84239beaa 100644
--- a/arch/riscv/kernel/unaligned_access_speed.c
+++ b/arch/riscv/kernel/unaligned_access_speed.c
@@ -20,6 +20,7 @@
 #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
 
 DEFINE_PER_CPU(long, misaligned_access_speed);
+DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
 
 #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
 static cpumask_t fast_misaligned_access;
@@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
 {
 	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
 
+	check_vector_unaligned_access_all_cpus();
+
 	if (!all_cpus_emulated)
 		return check_unaligned_access_speed_all_cpus();
 
@@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
 static int check_unaligned_access_all_cpus(void)
 {
 	check_unaligned_access_emulated_all_cpus();
+	check_vector_unaligned_access_all_cpus();
 
 	return 0;
 }
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 6727d1d3b8f2..2cceab739b2c 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
 #endif
 }
 
-static bool insn_is_vector(u32 insn_buf)
+bool insn_is_vector(u32 insn_buf)
 {
 	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
 	u32 width, csr;
-- 
2.43.0

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Charlie Jenkins]

On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
> Run a unaligned vector access to test if the system supports
> vector unaligned access. Add the result to a new key in hwprobe.
> This is useful for usermode to know if vector misaligned accesses are
> supported and if they are faster or slower than equivalent byte accesses.
> 
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
>  arch/riscv/include/asm/cpufeature.h        |  2 +
>  arch/riscv/include/asm/hwprobe.h           |  2 +-
>  arch/riscv/include/asm/vector.h            |  1 +
>  arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
>  arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
>  arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
>  arch/riscv/kernel/unaligned_access_speed.c |  4 ++
>  arch/riscv/kernel/vector.c                 |  2 +-
>  8 files changed, 129 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index 347805446151..a012c8490a27 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
>  
>  #if defined(CONFIG_RISCV_MISALIGNED)
>  bool check_unaligned_access_emulated_all_cpus(void);
> +bool check_vector_unaligned_access_all_cpus(void);
>  void unaligned_emulation_finish(void);
>  bool unaligned_ctl_available(void);
>  DECLARE_PER_CPU(long, misaligned_access_speed);
> +DECLARE_PER_CPU(long, vector_misaligned_access);
>  #else
>  static inline bool unaligned_ctl_available(void)
>  {
> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> index 630507dff5ea..150a9877b0af 100644
> --- a/arch/riscv/include/asm/hwprobe.h
> +++ b/arch/riscv/include/asm/hwprobe.h
> @@ -8,7 +8,7 @@
>  
>  #include <uapi/asm/hwprobe.h>
>  
> -#define RISCV_HWPROBE_MAX_KEY 6
> +#define RISCV_HWPROBE_MAX_KEY 7
>  
>  static inline bool riscv_hwprobe_key_is_valid(__s64 key)
>  {
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index 731dcd0ed4de..776af9b37e23 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -21,6 +21,7 @@
>  
>  extern unsigned long riscv_v_vsize;
>  int riscv_v_setup_vsize(void);
> +bool insn_is_vector(u32 insn_buf);
>  bool riscv_v_first_use_handler(struct pt_regs *regs);
>  void kernel_vector_begin(void);
>  void kernel_vector_end(void);
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 060212331a03..ebacff86f134 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -68,6 +68,12 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
>  #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
> +#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
>  /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>  
>  /* Flags */
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index b286b73e763e..ce641cc6e47a 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
>  }
>  #endif
>  
> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
> +{
> +	int cpu;
> +	u64 perf = -1ULL;
> +
> +	for_each_cpu(cpu, cpus) {
> +		int this_perf = per_cpu(vector_misaligned_access, cpu);
> +
> +		if (perf == -1ULL)
> +			perf = this_perf;
> +
> +		if (perf != this_perf) {
> +			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> +			break;
> +		}
> +	}
> +
> +	if (perf == -1ULL)
> +		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> +
> +	return perf;
> +}
> +#else
> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
> +{
> +	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> +}
> +#endif
> +
>  static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>  			     const struct cpumask *cpus)
>  {
> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>  		pair->value = hwprobe_misaligned(cpus);
>  		break;
>  
> +	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
> +		pair->value = hwprobe_vec_misaligned(cpus);
> +		break;
> +
>  	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
>  		pair->value = 0;
>  		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> index 2adb7c3e4dd5..8f26c3d92230 100644
> --- a/arch/riscv/kernel/traps_misaligned.c
> +++ b/arch/riscv/kernel/traps_misaligned.c
> @@ -16,6 +16,7 @@
>  #include <asm/entry-common.h>
>  #include <asm/hwprobe.h>
>  #include <asm/cpufeature.h>
> +#include <asm/vector.h>
>  
>  #define INSN_MATCH_LB			0x3
>  #define INSN_MASK_LB			0x707f
> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
>  
>  	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
>  
> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
> -#endif
> -
>  	if (!unaligned_enabled)
>  		return -1;
>  
> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
>  	if (get_insn(regs, epc, &insn))
>  		return -1;
>  
> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> +	if (insn_is_vector(insn) &&
> +	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
> +		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
> +		regs->epc = epc + INSN_LEN(insn);
> +		return 0;
> +	}
> +
> +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;

This unconditionally sets scalar unaligned accesses even if the
unaligned access is caused by vector. Scalar unaligned accesses should
only be set to emulated if this function is entered from a scalar
unaligned load.

The rest of this function handles how scalar unaligned accesses are
emulated, and the equivalent needs to happen for vector. You need to add
routines that manually load the data from the memory address into the
vector register. When Clément did this for scalar, he provided a test
case to help reviewers [1]. Please add onto these test cases or make
your own for vector.

Link: https://github.com/clementleger/unaligned_test [1]

> +#endif
> +
>  	regs->epc = 0;
>  
>  	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
>  	return misaligned_emu_detected;
>  }
>  
> +#ifdef CONFIG_RISCV_ISA_V
> +static void check_vector_unaligned_access(struct work_struct *unused)

Can you standardize this name with the scalar version by writing
emulated in it?

"check_vector_unaligned_access_emulated_all_cpus"

> +{
> +	int cpu = smp_processor_id();
> +	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
> +	unsigned long tmp_var;
> +
> +	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
> +		return;
> +
> +	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> +
> +	local_irq_enable();
> +	kernel_vector_begin();
> +	__asm__ __volatile__ (
> +		".balign 4\n\t"
> +		".option push\n\t"
> +		".option arch, +v\n\t"
> +		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
> +		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
> +		".option pop\n\t"
> +		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");

memory is being read from, but not written to, so there is no need to
have a memory clobber.

> +	kernel_vector_end();
> +
> +	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
> +		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> +}
> +
> +bool check_vector_unaligned_access_all_cpus(void)
> +{
> +	int cpu;
> +	bool ret = true;
> +
> +	for_each_online_cpu(cpu)
> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))

zicclsm is not specific to vector so it can be extracted out of this
vector specific function. Assuming that hardware properly reports the
extension, if zicclsm is present then it is known that both vector and
scalar unaligned accesses are supported.

> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;

Please use the exising UNKNOWN terminology instead of renaming to
SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
accesses are supported.

> +		else
> +			ret = false;
> +
> +
> +	if (ret)
> +		return true;
> +	ret = true;
> +
> +	schedule_on_each_cpu(check_vector_unaligned_access);
> +
> +	for_each_online_cpu(cpu)
> +		if (per_cpu(vector_misaligned_access, cpu)
> +		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
> +			return false;
> +
> +	return ret;
> +}
> +#else

If CONFIG_RISCV_ISA_V is not set, there is no value in checking if
vector unaligned accesses are supported because userspace will not be
allowed to use vector instructions anyway.

- Charlie

> +bool check_vector_unaligned_access_all_cpus(void)
> +{
> +	int cpu;
> +
> +	for_each_online_cpu(cpu)
> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> +		else
> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
> +
> +	return false;
> +}
> +#endif
> +
>  bool check_unaligned_access_emulated_all_cpus(void)
>  {
>  	int cpu;
> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> index a9a6bcb02acf..92a84239beaa 100644
> --- a/arch/riscv/kernel/unaligned_access_speed.c
> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> @@ -20,6 +20,7 @@
>  #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
>  
>  DEFINE_PER_CPU(long, misaligned_access_speed);
> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>  
>  #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>  static cpumask_t fast_misaligned_access;
> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
>  {
>  	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>  
> +	check_vector_unaligned_access_all_cpus();
> +
>  	if (!all_cpus_emulated)
>  		return check_unaligned_access_speed_all_cpus();
>  
> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
>  static int check_unaligned_access_all_cpus(void)
>  {
>  	check_unaligned_access_emulated_all_cpus();
> +	check_vector_unaligned_access_all_cpus();
>  
>  	return 0;
>  }
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 6727d1d3b8f2..2cceab739b2c 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
>  #endif
>  }
>  
> -static bool insn_is_vector(u32 insn_buf)
> +bool insn_is_vector(u32 insn_buf)
>  {
>  	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
>  	u32 width, csr;
> -- 
> 2.43.0
> 

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Charlie Jenkins]

On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote:
> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
> > Run a unaligned vector access to test if the system supports
> > vector unaligned access. Add the result to a new key in hwprobe.
> > This is useful for usermode to know if vector misaligned accesses are
> > supported and if they are faster or slower than equivalent byte accesses.
> > 
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/cpufeature.h        |  2 +
> >  arch/riscv/include/asm/hwprobe.h           |  2 +-
> >  arch/riscv/include/asm/vector.h            |  1 +
> >  arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
> >  arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
> >  arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
> >  arch/riscv/kernel/unaligned_access_speed.c |  4 ++
> >  arch/riscv/kernel/vector.c                 |  2 +-
> >  8 files changed, 129 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> > index 347805446151..a012c8490a27 100644
> > --- a/arch/riscv/include/asm/cpufeature.h
> > +++ b/arch/riscv/include/asm/cpufeature.h
> > @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
> >  
> >  #if defined(CONFIG_RISCV_MISALIGNED)
> >  bool check_unaligned_access_emulated_all_cpus(void);
> > +bool check_vector_unaligned_access_all_cpus(void);
> >  void unaligned_emulation_finish(void);
> >  bool unaligned_ctl_available(void);
> >  DECLARE_PER_CPU(long, misaligned_access_speed);
> > +DECLARE_PER_CPU(long, vector_misaligned_access);
> >  #else
> >  static inline bool unaligned_ctl_available(void)
> >  {
> > diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> > index 630507dff5ea..150a9877b0af 100644
> > --- a/arch/riscv/include/asm/hwprobe.h
> > +++ b/arch/riscv/include/asm/hwprobe.h
> > @@ -8,7 +8,7 @@
> >  
> >  #include <uapi/asm/hwprobe.h>
> >  
> > -#define RISCV_HWPROBE_MAX_KEY 6
> > +#define RISCV_HWPROBE_MAX_KEY 7
> >  
> >  static inline bool riscv_hwprobe_key_is_valid(__s64 key)
> >  {
> > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> > index 731dcd0ed4de..776af9b37e23 100644
> > --- a/arch/riscv/include/asm/vector.h
> > +++ b/arch/riscv/include/asm/vector.h
> > @@ -21,6 +21,7 @@
> >  
> >  extern unsigned long riscv_v_vsize;
> >  int riscv_v_setup_vsize(void);
> > +bool insn_is_vector(u32 insn_buf);
> >  bool riscv_v_first_use_handler(struct pt_regs *regs);
> >  void kernel_vector_begin(void);
> >  void kernel_vector_end(void);
> > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > index 060212331a03..ebacff86f134 100644
> > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > @@ -68,6 +68,12 @@ struct riscv_hwprobe {
> >  #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
> >  #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
> >  #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
> > +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
> > +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
> > +#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
> > +#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
> > +#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
> > +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
> >  /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
> >  
> >  /* Flags */
> > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > index b286b73e763e..ce641cc6e47a 100644
> > --- a/arch/riscv/kernel/sys_hwprobe.c
> > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
> >  }
> >  #endif
> >  
> > +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
> > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
> > +{
> > +	int cpu;
> > +	u64 perf = -1ULL;
> > +
> > +	for_each_cpu(cpu, cpus) {
> > +		int this_perf = per_cpu(vector_misaligned_access, cpu);
> > +
> > +		if (perf == -1ULL)
> > +			perf = this_perf;
> > +
> > +		if (perf != this_perf) {
> > +			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (perf == -1ULL)
> > +		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> > +
> > +	return perf;
> > +}
> > +#else
> > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
> > +{

I meant to mention this in my last message!

The scalar version has cutouts for configs here like:

	if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS))
		return RISCV_HWPROBE_MISALIGNED_FAST;

Having this functionality on vector as well would be much appreciated.
I don't think it's valid to assume that vector and scalar have the same
speed, so this would require a vector version of the RISCV_MISALIGNED
tree in arch/riscv/Kconfig.

- Charlie

> > +	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> > +}
> > +#endif
> > +
> >  static void hwprobe_one_pair(struct riscv_hwprobe *pair,
> >  			     const struct cpumask *cpus)
> >  {
> > @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
> >  		pair->value = hwprobe_misaligned(cpus);
> >  		break;
> >  
> > +	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
> > +		pair->value = hwprobe_vec_misaligned(cpus);
> > +		break;
> > +
> >  	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
> >  		pair->value = 0;
> >  		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
> > diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> > index 2adb7c3e4dd5..8f26c3d92230 100644
> > --- a/arch/riscv/kernel/traps_misaligned.c
> > +++ b/arch/riscv/kernel/traps_misaligned.c
> > @@ -16,6 +16,7 @@
> >  #include <asm/entry-common.h>
> >  #include <asm/hwprobe.h>
> >  #include <asm/cpufeature.h>
> > +#include <asm/vector.h>
> >  
> >  #define INSN_MATCH_LB			0x3
> >  #define INSN_MASK_LB			0x707f
> > @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
> >  
> >  	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
> >  
> > -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> > -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
> > -#endif
> > -
> >  	if (!unaligned_enabled)
> >  		return -1;
> >  
> > @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
> >  	if (get_insn(regs, epc, &insn))
> >  		return -1;
> >  
> > +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> > +	if (insn_is_vector(insn) &&
> > +	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
> > +		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
> > +		regs->epc = epc + INSN_LEN(insn);
> > +		return 0;
> > +	}
> > +
> > +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
> 
> This unconditionally sets scalar unaligned accesses even if the
> unaligned access is caused by vector. Scalar unaligned accesses should
> only be set to emulated if this function is entered from a scalar
> unaligned load.
> 
> The rest of this function handles how scalar unaligned accesses are
> emulated, and the equivalent needs to happen for vector. You need to add
> routines that manually load the data from the memory address into the
> vector register. When Clément did this for scalar, he provided a test
> case to help reviewers [1]. Please add onto these test cases or make
> your own for vector.
> 
> Link: https://github.com/clementleger/unaligned_test [1]
> 
> > +#endif
> > +
> >  	regs->epc = 0;
> >  
> >  	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
> > @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
> >  	return misaligned_emu_detected;
> >  }
> >  
> > +#ifdef CONFIG_RISCV_ISA_V
> > +static void check_vector_unaligned_access(struct work_struct *unused)
> 
> Can you standardize this name with the scalar version by writing
> emulated in it?
> 
> "check_vector_unaligned_access_emulated_all_cpus"
> 
> > +{
> > +	int cpu = smp_processor_id();
> > +	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
> > +	unsigned long tmp_var;
> > +
> > +	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
> > +		return;
> > +
> > +	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> > +
> > +	local_irq_enable();
> > +	kernel_vector_begin();
> > +	__asm__ __volatile__ (
> > +		".balign 4\n\t"
> > +		".option push\n\t"
> > +		".option arch, +v\n\t"
> > +		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
> > +		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
> > +		".option pop\n\t"
> > +		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");
> 
> memory is being read from, but not written to, so there is no need to
> have a memory clobber.
> 
> > +	kernel_vector_end();
> > +
> > +	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
> > +		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> > +}
> > +
> > +bool check_vector_unaligned_access_all_cpus(void)
> > +{
> > +	int cpu;
> > +	bool ret = true;
> > +
> > +	for_each_online_cpu(cpu)
> > +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
> 
> zicclsm is not specific to vector so it can be extracted out of this
> vector specific function. Assuming that hardware properly reports the
> extension, if zicclsm is present then it is known that both vector and
> scalar unaligned accesses are supported.
> 
> > +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> 
> Please use the exising UNKNOWN terminology instead of renaming to
> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
> accesses are supported.
> 
> > +		else
> > +			ret = false;
> > +
> > +
> > +	if (ret)
> > +		return true;
> > +	ret = true;
> > +
> > +	schedule_on_each_cpu(check_vector_unaligned_access);
> > +
> > +	for_each_online_cpu(cpu)
> > +		if (per_cpu(vector_misaligned_access, cpu)
> > +		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
> > +			return false;
> > +
> > +	return ret;
> > +}
> > +#else
> 
> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if
> vector unaligned accesses are supported because userspace will not be
> allowed to use vector instructions anyway.
> 
> - Charlie
> 
> > +bool check_vector_unaligned_access_all_cpus(void)
> > +{
> > +	int cpu;
> > +
> > +	for_each_online_cpu(cpu)
> > +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
> > +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> > +		else
> > +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
> > +
> > +	return false;
> > +}
> > +#endif
> > +
> >  bool check_unaligned_access_emulated_all_cpus(void)
> >  {
> >  	int cpu;
> > diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> > index a9a6bcb02acf..92a84239beaa 100644
> > --- a/arch/riscv/kernel/unaligned_access_speed.c
> > +++ b/arch/riscv/kernel/unaligned_access_speed.c
> > @@ -20,6 +20,7 @@
> >  #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
> >  
> >  DEFINE_PER_CPU(long, misaligned_access_speed);
> > +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> >  
> >  #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> >  static cpumask_t fast_misaligned_access;
> > @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
> >  {
> >  	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
> >  
> > +	check_vector_unaligned_access_all_cpus();
> > +
> >  	if (!all_cpus_emulated)
> >  		return check_unaligned_access_speed_all_cpus();
> >  
> > @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
> >  static int check_unaligned_access_all_cpus(void)
> >  {
> >  	check_unaligned_access_emulated_all_cpus();
> > +	check_vector_unaligned_access_all_cpus();
> >  
> >  	return 0;
> >  }
> > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> > index 6727d1d3b8f2..2cceab739b2c 100644
> > --- a/arch/riscv/kernel/vector.c
> > +++ b/arch/riscv/kernel/vector.c
> > @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
> >  #endif
> >  }
> >  
> > -static bool insn_is_vector(u32 insn_buf)
> > +bool insn_is_vector(u32 insn_buf)
> >  {
> >  	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
> >  	u32 width, csr;
> > -- 
> > 2.43.0
> > 

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Jesse Taube]

On 6/6/24 19:13, Charlie Jenkins wrote:
> On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote:
>> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
>>> Run a unaligned vector access to test if the system supports
>>> vector unaligned access. Add the result to a new key in hwprobe.
>>> This is useful for usermode to know if vector misaligned accesses are
>>> supported and if they are faster or slower than equivalent byte accesses.
>>>
>>> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
>>> ---
>>>   arch/riscv/include/asm/cpufeature.h        |  2 +
>>>   arch/riscv/include/asm/hwprobe.h           |  2 +-
>>>   arch/riscv/include/asm/vector.h            |  1 +
>>>   arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
>>>   arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
>>>   arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
>>>   arch/riscv/kernel/unaligned_access_speed.c |  4 ++
>>>   arch/riscv/kernel/vector.c                 |  2 +-
>>>   8 files changed, 129 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
>>> index 347805446151..a012c8490a27 100644
>>> --- a/arch/riscv/include/asm/cpufeature.h
>>> +++ b/arch/riscv/include/asm/cpufeature.h
>>> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
>>>   
>>>   #if defined(CONFIG_RISCV_MISALIGNED)
>>>   bool check_unaligned_access_emulated_all_cpus(void);
>>> +bool check_vector_unaligned_access_all_cpus(void);
>>>   void unaligned_emulation_finish(void);
>>>   bool unaligned_ctl_available(void);
>>>   DECLARE_PER_CPU(long, misaligned_access_speed);
>>> +DECLARE_PER_CPU(long, vector_misaligned_access);
>>>   #else
>>>   static inline bool unaligned_ctl_available(void)
>>>   {
>>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
>>> index 630507dff5ea..150a9877b0af 100644
>>> --- a/arch/riscv/include/asm/hwprobe.h
>>> +++ b/arch/riscv/include/asm/hwprobe.h
>>> @@ -8,7 +8,7 @@
>>>   
>>>   #include <uapi/asm/hwprobe.h>
>>>   
>>> -#define RISCV_HWPROBE_MAX_KEY 6
>>> +#define RISCV_HWPROBE_MAX_KEY 7
>>>   
>>>   static inline bool riscv_hwprobe_key_is_valid(__s64 key)
>>>   {
>>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
>>> index 731dcd0ed4de..776af9b37e23 100644
>>> --- a/arch/riscv/include/asm/vector.h
>>> +++ b/arch/riscv/include/asm/vector.h
>>> @@ -21,6 +21,7 @@
>>>   
>>>   extern unsigned long riscv_v_vsize;
>>>   int riscv_v_setup_vsize(void);
>>> +bool insn_is_vector(u32 insn_buf);
>>>   bool riscv_v_first_use_handler(struct pt_regs *regs);
>>>   void kernel_vector_begin(void);
>>>   void kernel_vector_end(void);
>>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
>>> index 060212331a03..ebacff86f134 100644
>>> --- a/arch/riscv/include/uapi/asm/hwprobe.h
>>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
>>> @@ -68,6 +68,12 @@ struct riscv_hwprobe {
>>>   #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
>>>   #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
>>>   #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
>>> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
>>>   /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>>>   
>>>   /* Flags */
>>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
>>> index b286b73e763e..ce641cc6e47a 100644
>>> --- a/arch/riscv/kernel/sys_hwprobe.c
>>> +++ b/arch/riscv/kernel/sys_hwprobe.c
>>> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
>>>   }
>>>   #endif
>>>   
>>> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>>> +{
>>> +	int cpu;
>>> +	u64 perf = -1ULL;
>>> +
>>> +	for_each_cpu(cpu, cpus) {
>>> +		int this_perf = per_cpu(vector_misaligned_access, cpu);
>>> +
>>> +		if (perf == -1ULL)
>>> +			perf = this_perf;
>>> +
>>> +		if (perf != this_perf) {
>>> +			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	if (perf == -1ULL)
>>> +		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>> +
>>> +	return perf;
>>> +}
>>> +#else
>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>>> +{
> 
> I meant to mention this in my last message!
> 
> The scalar version has cutouts for configs here like:
> 
> 	if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS))
> 		return RISCV_HWPROBE_MISALIGNED_FAST;
> 

Will add

> Having this functionality on vector as well would be much appreciated.
> I don't think it's valid to assume that vector and scalar have the same
> speed, so this would require a vector version of the RISCV_MISALIGNED
> tree in arch/riscv/Kconfig.
> 
> - Charlie
> 
>>> +	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>> +}
>>> +#endif
>>> +
>>>   static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>>   			     const struct cpumask *cpus)
>>>   {
>>> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>>   		pair->value = hwprobe_misaligned(cpus);
>>>   		break;
>>>   
>>> +	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
>>> +		pair->value = hwprobe_vec_misaligned(cpus);
>>> +		break;
>>> +
>>>   	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
>>>   		pair->value = 0;
>>>   		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
>>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
>>> index 2adb7c3e4dd5..8f26c3d92230 100644
>>> --- a/arch/riscv/kernel/traps_misaligned.c
>>> +++ b/arch/riscv/kernel/traps_misaligned.c
>>> @@ -16,6 +16,7 @@
>>>   #include <asm/entry-common.h>
>>>   #include <asm/hwprobe.h>
>>>   #include <asm/cpufeature.h>
>>> +#include <asm/vector.h>
>>>   
>>>   #define INSN_MATCH_LB			0x3
>>>   #define INSN_MASK_LB			0x707f
>>> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
>>>   
>>>   	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
>>>   
>>> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>> -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
>>> -#endif
>>> -
>>>   	if (!unaligned_enabled)
>>>   		return -1;
>>>   
>>> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
>>>   	if (get_insn(regs, epc, &insn))
>>>   		return -1;
>>>   
>>> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>> +	if (insn_is_vector(insn) &&
>>> +	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
>>> +		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>>> +		regs->epc = epc + INSN_LEN(insn);
>>> +		return 0;

There is a return before scalar speed is set.

>>> +	}
>>> +
>>> +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
>>
>> This unconditionally sets scalar unaligned accesses even if the
>> unaligned access is caused by vector. Scalar unaligned accesses should
>> only be set to emulated if this function is entered from a scalar
>> unaligned load.
>>
>> The rest of this function handles how scalar unaligned accesses are
>> emulated, and the equivalent needs to happen for vector. You need to add
>> routines that manually load the data from the memory address into the
>> vector register. When Clément did this for scalar, he provided a test
>> case to help reviewers [1]. Please add onto these test cases or make
>> your own for vector.

I wansnt planing on adding emulation in this patch. I can if needed.

>>
>> Link: https://github.com/clementleger/unaligned_test [1]
>>
>>> +#endif
>>> +
>>>   	regs->epc = 0;
>>>   
>>>   	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
>>> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
>>>   	return misaligned_emu_detected;
>>>   }
>>>   
>>> +#ifdef CONFIG_RISCV_ISA_V
>>> +static void check_vector_unaligned_access(struct work_struct *unused)
>>
>> Can you standardize this name with the scalar version by writing
>> emulated in it?

We dont emulate it so that wouldn't make sence.

>>
>> "check_vector_unaligned_access_emulated_all_cpus"
>>
>>> +{
>>> +	int cpu = smp_processor_id();
>>> +	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
>>> +	unsigned long tmp_var;
>>> +
>>> +	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
>>> +		return;
>>> +
>>> +	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>> +
>>> +	local_irq_enable();
>>> +	kernel_vector_begin();
>>> +	__asm__ __volatile__ (
>>> +		".balign 4\n\t"
>>> +		".option push\n\t"
>>> +		".option arch, +v\n\t"
>>> +		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
>>> +		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
>>> +		".option pop\n\t"
>>> +		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");
>>
>> memory is being read from, but not written to, so there is no need to
>> have a memory clobber.

fixed.

>>
>>> +	kernel_vector_end();
>>> +
>>> +	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
>>> +		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>> +}
>>> +
>>> +bool check_vector_unaligned_access_all_cpus(void)
>>> +{
>>> +	int cpu;
>>> +	bool ret = true;
>>> +
>>> +	for_each_online_cpu(cpu)
>>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
>>
>> zicclsm is not specific to vector so it can be extracted out of this
>> vector specific function. Assuming that hardware properly reports the
>> extension, if zicclsm is present then it is known that both vector and
>> scalar unaligned accesses are supported.

Added so we don't need to waste cycles testing support.

>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>
>> Please use the exising UNKNOWN terminology instead of renaming to
>> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
>> accesses are supported.

Conor didnt like using UNKNOWN a proxy for "SUPPORTED"
Having SUPPORTED is better then assuing the speed to be slow.

>>
>>> +		else
>>> +			ret = false;
>>> +
>>> +
>>> +	if (ret)
>>> +		return true;
>>> +	ret = true;
>>> +
>>> +	schedule_on_each_cpu(check_vector_unaligned_access);
>>> +
>>> +	for_each_online_cpu(cpu)
>>> +		if (per_cpu(vector_misaligned_access, cpu)
>>> +		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
>>> +			return false;
>>> +
>>> +	return ret;
>>> +}
>>> +#else
>>
>> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if
>> vector unaligned accesses are supported because userspace will not be
>> allowed to use vector instructions anyway.

Oh I'm silly meant to be seting to all UNSUPPORTED.


Thanks,
Jesse Taube

>>
>> - Charlie
>>
>>> +bool check_vector_unaligned_access_all_cpus(void)
>>> +{
>>> +	int cpu;
>>> +
>>> +	for_each_online_cpu(cpu)
>>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>> +		else
>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>>> +
>>> +	return false;
>>> +}
>>> +#endif
>>> +
>>>   bool check_unaligned_access_emulated_all_cpus(void)
>>>   {
>>>   	int cpu;
>>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
>>> index a9a6bcb02acf..92a84239beaa 100644
>>> --- a/arch/riscv/kernel/unaligned_access_speed.c
>>> +++ b/arch/riscv/kernel/unaligned_access_speed.c
>>> @@ -20,6 +20,7 @@
>>>   #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
>>>   
>>>   DEFINE_PER_CPU(long, misaligned_access_speed);
>>> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>>   
>>>   #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>>   static cpumask_t fast_misaligned_access;
>>> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
>>>   {
>>>   	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>>>   
>>> +	check_vector_unaligned_access_all_cpus();
>>> +
>>>   	if (!all_cpus_emulated)
>>>   		return check_unaligned_access_speed_all_cpus();
>>>   
>>> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
>>>   static int check_unaligned_access_all_cpus(void)
>>>   {
>>>   	check_unaligned_access_emulated_all_cpus();
>>> +	check_vector_unaligned_access_all_cpus();
>>>   
>>>   	return 0;
>>>   }
>>> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
>>> index 6727d1d3b8f2..2cceab739b2c 100644
>>> --- a/arch/riscv/kernel/vector.c
>>> +++ b/arch/riscv/kernel/vector.c
>>> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
>>>   #endif
>>>   }
>>>   
>>> -static bool insn_is_vector(u32 insn_buf)
>>> +bool insn_is_vector(u32 insn_buf)
>>>   {
>>>   	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
>>>   	u32 width, csr;
>>> -- 
>>> 2.43.0
>>>

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Jesse Taube]

On 6/7/24 15:53, Jesse Taube wrote:
> 
> 
> On 6/6/24 19:13, Charlie Jenkins wrote:
>> On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote:
>>> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
>>>> Run a unaligned vector access to test if the system supports
>>>> vector unaligned access. Add the result to a new key in hwprobe.
>>>> This is useful for usermode to know if vector misaligned accesses are
>>>> supported and if they are faster or slower than equivalent byte accesses.
>>>>
>>>> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
>>>> ---
>>>>    arch/riscv/include/asm/cpufeature.h        |  2 +
>>>>    arch/riscv/include/asm/hwprobe.h           |  2 +-
>>>>    arch/riscv/include/asm/vector.h            |  1 +
>>>>    arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
>>>>    arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
>>>>    arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
>>>>    arch/riscv/kernel/unaligned_access_speed.c |  4 ++
>>>>    arch/riscv/kernel/vector.c                 |  2 +-
>>>>    8 files changed, 129 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
>>>> index 347805446151..a012c8490a27 100644
>>>> --- a/arch/riscv/include/asm/cpufeature.h
>>>> +++ b/arch/riscv/include/asm/cpufeature.h
>>>> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
>>>>    
>>>>    #if defined(CONFIG_RISCV_MISALIGNED)
>>>>    bool check_unaligned_access_emulated_all_cpus(void);
>>>> +bool check_vector_unaligned_access_all_cpus(void);
>>>>    void unaligned_emulation_finish(void);
>>>>    bool unaligned_ctl_available(void);
>>>>    DECLARE_PER_CPU(long, misaligned_access_speed);
>>>> +DECLARE_PER_CPU(long, vector_misaligned_access);
>>>>    #else
>>>>    static inline bool unaligned_ctl_available(void)
>>>>    {
>>>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
>>>> index 630507dff5ea..150a9877b0af 100644
>>>> --- a/arch/riscv/include/asm/hwprobe.h
>>>> +++ b/arch/riscv/include/asm/hwprobe.h
>>>> @@ -8,7 +8,7 @@
>>>>    
>>>>    #include <uapi/asm/hwprobe.h>
>>>>    
>>>> -#define RISCV_HWPROBE_MAX_KEY 6
>>>> +#define RISCV_HWPROBE_MAX_KEY 7
>>>>    
>>>>    static inline bool riscv_hwprobe_key_is_valid(__s64 key)
>>>>    {
>>>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
>>>> index 731dcd0ed4de..776af9b37e23 100644
>>>> --- a/arch/riscv/include/asm/vector.h
>>>> +++ b/arch/riscv/include/asm/vector.h
>>>> @@ -21,6 +21,7 @@
>>>>    
>>>>    extern unsigned long riscv_v_vsize;
>>>>    int riscv_v_setup_vsize(void);
>>>> +bool insn_is_vector(u32 insn_buf);
>>>>    bool riscv_v_first_use_handler(struct pt_regs *regs);
>>>>    void kernel_vector_begin(void);
>>>>    void kernel_vector_end(void);
>>>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
>>>> index 060212331a03..ebacff86f134 100644
>>>> --- a/arch/riscv/include/uapi/asm/hwprobe.h
>>>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
>>>> @@ -68,6 +68,12 @@ struct riscv_hwprobe {
>>>>    #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
>>>>    #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
>>>>    #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
>>>> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
>>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
>>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
>>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
>>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
>>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
>>>>    /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>>>>    
>>>>    /* Flags */
>>>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
>>>> index b286b73e763e..ce641cc6e47a 100644
>>>> --- a/arch/riscv/kernel/sys_hwprobe.c
>>>> +++ b/arch/riscv/kernel/sys_hwprobe.c
>>>> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
>>>>    }
>>>>    #endif
>>>>    
>>>> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
>>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>>>> +{
>>>> +	int cpu;
>>>> +	u64 perf = -1ULL;
>>>> +
>>>> +	for_each_cpu(cpu, cpus) {
>>>> +		int this_perf = per_cpu(vector_misaligned_access, cpu);
>>>> +
>>>> +		if (perf == -1ULL)
>>>> +			perf = this_perf;
>>>> +
>>>> +		if (perf != this_perf) {
>>>> +			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>>> +			break;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	if (perf == -1ULL)
>>>> +		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>>> +
>>>> +	return perf;
>>>> +}
>>>> +#else
>>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>>>> +{
>>
>> I meant to mention this in my last message!
>>
>> The scalar version has cutouts for configs here like:
>>
>> 	if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS))
>> 		return RISCV_HWPROBE_MISALIGNED_FAST;
>>
> 

For both scalar and vector CONFIG_RISCV_PROBE_UNALIGNED_ACCESS probes 
the speed not support. So misaligned_access_speed will have a valid value.

```

	if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_VEC_UNALIGNED_ACCESS))
		return RISCV_HWPROBE_MISALIGNED_FAST;

	if(IS_ENABLED(CONFIG_RISCV_SLOW_VEC_UNALIGNED_ACCESS))
		return RISCV_HWPROBE_VEC_MISALIGNED_SLOW;

	if(IS_ENABLED(CONFIG_RISCV_VEC_UNALIGNED_ACCESS))
		
/* Sence we cant gurentee that we do have UNALIGNED_ACCESS we can return 
the result from the test */

		return per_cpu(vector_misaligned_access, cpu);

	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
```

> Will add
> 
>> Having this functionality on vector as well would be much appreciated.
>> I don't think it's valid to assume that vector and scalar have the same
>> speed, so this would require a vector version of the RISCV_MISALIGNED
>> tree in arch/riscv/Kconfig.
>>
>> - Charlie
>>
>>>> +	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>>> +}
>>>> +#endif
>>>> +
>>>>    static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>>>    			     const struct cpumask *cpus)
>>>>    {
>>>> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>>>    		pair->value = hwprobe_misaligned(cpus);
>>>>    		break;
>>>>    
>>>> +	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
>>>> +		pair->value = hwprobe_vec_misaligned(cpus);
>>>> +		break;
>>>> +
>>>>    	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
>>>>    		pair->value = 0;
>>>>    		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
>>>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
>>>> index 2adb7c3e4dd5..8f26c3d92230 100644
>>>> --- a/arch/riscv/kernel/traps_misaligned.c
>>>> +++ b/arch/riscv/kernel/traps_misaligned.c
>>>> @@ -16,6 +16,7 @@
>>>>    #include <asm/entry-common.h>
>>>>    #include <asm/hwprobe.h>
>>>>    #include <asm/cpufeature.h>
>>>> +#include <asm/vector.h>
>>>>    
>>>>    #define INSN_MATCH_LB			0x3
>>>>    #define INSN_MASK_LB			0x707f
>>>> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
>>>>    
>>>>    	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
>>>>    
>>>> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>>> -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
>>>> -#endif
>>>> -
>>>>    	if (!unaligned_enabled)
>>>>    		return -1;
>>>>    
>>>> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
>>>>    	if (get_insn(regs, epc, &insn))
>>>>    		return -1;
>>>>    
>>>> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>>> +	if (insn_is_vector(insn) &&
>>>> +	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
>>>> +		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>>>> +		regs->epc = epc + INSN_LEN(insn);
>>>> +		return 0;
> 
> There is a return before scalar speed is set.
> 
>>>> +	}
>>>> +
>>>> +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
>>>
>>> This unconditionally sets scalar unaligned accesses even if the
>>> unaligned access is caused by vector. Scalar unaligned accesses should
>>> only be set to emulated if this function is entered from a scalar
>>> unaligned load.
>>>
>>> The rest of this function handles how scalar unaligned accesses are
>>> emulated, and the equivalent needs to happen for vector. You need to add
>>> routines that manually load the data from the memory address into the
>>> vector register. When Clément did this for scalar, he provided a test
>>> case to help reviewers [1]. Please add onto these test cases or make
>>> your own for vector.
> 
> I wansnt planing on adding emulation in this patch. I can if needed.
> 
>>>
>>> Link: https://github.com/clementleger/unaligned_test [1]
>>>
>>>> +#endif
>>>> +
>>>>    	regs->epc = 0;
>>>>    
>>>>    	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
>>>> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
>>>>    	return misaligned_emu_detected;
>>>>    }
>>>>    
>>>> +#ifdef CONFIG_RISCV_ISA_V
>>>> +static void check_vector_unaligned_access(struct work_struct *unused)
>>>
>>> Can you standardize this name with the scalar version by writing
>>> emulated in it?
> 
> We dont emulate it so that wouldn't make sence.
> 
>>>
>>> "check_vector_unaligned_access_emulated_all_cpus"
>>>
>>>> +{
>>>> +	int cpu = smp_processor_id();
>>>> +	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
>>>> +	unsigned long tmp_var;
>>>> +
>>>> +	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
>>>> +		return;
>>>> +
>>>> +	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>>> +
>>>> +	local_irq_enable();
>>>> +	kernel_vector_begin();
>>>> +	__asm__ __volatile__ (
>>>> +		".balign 4\n\t"
>>>> +		".option push\n\t"
>>>> +		".option arch, +v\n\t"
>>>> +		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
>>>> +		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
>>>> +		".option pop\n\t"
>>>> +		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");
>>>
>>> memory is being read from, but not written to, so there is no need to
>>> have a memory clobber.
> 
> fixed.
> 
>>>
>>>> +	kernel_vector_end();
>>>> +
>>>> +	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
>>>> +		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>>> +}
>>>> +
>>>> +bool check_vector_unaligned_access_all_cpus(void)
>>>> +{
>>>> +	int cpu;
>>>> +	bool ret = true;
>>>> +
>>>> +	for_each_online_cpu(cpu)
>>>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
>>>
>>> zicclsm is not specific to vector so it can be extracted out of this
>>> vector specific function. Assuming that hardware properly reports the
>>> extension, if zicclsm is present then it is known that both vector and
>>> scalar unaligned accesses are supported.
> 
> Added so we don't need to waste cycles testing support.
> 
>>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>>
>>> Please use the exising UNKNOWN terminology instead of renaming to
>>> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
>>> accesses are supported.
> 
> Conor didnt like using UNKNOWN a proxy for "SUPPORTED"
> Having SUPPORTED is better then assuing the speed to be slow.
> 
>>>
>>>> +		else
>>>> +			ret = false;
>>>> +
>>>> +
>>>> +	if (ret)
>>>> +		return true;
>>>> +	ret = true;
>>>> +
>>>> +	schedule_on_each_cpu(check_vector_unaligned_access);
>>>> +
>>>> +	for_each_online_cpu(cpu)
>>>> +		if (per_cpu(vector_misaligned_access, cpu)
>>>> +		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
>>>> +			return false;
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +#else
>>>
>>> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if
>>> vector unaligned accesses are supported because userspace will not be
>>> allowed to use vector instructions anyway.
> 
> Oh I'm silly meant to be seting to all UNSUPPORTED.
> 
> 
> Thanks,
> Jesse Taube
> 
>>>
>>> - Charlie
>>>
>>>> +bool check_vector_unaligned_access_all_cpus(void)
>>>> +{
>>>> +	int cpu;
>>>> +
>>>> +	for_each_online_cpu(cpu)
>>>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
>>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>>> +		else
>>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>>>> +
>>>> +	return false;
>>>> +}
>>>> +#endif
>>>> +
>>>>    bool check_unaligned_access_emulated_all_cpus(void)
>>>>    {
>>>>    	int cpu;
>>>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
>>>> index a9a6bcb02acf..92a84239beaa 100644
>>>> --- a/arch/riscv/kernel/unaligned_access_speed.c
>>>> +++ b/arch/riscv/kernel/unaligned_access_speed.c
>>>> @@ -20,6 +20,7 @@
>>>>    #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
>>>>    
>>>>    DEFINE_PER_CPU(long, misaligned_access_speed);
>>>> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>>>    
>>>>    #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>>>    static cpumask_t fast_misaligned_access;
>>>> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
>>>>    {
>>>>    	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>>>>    
>>>> +	check_vector_unaligned_access_all_cpus();
>>>> +
>>>>    	if (!all_cpus_emulated)
>>>>    		return check_unaligned_access_speed_all_cpus();
>>>>    
>>>> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
>>>>    static int check_unaligned_access_all_cpus(void)
>>>>    {
>>>>    	check_unaligned_access_emulated_all_cpus();
>>>> +	check_vector_unaligned_access_all_cpus();
>>>>    
>>>>    	return 0;
>>>>    }
>>>> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
>>>> index 6727d1d3b8f2..2cceab739b2c 100644
>>>> --- a/arch/riscv/kernel/vector.c
>>>> +++ b/arch/riscv/kernel/vector.c
>>>> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
>>>>    #endif
>>>>    }
>>>>    
>>>> -static bool insn_is_vector(u32 insn_buf)
>>>> +bool insn_is_vector(u32 insn_buf)
>>>>    {
>>>>    	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
>>>>    	u32 width, csr;
>>>> -- 
>>>> 2.43.0
>>>>

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Charlie Jenkins]

On Fri, Jun 07, 2024 at 03:53:23PM -0400, Jesse Taube wrote:
> 
> 
> On 6/6/24 19:13, Charlie Jenkins wrote:
> > On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote:
> > > On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
> > > > Run a unaligned vector access to test if the system supports
> > > > vector unaligned access. Add the result to a new key in hwprobe.
> > > > This is useful for usermode to know if vector misaligned accesses are
> > > > supported and if they are faster or slower than equivalent byte accesses.
> > > > 
> > > > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> > > > ---
> > > >   arch/riscv/include/asm/cpufeature.h        |  2 +
> > > >   arch/riscv/include/asm/hwprobe.h           |  2 +-
> > > >   arch/riscv/include/asm/vector.h            |  1 +
> > > >   arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
> > > >   arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
> > > >   arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
> > > >   arch/riscv/kernel/unaligned_access_speed.c |  4 ++
> > > >   arch/riscv/kernel/vector.c                 |  2 +-
> > > >   8 files changed, 129 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> > > > index 347805446151..a012c8490a27 100644
> > > > --- a/arch/riscv/include/asm/cpufeature.h
> > > > +++ b/arch/riscv/include/asm/cpufeature.h
> > > > @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
> > > >   #if defined(CONFIG_RISCV_MISALIGNED)
> > > >   bool check_unaligned_access_emulated_all_cpus(void);
> > > > +bool check_vector_unaligned_access_all_cpus(void);
> > > >   void unaligned_emulation_finish(void);
> > > >   bool unaligned_ctl_available(void);
> > > >   DECLARE_PER_CPU(long, misaligned_access_speed);
> > > > +DECLARE_PER_CPU(long, vector_misaligned_access);
> > > >   #else
> > > >   static inline bool unaligned_ctl_available(void)
> > > >   {
> > > > diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> > > > index 630507dff5ea..150a9877b0af 100644
> > > > --- a/arch/riscv/include/asm/hwprobe.h
> > > > +++ b/arch/riscv/include/asm/hwprobe.h
> > > > @@ -8,7 +8,7 @@
> > > >   #include <uapi/asm/hwprobe.h>
> > > > -#define RISCV_HWPROBE_MAX_KEY 6
> > > > +#define RISCV_HWPROBE_MAX_KEY 7
> > > >   static inline bool riscv_hwprobe_key_is_valid(__s64 key)
> > > >   {
> > > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> > > > index 731dcd0ed4de..776af9b37e23 100644
> > > > --- a/arch/riscv/include/asm/vector.h
> > > > +++ b/arch/riscv/include/asm/vector.h
> > > > @@ -21,6 +21,7 @@
> > > >   extern unsigned long riscv_v_vsize;
> > > >   int riscv_v_setup_vsize(void);
> > > > +bool insn_is_vector(u32 insn_buf);
> > > >   bool riscv_v_first_use_handler(struct pt_regs *regs);
> > > >   void kernel_vector_begin(void);
> > > >   void kernel_vector_end(void);
> > > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > > > index 060212331a03..ebacff86f134 100644
> > > > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > > > @@ -68,6 +68,12 @@ struct riscv_hwprobe {
> > > >   #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
> > > >   #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
> > > >   #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
> > > > +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
> > > > +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
> > > > +#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
> > > > +#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
> > > > +#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
> > > > +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
> > > >   /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
> > > >   /* Flags */
> > > > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > > > index b286b73e763e..ce641cc6e47a 100644
> > > > --- a/arch/riscv/kernel/sys_hwprobe.c
> > > > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > > > @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
> > > >   }
> > > >   #endif
> > > > +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
> > > > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
> > > > +{
> > > > +	int cpu;
> > > > +	u64 perf = -1ULL;
> > > > +
> > > > +	for_each_cpu(cpu, cpus) {
> > > > +		int this_perf = per_cpu(vector_misaligned_access, cpu);
> > > > +
> > > > +		if (perf == -1ULL)
> > > > +			perf = this_perf;
> > > > +
> > > > +		if (perf != this_perf) {
> > > > +			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	if (perf == -1ULL)
> > > > +		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> > > > +
> > > > +	return perf;
> > > > +}
> > > > +#else
> > > > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
> > > > +{
> > 
> > I meant to mention this in my last message!
> > 
> > The scalar version has cutouts for configs here like:
> > 
> > 	if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS))
> > 		return RISCV_HWPROBE_MISALIGNED_FAST;
> > 
> 
> Will add
> 
> > Having this functionality on vector as well would be much appreciated.
> > I don't think it's valid to assume that vector and scalar have the same
> > speed, so this would require a vector version of the RISCV_MISALIGNED
> > tree in arch/riscv/Kconfig.
> > 
> > - Charlie
> > 
> > > > +	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> > > > +}
> > > > +#endif
> > > > +
> > > >   static void hwprobe_one_pair(struct riscv_hwprobe *pair,
> > > >   			     const struct cpumask *cpus)
> > > >   {
> > > > @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
> > > >   		pair->value = hwprobe_misaligned(cpus);
> > > >   		break;
> > > > +	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
> > > > +		pair->value = hwprobe_vec_misaligned(cpus);
> > > > +		break;
> > > > +
> > > >   	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
> > > >   		pair->value = 0;
> > > >   		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
> > > > diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> > > > index 2adb7c3e4dd5..8f26c3d92230 100644
> > > > --- a/arch/riscv/kernel/traps_misaligned.c
> > > > +++ b/arch/riscv/kernel/traps_misaligned.c
> > > > @@ -16,6 +16,7 @@
> > > >   #include <asm/entry-common.h>
> > > >   #include <asm/hwprobe.h>
> > > >   #include <asm/cpufeature.h>
> > > > +#include <asm/vector.h>
> > > >   #define INSN_MATCH_LB			0x3
> > > >   #define INSN_MASK_LB			0x707f
> > > > @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
> > > >   	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
> > > > -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> > > > -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
> > > > -#endif
> > > > -
> > > >   	if (!unaligned_enabled)
> > > >   		return -1;
> > > > @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
> > > >   	if (get_insn(regs, epc, &insn))
> > > >   		return -1;
> > > > +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> > > > +	if (insn_is_vector(insn) &&
> > > > +	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
> > > > +		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
> > > > +		regs->epc = epc + INSN_LEN(insn);
> > > > +		return 0;
> 
> There is a return before scalar speed is set.

Aww I missed that.

> 
> > > > +	}
> > > > +
> > > > +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
> > > 
> > > This unconditionally sets scalar unaligned accesses even if the
> > > unaligned access is caused by vector. Scalar unaligned accesses should
> > > only be set to emulated if this function is entered from a scalar
> > > unaligned load.
> > > 
> > > The rest of this function handles how scalar unaligned accesses are
> > > emulated, and the equivalent needs to happen for vector. You need to add
> > > routines that manually load the data from the memory address into the
> > > vector register. When Clément did this for scalar, he provided a test
> > > case to help reviewers [1]. Please add onto these test cases or make
> > > your own for vector.
> 
> I wansnt planing on adding emulation in this patch. I can if needed.

Emulation is required. Right now vector unaligned accesses trap into
this function and just return without setting the value in the vector
register so the value in the vector register is incorrect but the
program doesn't know because the trap is swallowed by this function.

> 
> > > 
> > > Link: https://github.com/clementleger/unaligned_test [1]
> > > 
> > > > +#endif
> > > > +
> > > >   	regs->epc = 0;
> > > >   	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
> > > > @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
> > > >   	return misaligned_emu_detected;
> > > >   }
> > > > +#ifdef CONFIG_RISCV_ISA_V
> > > > +static void check_vector_unaligned_access(struct work_struct *unused)
> > > 
> > > Can you standardize this name with the scalar version by writing
> > > emulated in it?
> 
> We dont emulate it so that wouldn't make sence.
> 
> > > 
> > > "check_vector_unaligned_access_emulated_all_cpus"
> > > 
> > > > +{
> > > > +	int cpu = smp_processor_id();
> > > > +	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
> > > > +	unsigned long tmp_var;
> > > > +
> > > > +	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
> > > > +		return;
> > > > +
> > > > +	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> > > > +
> > > > +	local_irq_enable();
> > > > +	kernel_vector_begin();
> > > > +	__asm__ __volatile__ (
> > > > +		".balign 4\n\t"
> > > > +		".option push\n\t"
> > > > +		".option arch, +v\n\t"
> > > > +		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
> > > > +		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
> > > > +		".option pop\n\t"
> > > > +		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");
> > > 
> > > memory is being read from, but not written to, so there is no need to
> > > have a memory clobber.
> 
> fixed.
> 
> > > 
> > > > +	kernel_vector_end();
> > > > +
> > > > +	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
> > > > +		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> > > > +}
> > > > +
> > > > +bool check_vector_unaligned_access_all_cpus(void)
> > > > +{
> > > > +	int cpu;
> > > > +	bool ret = true;
> > > > +
> > > > +	for_each_online_cpu(cpu)
> > > > +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
> > > 
> > > zicclsm is not specific to vector so it can be extracted out of this
> > > vector specific function. Assuming that hardware properly reports the
> > > extension, if zicclsm is present then it is known that both vector and
> > > scalar unaligned accesses are supported.
> 
> Added so we don't need to waste cycles testing support.
> 

Yes exactly, that is why it should be added to the scalar version as
well :)

> > > > +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> > > 
> > > Please use the exising UNKNOWN terminology instead of renaming to
> > > SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
> > > accesses are supported.
> 
> Conor didnt like using UNKNOWN a proxy for "SUPPORTED"
> Having SUPPORTED is better then assuing the speed to be slow.

The HWPROBE key is about misaligned access performance. UNKNOWN means
that the performance is unknown. The scalar and vector names need to
match up. UNKNOWN was already merged and is supported by linux so if you
want to use SUPPORTED here then you need to add a scalar SUPPORTED key
that is an alias of the UNKNOWN key. I would rather keep UNKNOWN as it
is, but that's up to you.

> 
> > > 
> > > > +		else
> > > > +			ret = false;
> > > > +
> > > > +
> > > > +	if (ret)
> > > > +		return true;
> > > > +	ret = true;
> > > > +
> > > > +	schedule_on_each_cpu(check_vector_unaligned_access);
> > > > +
> > > > +	for_each_online_cpu(cpu)
> > > > +		if (per_cpu(vector_misaligned_access, cpu)
> > > > +		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
> > > > +			return false;
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +#else
> > > 
> > > If CONFIG_RISCV_ISA_V is not set, there is no value in checking if
> > > vector unaligned accesses are supported because userspace will not be
> > > allowed to use vector instructions anyway.
> 
> Oh I'm silly meant to be seting to all UNSUPPORTED.
> 
> 
> Thanks,
> Jesse Taube
> 
> > > 
> > > - Charlie
> > > 
> > > > +bool check_vector_unaligned_access_all_cpus(void)
> > > > +{
> > > > +	int cpu;
> > > > +
> > > > +	for_each_online_cpu(cpu)
> > > > +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
> > > > +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> > > > +		else
> > > > +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
> > > > +
> > > > +	return false;
> > > > +}
> > > > +#endif
> > > > +
> > > >   bool check_unaligned_access_emulated_all_cpus(void)
> > > >   {
> > > >   	int cpu;
> > > > diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> > > > index a9a6bcb02acf..92a84239beaa 100644
> > > > --- a/arch/riscv/kernel/unaligned_access_speed.c
> > > > +++ b/arch/riscv/kernel/unaligned_access_speed.c
> > > > @@ -20,6 +20,7 @@
> > > >   #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
> > > >   DEFINE_PER_CPU(long, misaligned_access_speed);
> > > > +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
> > > >   #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> > > >   static cpumask_t fast_misaligned_access;
> > > > @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
> > > >   {
> > > >   	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
> > > > +	check_vector_unaligned_access_all_cpus();
> > > > +
> > > >   	if (!all_cpus_emulated)
> > > >   		return check_unaligned_access_speed_all_cpus();
> > > > @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
> > > >   static int check_unaligned_access_all_cpus(void)
> > > >   {
> > > >   	check_unaligned_access_emulated_all_cpus();
> > > > +	check_vector_unaligned_access_all_cpus();
> > > >   	return 0;
> > > >   }
> > > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> > > > index 6727d1d3b8f2..2cceab739b2c 100644
> > > > --- a/arch/riscv/kernel/vector.c
> > > > +++ b/arch/riscv/kernel/vector.c
> > > > @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
> > > >   #endif
> > > >   }
> > > > -static bool insn_is_vector(u32 insn_buf)
> > > > +bool insn_is_vector(u32 insn_buf)
> > > >   {
> > > >   	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
> > > >   	u32 width, csr;
> > > > -- 
> > > > 2.43.0
> > > > 

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]

On Fri, Jun 07, 2024 at 02:06:27PM -0700, Charlie Jenkins wrote:
> On Fri, Jun 07, 2024 at 03:53:23PM -0400, Jesse Taube wrote:
> > On 6/6/24 19:13, Charlie Jenkins wrote:
> > > On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote:
> > > > On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:

> > > > Please use the exising UNKNOWN terminology instead of renaming to
> > > > SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
> > > > accesses are supported.
> > 
> > Conor didnt like using UNKNOWN a proxy for "SUPPORTED"

I did say this, but in the context of wanting you to actually add the
performance probing (and potentially the other infrastructure that
Charlie added for scalar).

> > Having SUPPORTED is better then assuing the speed to be slow.
> 
> The HWPROBE key is about misaligned access performance. UNKNOWN means
> that the performance is unknown.

Right. I also don't think that assuming "slow" is even problematic -
seemingly all extant hardware doesn't even support misaligned access.
But really, just whack in the probing, it shouldn't be too bad, right?

> The scalar and vector names need to
> match up.

That's definitely not the case. A different hwprobe key is allowed to
behave differently, but...

> UNKNOWN was already merged and is supported by linux so if you
> want to use SUPPORTED here then you need to add a scalar SUPPORTED key
> that is an alias of the UNKNOWN key.

...this suggestion of a scalar change I disagree with anyway, so it's
moot. Unknown should be a state that we only have internally when we
actually do not know, and not something that userspace should ever see,
unless there's a bug in the probing code IMO. Unknown gives userspace no
actionable information anyways.

> I would rather keep UNKNOWN as it
> is, but that's up to you.

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

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Charlie Jenkins]

On Fri, Jun 07, 2024 at 10:21:19PM +0100, Conor Dooley wrote:
> On Fri, Jun 07, 2024 at 02:06:27PM -0700, Charlie Jenkins wrote:
> > On Fri, Jun 07, 2024 at 03:53:23PM -0400, Jesse Taube wrote:
> > > On 6/6/24 19:13, Charlie Jenkins wrote:
> > > > On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote:
> > > > > On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
> 
> > > > > Please use the exising UNKNOWN terminology instead of renaming to
> > > > > SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
> > > > > accesses are supported.
> > > 
> > > Conor didnt like using UNKNOWN a proxy for "SUPPORTED"
> 
> I did say this, but in the context of wanting you to actually add the
> performance probing (and potentially the other infrastructure that
> Charlie added for scalar).
> 
> > > Having SUPPORTED is better then assuing the speed to be slow.
> > 
> > The HWPROBE key is about misaligned access performance. UNKNOWN means
> > that the performance is unknown.
> 
> Right. I also don't think that assuming "slow" is even problematic -
> seemingly all extant hardware doesn't even support misaligned access.
> But really, just whack in the probing, it shouldn't be too bad, right?
>

Yeah that's a good point, slow is a reasonable default.

> > The scalar and vector names need to
> > match up.
> 
> That's definitely not the case. A different hwprobe key is allowed to
> behave differently, but...

It of course can behave differently in purely technical sense, I said
"need" because it would not be a very intuitive interface to have a
different name for vector and scalar versions of the same thing.

> 
> > UNKNOWN was already merged and is supported by linux so if you
> > want to use SUPPORTED here then you need to add a scalar SUPPORTED key
> > that is an alias of the UNKNOWN key.
> 
> ...this suggestion of a scalar change I disagree with anyway, so it's
> moot. Unknown should be a state that we only have internally when we
> actually do not know, and not something that userspace should ever see,
> unless there's a bug in the probing code IMO. Unknown gives userspace no
> actionable information anyways.
> 

I agree, returning slow is probably always be more useful than unknown.

- Charlie

> > I would rather keep UNKNOWN as it
> > is, but that's up to you.

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Clément Léger]

On 06/06/2024 23:29, Charlie Jenkins wrote:
> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
>> Run a unaligned vector access to test if the system supports
>> vector unaligned access. Add the result to a new key in hwprobe.
>> This is useful for usermode to know if vector misaligned accesses are
>> supported and if they are faster or slower than equivalent byte accesses.
>>
>> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
>> ---
>>  arch/riscv/include/asm/cpufeature.h        |  2 +
>>  arch/riscv/include/asm/hwprobe.h           |  2 +-
>>  arch/riscv/include/asm/vector.h            |  1 +
>>  arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
>>  arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
>>  arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
>>  arch/riscv/kernel/unaligned_access_speed.c |  4 ++
>>  arch/riscv/kernel/vector.c                 |  2 +-
>>  8 files changed, 129 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
>> index 347805446151..a012c8490a27 100644
>> --- a/arch/riscv/include/asm/cpufeature.h
>> +++ b/arch/riscv/include/asm/cpufeature.h
>> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
>>  
>>  #if defined(CONFIG_RISCV_MISALIGNED)
>>  bool check_unaligned_access_emulated_all_cpus(void);
>> +bool check_vector_unaligned_access_all_cpus(void);
>>  void unaligned_emulation_finish(void);
>>  bool unaligned_ctl_available(void);
>>  DECLARE_PER_CPU(long, misaligned_access_speed);
>> +DECLARE_PER_CPU(long, vector_misaligned_access);
>>  #else
>>  static inline bool unaligned_ctl_available(void)
>>  {
>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
>> index 630507dff5ea..150a9877b0af 100644
>> --- a/arch/riscv/include/asm/hwprobe.h
>> +++ b/arch/riscv/include/asm/hwprobe.h
>> @@ -8,7 +8,7 @@
>>  
>>  #include <uapi/asm/hwprobe.h>
>>  
>> -#define RISCV_HWPROBE_MAX_KEY 6
>> +#define RISCV_HWPROBE_MAX_KEY 7
>>  
>>  static inline bool riscv_hwprobe_key_is_valid(__s64 key)
>>  {
>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
>> index 731dcd0ed4de..776af9b37e23 100644
>> --- a/arch/riscv/include/asm/vector.h
>> +++ b/arch/riscv/include/asm/vector.h
>> @@ -21,6 +21,7 @@
>>  
>>  extern unsigned long riscv_v_vsize;
>>  int riscv_v_setup_vsize(void);
>> +bool insn_is_vector(u32 insn_buf);
>>  bool riscv_v_first_use_handler(struct pt_regs *regs);
>>  void kernel_vector_begin(void);
>>  void kernel_vector_end(void);
>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
>> index 060212331a03..ebacff86f134 100644
>> --- a/arch/riscv/include/uapi/asm/hwprobe.h
>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
>> @@ -68,6 +68,12 @@ struct riscv_hwprobe {
>>  #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
>>  #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
>>  #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
>> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
>>  /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>>  
>>  /* Flags */
>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
>> index b286b73e763e..ce641cc6e47a 100644
>> --- a/arch/riscv/kernel/sys_hwprobe.c
>> +++ b/arch/riscv/kernel/sys_hwprobe.c
>> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
>>  }
>>  #endif
>>  
>> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>> +{
>> +	int cpu;
>> +	u64 perf = -1ULL;
>> +
>> +	for_each_cpu(cpu, cpus) {
>> +		int this_perf = per_cpu(vector_misaligned_access, cpu);
>> +
>> +		if (perf == -1ULL)
>> +			perf = this_perf;
>> +
>> +		if (perf != this_perf) {
>> +			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (perf == -1ULL)
>> +		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>> +
>> +	return perf;
>> +}
>> +#else
>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>> +{
>> +	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>> +}
>> +#endif
>> +
>>  static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>  			     const struct cpumask *cpus)
>>  {
>> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>  		pair->value = hwprobe_misaligned(cpus);
>>  		break;
>>  
>> +	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
>> +		pair->value = hwprobe_vec_misaligned(cpus);
>> +		break;
>> +
>>  	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
>>  		pair->value = 0;
>>  		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
>> index 2adb7c3e4dd5..8f26c3d92230 100644
>> --- a/arch/riscv/kernel/traps_misaligned.c
>> +++ b/arch/riscv/kernel/traps_misaligned.c
>> @@ -16,6 +16,7 @@
>>  #include <asm/entry-common.h>
>>  #include <asm/hwprobe.h>
>>  #include <asm/cpufeature.h>
>> +#include <asm/vector.h>
>>  
>>  #define INSN_MATCH_LB			0x3
>>  #define INSN_MASK_LB			0x707f
>> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
>>  
>>  	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
>>  
>> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>> -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
>> -#endif
>> -
>>  	if (!unaligned_enabled)
>>  		return -1;
>>  
>> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
>>  	if (get_insn(regs, epc, &insn))
>>  		return -1;
>>  
>> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>> +	if (insn_is_vector(insn) &&
>> +	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
>> +		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>> +		regs->epc = epc + INSN_LEN(insn);
>> +		return 0;
>> +	}
>> +
>> +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
> 
> This unconditionally sets scalar unaligned accesses even if the
> unaligned access is caused by vector. Scalar unaligned accesses should
> only be set to emulated if this function is entered from a scalar
> unaligned load.
> 
> The rest of this function handles how scalar unaligned accesses are
> emulated, and the equivalent needs to happen for vector. You need to add
> routines that manually load the data from the memory address into the
> vector register. When Clément did this for scalar, he provided a test
> case to help reviewers [1]. Please add onto these test cases or make
> your own for vector.
> 
> Link: https://github.com/clementleger/unaligned_test [1]

Hi Jesse,

I already mentionned that in a previous message and got no answer [1].
Please address all feedback before sending another version. Adding a
cover letter for such series would also be appreciated.

Thanks,

Clément

Link :https://lore.kernel.org/all/2c355b76-c768-46b1-9f37
fd1991a54bfd@rivosinc.com/ [1]

> 
>> +#endif
>> +
>>  	regs->epc = 0;
>>  
>>  	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
>> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
>>  	return misaligned_emu_detected;
>>  }
>>  
>> +#ifdef CONFIG_RISCV_ISA_V
>> +static void check_vector_unaligned_access(struct work_struct *unused)
> 
> Can you standardize this name with the scalar version by writing
> emulated in it?
> 
> "check_vector_unaligned_access_emulated_all_cpus"
> 
>> +{
>> +	int cpu = smp_processor_id();
>> +	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
>> +	unsigned long tmp_var;
>> +
>> +	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
>> +		return;
>> +
>> +	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>> +
>> +	local_irq_enable();
>> +	kernel_vector_begin();
>> +	__asm__ __volatile__ (
>> +		".balign 4\n\t"
>> +		".option push\n\t"
>> +		".option arch, +v\n\t"
>> +		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
>> +		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
>> +		".option pop\n\t"
>> +		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");
> 
> memory is being read from, but not written to, so there is no need to
> have a memory clobber.
> 
>> +	kernel_vector_end();
>> +
>> +	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
>> +		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>> +}
>> +
>> +bool check_vector_unaligned_access_all_cpus(void)
>> +{
>> +	int cpu;
>> +	bool ret = true;
>> +
>> +	for_each_online_cpu(cpu)
>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
> 
> zicclsm is not specific to vector so it can be extracted out of this
> vector specific function. Assuming that hardware properly reports the
> extension, if zicclsm is present then it is known that both vector and
> scalar unaligned accesses are supported.
> 
>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
> 
> Please use the exising UNKNOWN terminology instead of renaming to
> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
> accesses are supported.
> 
>> +		else
>> +			ret = false;
>> +
>> +
>> +	if (ret)
>> +		return true;
>> +	ret = true;
>> +
>> +	schedule_on_each_cpu(check_vector_unaligned_access);
>> +
>> +	for_each_online_cpu(cpu)
>> +		if (per_cpu(vector_misaligned_access, cpu)
>> +		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
>> +			return false;
>> +
>> +	return ret;
>> +}
>> +#else
> 
> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if
> vector unaligned accesses are supported because userspace will not be
> allowed to use vector instructions anyway.
> 
> - Charlie
> 
>> +bool check_vector_unaligned_access_all_cpus(void)
>> +{
>> +	int cpu;
>> +
>> +	for_each_online_cpu(cpu)
>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>> +		else
>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>> +
>> +	return false;
>> +}
>> +#endif
>> +
>>  bool check_unaligned_access_emulated_all_cpus(void)
>>  {
>>  	int cpu;
>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
>> index a9a6bcb02acf..92a84239beaa 100644
>> --- a/arch/riscv/kernel/unaligned_access_speed.c
>> +++ b/arch/riscv/kernel/unaligned_access_speed.c
>> @@ -20,6 +20,7 @@
>>  #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
>>  
>>  DEFINE_PER_CPU(long, misaligned_access_speed);
>> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>  
>>  #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>  static cpumask_t fast_misaligned_access;
>> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
>>  {
>>  	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>>  
>> +	check_vector_unaligned_access_all_cpus();
>> +
>>  	if (!all_cpus_emulated)
>>  		return check_unaligned_access_speed_all_cpus();
>>  
>> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
>>  static int check_unaligned_access_all_cpus(void)
>>  {
>>  	check_unaligned_access_emulated_all_cpus();
>> +	check_vector_unaligned_access_all_cpus();
>>  
>>  	return 0;
>>  }
>> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
>> index 6727d1d3b8f2..2cceab739b2c 100644
>> --- a/arch/riscv/kernel/vector.c
>> +++ b/arch/riscv/kernel/vector.c
>> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
>>  #endif
>>  }
>>  
>> -static bool insn_is_vector(u32 insn_buf)
>> +bool insn_is_vector(u32 insn_buf)
>>  {
>>  	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
>>  	u32 width, csr;
>> -- 
>> 2.43.0
>>

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[Jesse Taube]

On 6/10/24 04:23, Clément Léger wrote:
> 
> 
> On 06/06/2024 23:29, Charlie Jenkins wrote:
>> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote:
>>> Run a unaligned vector access to test if the system supports
>>> vector unaligned access. Add the result to a new key in hwprobe.
>>> This is useful for usermode to know if vector misaligned accesses are
>>> supported and if they are faster or slower than equivalent byte accesses.
>>>
>>> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
>>> ---
>>>   arch/riscv/include/asm/cpufeature.h        |  2 +
>>>   arch/riscv/include/asm/hwprobe.h           |  2 +-
>>>   arch/riscv/include/asm/vector.h            |  1 +
>>>   arch/riscv/include/uapi/asm/hwprobe.h      |  6 ++
>>>   arch/riscv/kernel/sys_hwprobe.c            | 34 +++++++++
>>>   arch/riscv/kernel/traps_misaligned.c       | 84 ++++++++++++++++++++--
>>>   arch/riscv/kernel/unaligned_access_speed.c |  4 ++
>>>   arch/riscv/kernel/vector.c                 |  2 +-
>>>   8 files changed, 129 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
>>> index 347805446151..a012c8490a27 100644
>>> --- a/arch/riscv/include/asm/cpufeature.h
>>> +++ b/arch/riscv/include/asm/cpufeature.h
>>> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void);
>>>   
>>>   #if defined(CONFIG_RISCV_MISALIGNED)
>>>   bool check_unaligned_access_emulated_all_cpus(void);
>>> +bool check_vector_unaligned_access_all_cpus(void);
>>>   void unaligned_emulation_finish(void);
>>>   bool unaligned_ctl_available(void);
>>>   DECLARE_PER_CPU(long, misaligned_access_speed);
>>> +DECLARE_PER_CPU(long, vector_misaligned_access);
>>>   #else
>>>   static inline bool unaligned_ctl_available(void)
>>>   {
>>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
>>> index 630507dff5ea..150a9877b0af 100644
>>> --- a/arch/riscv/include/asm/hwprobe.h
>>> +++ b/arch/riscv/include/asm/hwprobe.h
>>> @@ -8,7 +8,7 @@
>>>   
>>>   #include <uapi/asm/hwprobe.h>
>>>   
>>> -#define RISCV_HWPROBE_MAX_KEY 6
>>> +#define RISCV_HWPROBE_MAX_KEY 7
>>>   
>>>   static inline bool riscv_hwprobe_key_is_valid(__s64 key)
>>>   {
>>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
>>> index 731dcd0ed4de..776af9b37e23 100644
>>> --- a/arch/riscv/include/asm/vector.h
>>> +++ b/arch/riscv/include/asm/vector.h
>>> @@ -21,6 +21,7 @@
>>>   
>>>   extern unsigned long riscv_v_vsize;
>>>   int riscv_v_setup_vsize(void);
>>> +bool insn_is_vector(u32 insn_buf);
>>>   bool riscv_v_first_use_handler(struct pt_regs *regs);
>>>   void kernel_vector_begin(void);
>>>   void kernel_vector_end(void);
>>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
>>> index 060212331a03..ebacff86f134 100644
>>> --- a/arch/riscv/include/uapi/asm/hwprobe.h
>>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
>>> @@ -68,6 +68,12 @@ struct riscv_hwprobe {
>>>   #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
>>>   #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
>>>   #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
>>> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF	7
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN		0
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED		1
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_SLOW		2
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_FAST		3
>>> +#define		RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED	4
>>>   /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>>>   
>>>   /* Flags */
>>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
>>> index b286b73e763e..ce641cc6e47a 100644
>>> --- a/arch/riscv/kernel/sys_hwprobe.c
>>> +++ b/arch/riscv/kernel/sys_hwprobe.c
>>> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
>>>   }
>>>   #endif
>>>   
>>> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)
>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>>> +{
>>> +	int cpu;
>>> +	u64 perf = -1ULL;
>>> +
>>> +	for_each_cpu(cpu, cpus) {
>>> +		int this_perf = per_cpu(vector_misaligned_access, cpu);
>>> +
>>> +		if (perf == -1ULL)
>>> +			perf = this_perf;
>>> +
>>> +		if (perf != this_perf) {
>>> +			perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	if (perf == -1ULL)
>>> +		return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>> +
>>> +	return perf;
>>> +}
>>> +#else
>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus)
>>> +{
>>> +	return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>> +}
>>> +#endif
>>> +
>>>   static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>>   			     const struct cpumask *cpus)
>>>   {
>>> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>>   		pair->value = hwprobe_misaligned(cpus);
>>>   		break;
>>>   
>>> +	case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF:
>>> +		pair->value = hwprobe_vec_misaligned(cpus);
>>> +		break;
>>> +
>>>   	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
>>>   		pair->value = 0;
>>>   		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
>>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
>>> index 2adb7c3e4dd5..8f26c3d92230 100644
>>> --- a/arch/riscv/kernel/traps_misaligned.c
>>> +++ b/arch/riscv/kernel/traps_misaligned.c
>>> @@ -16,6 +16,7 @@
>>>   #include <asm/entry-common.h>
>>>   #include <asm/hwprobe.h>
>>>   #include <asm/cpufeature.h>
>>> +#include <asm/vector.h>
>>>   
>>>   #define INSN_MATCH_LB			0x3
>>>   #define INSN_MASK_LB			0x707f
>>> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs)
>>>   
>>>   	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
>>>   
>>> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>> -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
>>> -#endif
>>> -
>>>   	if (!unaligned_enabled)
>>>   		return -1;
>>>   
>>> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs)
>>>   	if (get_insn(regs, epc, &insn))
>>>   		return -1;
>>>   
>>> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>> +	if (insn_is_vector(insn) &&
>>> +	    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
>>> +		*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>>> +		regs->epc = epc + INSN_LEN(insn);
>>> +		return 0;
>>> +	}

I'm going to add


+	/* If vector instruction we don't emulate it yet */
+	if (insn_is_vector(insn)) {
+		regs->epc = epc;
+		return -1;
+	}

>>> +
>>> +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
>>
>> This unconditionally sets scalar unaligned accesses even if the
>> unaligned access is caused by vector. Scalar unaligned accesses should
>> only be set to emulated if this function is entered from a scalar
>> unaligned load.
>>

Im sorry I missunderstood what you were reffering to. The check above 
does return, but you are saying when a misaligned vector does come here.

If a misaligned vector load trap does happen when we arent checking
it will still cause a fault as `vector_misaligned_access != 
RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED`. Basicaly the return 0 above 
will only ever be posible duing check_vector_unaligned_access.

>> The rest of this function handles how scalar unaligned accesses are
>> emulated, and the equivalent needs to happen for vector.

I am hesitant to do so because I have very little knowlage of how RVV 
works. It will maintain the previous behavoir of trapping if
VEC_MISALIGNED_UNSUPPORTED.

> You need to add
>> routines that manually load the data from the memory address into the
>> vector register. When Clément did this for scalar, he provided a test
>> case to help reviewers [1]. Please add onto these test cases or make
>> your own for vector.
>>
>> Link: https://github.com/clementleger/unaligned_test [1]

I will add a test.

> 
> Hi Jesse,
> 
> I already mentionned that in a previous message and got no answer [1].
> Please address all feedback before sending another version. Adding a
> cover letter for such series would also be appreciated.

Sorry I didnt understand and thought your question was the same as
charlies.

Thanks,
Jesse Taube

> 
> Thanks,
> 
> Clément
> 
> Link :https://lore.kernel.org/all/2c355b76-c768-46b1-9f37
> fd1991a54bfd@rivosinc.com/ [1]
> 
>>
>>> +#endif
>>> +
>>>   	regs->epc = 0;
>>>   
>>>   	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
>>> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu)
>>>   	return misaligned_emu_detected;
>>>   }
>>>   
>>> +#ifdef CONFIG_RISCV_ISA_V
>>> +static void check_vector_unaligned_access(struct work_struct *unused)
>>
>> Can you standardize this name with the scalar version by writing
>> emulated in it?
>>
>> "check_vector_unaligned_access_emulated_all_cpus"
>>
>>> +{
>>> +	int cpu = smp_processor_id();
>>> +	long *mas_ptr = this_cpu_ptr(&vector_misaligned_access);
>>> +	unsigned long tmp_var;
>>> +
>>> +	if (!riscv_isa_extension_available(hart_isa[cpu].isa, v))
>>> +		return;
>>> +
>>> +	*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>> +
>>> +	local_irq_enable();
>>> +	kernel_vector_begin();
>>> +	__asm__ __volatile__ (
>>> +		".balign 4\n\t"
>>> +		".option push\n\t"
>>> +		".option arch, +v\n\t"
>>> +		"       vsetivli zero, 1, e16, m1, ta, ma\n\t"	// Vectors of 16b
>>> +		"	vle16.v v0, (%[ptr])\n\t"		// Load bytes
>>> +		".option pop\n\t"
>>> +		: : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory");
>>
>> memory is being read from, but not written to, so there is no need to
>> have a memory clobber.
>>
>>> +	kernel_vector_end();
>>> +
>>> +	if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN)
>>> +		*mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>> +}
>>> +
>>> +bool check_vector_unaligned_access_all_cpus(void)
>>> +{
>>> +	int cpu;
>>> +	bool ret = true;
>>> +
>>> +	for_each_online_cpu(cpu)
>>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
>>
>> zicclsm is not specific to vector so it can be extracted out of this
>> vector specific function. Assuming that hardware properly reports the
>> extension, if zicclsm is present then it is known that both vector and
>> scalar unaligned accesses are supported.
>>
>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>
>> Please use the exising UNKNOWN terminology instead of renaming to
>> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned
>> accesses are supported.
>>
>>> +		else
>>> +			ret = false;
>>> +
>>> +
>>> +	if (ret)
>>> +		return true;
>>> +	ret = true;
>>> +
>>> +	schedule_on_each_cpu(check_vector_unaligned_access);
>>> +
>>> +	for_each_online_cpu(cpu)
>>> +		if (per_cpu(vector_misaligned_access, cpu)
>>> +		    != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
>>> +			return false;
>>> +
>>> +	return ret;
>>> +}
>>> +#else
>>
>> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if
>> vector unaligned accesses are supported because userspace will not be
>> allowed to use vector instructions anyway.
>>
>> - Charlie
>>
>>> +bool check_vector_unaligned_access_all_cpus(void)
>>> +{
>>> +	int cpu;
>>> +
>>> +	for_each_online_cpu(cpu)
>>> +		if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM))
>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED;
>>> +		else
>>> +			per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
>>> +
>>> +	return false;
>>> +}
>>> +#endif
>>> +
>>>   bool check_unaligned_access_emulated_all_cpus(void)
>>>   {
>>>   	int cpu;
>>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
>>> index a9a6bcb02acf..92a84239beaa 100644
>>> --- a/arch/riscv/kernel/unaligned_access_speed.c
>>> +++ b/arch/riscv/kernel/unaligned_access_speed.c
>>> @@ -20,6 +20,7 @@
>>>   #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80)
>>>   
>>>   DEFINE_PER_CPU(long, misaligned_access_speed);
>>> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN;
>>>   
>>>   #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
>>>   static cpumask_t fast_misaligned_access;
>>> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void)
>>>   {
>>>   	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>>>   
>>> +	check_vector_unaligned_access_all_cpus();
>>> +
>>>   	if (!all_cpus_emulated)
>>>   		return check_unaligned_access_speed_all_cpus();
>>>   
>>> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void)
>>>   static int check_unaligned_access_all_cpus(void)
>>>   {
>>>   	check_unaligned_access_emulated_all_cpus();
>>> +	check_vector_unaligned_access_all_cpus();
>>>   
>>>   	return 0;
>>>   }
>>> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
>>> index 6727d1d3b8f2..2cceab739b2c 100644
>>> --- a/arch/riscv/kernel/vector.c
>>> +++ b/arch/riscv/kernel/vector.c
>>> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void)
>>>   #endif
>>>   }
>>>   
>>> -static bool insn_is_vector(u32 insn_buf)
>>> +bool insn_is_vector(u32 insn_buf)
>>>   {
>>>   	u32 opcode = insn_buf & __INSN_OPCODE_MASK;
>>>   	u32 width, csr;
>>> -- 
>>> 2.43.0
>>>

Re: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.

[kernel test robot]

Hi Jesse,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.9]
[cannot apply to akpm-mm/mm-everything linus/master v6.10-rc2 v6.10-rc1 next-20240606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jesse-Taube/RISC-V-Detect-unaligned-vector-accesses-supported/20240607-023434
base:   v6.9
patch link:    https://lore.kernel.org/r/20240606183215.416829-2-jesse%40rivosinc.com
patch subject: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported.
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20240607/202406070508.6UJUx2rO-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070508.6UJUx2rO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406070508.6UJUx2rO-lkp@intel.com/

All errors (new ones prefixed by >>):

   arch/riscv/kernel/traps_misaligned.c: In function 'handle_misaligned_load':
>> arch/riscv/kernel/traps_misaligned.c:427:13: error: implicit declaration of function 'insn_is_vector' [-Werror=implicit-function-declaration]
     427 |         if (insn_is_vector(insn) &&
         |             ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/insn_is_vector +427 arch/riscv/kernel/traps_misaligned.c

   406	
   407	int handle_misaligned_load(struct pt_regs *regs)
   408	{
   409		union reg_data val;
   410		unsigned long epc = regs->epc;
   411		unsigned long insn;
   412		unsigned long addr = regs->badaddr;
   413		int i, fp = 0, shift = 0, len = 0;
   414	
   415		perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
   416	
   417		if (!unaligned_enabled)
   418			return -1;
   419	
   420		if (user_mode(regs) && (current->thread.align_ctl & PR_UNALIGN_SIGBUS))
   421			return -1;
   422	
   423		if (get_insn(regs, epc, &insn))
   424			return -1;
   425	
   426	#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
 > 427		if (insn_is_vector(insn) &&
   428		    *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) {
   429			*this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED;
   430			regs->epc = epc + INSN_LEN(insn);
   431			return 0;
   432		}
   433	
   434		*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
   435	#endif
   436	
   437		regs->epc = 0;
   438	
   439		if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
   440			len = 4;
   441			shift = 8 * (sizeof(unsigned long) - len);
   442	#if defined(CONFIG_64BIT)
   443		} else if ((insn & INSN_MASK_LD) == INSN_MATCH_LD) {
   444			len = 8;
   445			shift = 8 * (sizeof(unsigned long) - len);
   446		} else if ((insn & INSN_MASK_LWU) == INSN_MATCH_LWU) {
   447			len = 4;
   448	#endif
   449		} else if ((insn & INSN_MASK_FLD) == INSN_MATCH_FLD) {
   450			fp = 1;
   451			len = 8;
   452		} else if ((insn & INSN_MASK_FLW) == INSN_MATCH_FLW) {
   453			fp = 1;
   454			len = 4;
   455		} else if ((insn & INSN_MASK_LH) == INSN_MATCH_LH) {
   456			len = 2;
   457			shift = 8 * (sizeof(unsigned long) - len);
   458		} else if ((insn & INSN_MASK_LHU) == INSN_MATCH_LHU) {
   459			len = 2;
   460	#if defined(CONFIG_64BIT)
   461		} else if ((insn & INSN_MASK_C_LD) == INSN_MATCH_C_LD) {
   462			len = 8;
   463			shift = 8 * (sizeof(unsigned long) - len);
   464			insn = RVC_RS2S(insn) << SH_RD;
   465		} else if ((insn & INSN_MASK_C_LDSP) == INSN_MATCH_C_LDSP &&
   466			   ((insn >> SH_RD) & 0x1f)) {
   467			len = 8;
   468			shift = 8 * (sizeof(unsigned long) - len);
   469	#endif
   470		} else if ((insn & INSN_MASK_C_LW) == INSN_MATCH_C_LW) {
   471			len = 4;
   472			shift = 8 * (sizeof(unsigned long) - len);
   473			insn = RVC_RS2S(insn) << SH_RD;
   474		} else if ((insn & INSN_MASK_C_LWSP) == INSN_MATCH_C_LWSP &&
   475			   ((insn >> SH_RD) & 0x1f)) {
   476			len = 4;
   477			shift = 8 * (sizeof(unsigned long) - len);
   478		} else if ((insn & INSN_MASK_C_FLD) == INSN_MATCH_C_FLD) {
   479			fp = 1;
   480			len = 8;
   481			insn = RVC_RS2S(insn) << SH_RD;
   482		} else if ((insn & INSN_MASK_C_FLDSP) == INSN_MATCH_C_FLDSP) {
   483			fp = 1;
   484			len = 8;
   485	#if defined(CONFIG_32BIT)
   486		} else if ((insn & INSN_MASK_C_FLW) == INSN_MATCH_C_FLW) {
   487			fp = 1;
   488			len = 4;
   489			insn = RVC_RS2S(insn) << SH_RD;
   490		} else if ((insn & INSN_MASK_C_FLWSP) == INSN_MATCH_C_FLWSP) {
   491			fp = 1;
   492			len = 4;
   493	#endif
   494		} else {
   495			regs->epc = epc;
   496			return -1;
   497		}
   498	
   499		if (!IS_ENABLED(CONFIG_FPU) && fp)
   500			return -EOPNOTSUPP;
   501	
   502		val.data_u64 = 0;
   503		for (i = 0; i < len; i++) {
   504			if (load_u8(regs, (void *)(addr + i), &val.data_bytes[i]))
   505				return -1;
   506		}
   507	
   508		if (!fp)
   509			SET_RD(insn, regs, val.data_ulong << shift >> shift);
   510		else if (len == 8)
   511			set_f64_rd(insn, regs, val.data_u64);
   512		else
   513			set_f32_rd(insn, regs, val.data_ulong);
   514	
   515		regs->epc = epc + INSN_LEN(insn);
   516	
   517		return 0;
   518	}
   519	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 3/3] RISC-V: Report vector unaligned accesse speed hwprobe

[Jesse Taube]

Detect if vector misaligned accesses are faster or slower than
equivalent vector byte accesses. This is useful for usermode to know
whether vector byte accesses or vector misaligned accesses have a better
bandwidth for operations like memcpy.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/kernel/Makefile                 |   3 +
 arch/riscv/kernel/copy-unaligned.h         |   5 +
 arch/riscv/kernel/unaligned_access_speed.c | 127 ++++++++++++++++++++-
 arch/riscv/kernel/vec-copy-unaligned.S     |  58 ++++++++++
 4 files changed, 192 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/kernel/vec-copy-unaligned.S

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 81d94a8ee10f..61cec0688559 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -65,6 +65,9 @@ obj-$(CONFIG_MMU) += vdso.o vdso/
 obj-$(CONFIG_RISCV_MISALIGNED)	+= traps_misaligned.o
 obj-$(CONFIG_RISCV_MISALIGNED)	+= unaligned_access_speed.o
 obj-$(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)	+= copy-unaligned.o
+ifeq ($(CONFIG_RISCV_ISA_V), y)
+obj-$(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)	+= vec-copy-unaligned.o
+endif
 
 obj-$(CONFIG_FPU)		+= fpu.o
 obj-$(CONFIG_RISCV_ISA_V)	+= vector.o
diff --git a/arch/riscv/kernel/copy-unaligned.h b/arch/riscv/kernel/copy-unaligned.h
index e3d70d35b708..88be070085cb 100644
--- a/arch/riscv/kernel/copy-unaligned.h
+++ b/arch/riscv/kernel/copy-unaligned.h
@@ -10,4 +10,9 @@
 void __riscv_copy_words_unaligned(void *dst, const void *src, size_t size);
 void __riscv_copy_bytes_unaligned(void *dst, const void *src, size_t size);
 
+#ifdef CONFIG_RISCV_ISA_V
+void __riscv_copy_vec_words_unaligned(void *dst, const void *src, size_t size);
+void __riscv_copy_vec_bytes_unaligned(void *dst, const void *src, size_t size);
+#endif
+
 #endif /* __RISCV_KERNEL_COPY_UNALIGNED_H */
diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
index 92a84239beaa..4e6f753b659a 100644
--- a/arch/riscv/kernel/unaligned_access_speed.c
+++ b/arch/riscv/kernel/unaligned_access_speed.c
@@ -8,9 +8,11 @@
 #include <linux/jump_label.h>
 #include <linux/mm.h>
 #include <linux/smp.h>
+#include <linux/kthread.h>
 #include <linux/types.h>
 #include <asm/cpufeature.h>
 #include <asm/hwprobe.h>
+#include <asm/vector.h>
 
 #include "copy-unaligned.h"
 
@@ -128,6 +130,107 @@ static void check_unaligned_access_nonboot_cpu(void *param)
 		check_unaligned_access(pages[cpu]);
 }
 
+#ifdef CONFIG_RISCV_ISA_V
+static void check_vector_unaligned_access(struct work_struct *unused)
+{
+	int cpu = smp_processor_id();
+	u64 start_cycles, end_cycles;
+	u64 word_cycles;
+	u64 byte_cycles;
+	int ratio;
+	unsigned long start_jiffies, now;
+	struct page *page;
+	void *dst;
+	void *src;
+	long speed = RISCV_HWPROBE_VEC_MISALIGNED_SLOW;
+
+	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
+		return;
+
+	page = alloc_pages(GFP_KERNEL, MISALIGNED_BUFFER_ORDER);
+	if (!page) {
+		pr_warn("Allocation failure, not measuring vector misaligned performance\n");
+		return;
+	}
+
+	/* Make an unaligned destination buffer. */
+	dst = (void *)((unsigned long)page_address(page) | 0x1);
+	/* Unalign src as well, but differently (off by 1 + 2 = 3). */
+	src = dst + (MISALIGNED_BUFFER_SIZE / 2);
+	src += 2;
+	word_cycles = -1ULL;
+
+	/* Do a warmup. */
+	local_irq_enable();
+	kernel_vector_begin();
+	__riscv_copy_vec_words_unaligned(dst, src, MISALIGNED_COPY_SIZE);
+
+	start_jiffies = jiffies;
+	while ((now = jiffies) == start_jiffies)
+		cpu_relax();
+
+	/*
+	 * For a fixed amount of time, repeatedly try the function, and take
+	 * the best time in cycles as the measurement.
+	 */
+	while (time_before(jiffies, now + (1 << MISALIGNED_ACCESS_JIFFIES_LG2))) {
+		start_cycles = get_cycles64();
+		/* Ensure the CSR read can't reorder WRT to the copy. */
+		mb();
+		__riscv_copy_vec_words_unaligned(dst, src, MISALIGNED_COPY_SIZE);
+		/* Ensure the copy ends before the end time is snapped. */
+		mb();
+		end_cycles = get_cycles64();
+		if ((end_cycles - start_cycles) < word_cycles)
+			word_cycles = end_cycles - start_cycles;
+	}
+
+	byte_cycles = -1ULL;
+	__riscv_copy_vec_bytes_unaligned(dst, src, MISALIGNED_COPY_SIZE);
+	start_jiffies = jiffies;
+	while ((now = jiffies) == start_jiffies)
+		cpu_relax();
+
+	while (time_before(jiffies, now + (1 << MISALIGNED_ACCESS_JIFFIES_LG2))) {
+		start_cycles = get_cycles64();
+		mb();
+		__riscv_copy_vec_bytes_unaligned(dst, src, MISALIGNED_COPY_SIZE);
+		mb();
+		end_cycles = get_cycles64();
+		if ((end_cycles - start_cycles) < byte_cycles)
+			byte_cycles = end_cycles - start_cycles;
+	}
+
+	kernel_vector_end();
+
+	/* Don't divide by zero. */
+	if (!word_cycles || !byte_cycles) {
+		pr_warn("cpu%d: rdtime lacks granularity needed to measure unaligned vector access speed\n",
+			cpu);
+
+		return;
+	}
+
+	if (word_cycles < byte_cycles)
+		speed = RISCV_HWPROBE_VEC_MISALIGNED_FAST;
+
+	ratio = div_u64((byte_cycles * 100), word_cycles);
+	pr_info("cpu%d: Ratio of vector byte access time to vector unaligned word access is %d.%02d, unaligned accesses are %s\n",
+		cpu,
+		ratio / 100,
+		ratio % 100,
+		(speed ==  RISCV_HWPROBE_VEC_MISALIGNED_FAST) ? "fast" : "slow");
+
+	per_cpu(vector_misaligned_access, cpu) = speed;
+}
+
+static int riscv_online_cpu_vec(unsigned int cpu)
+{
+	check_vector_unaligned_access(NULL);
+	return 0;
+}
+#endif
+
 DEFINE_STATIC_KEY_FALSE(fast_unaligned_access_speed_key);
 
 static void modify_unaligned_access_branches(cpumask_t *mask, int weight)
@@ -261,11 +364,33 @@ static int check_unaligned_access_speed_all_cpus(void)
 	return 0;
 }
 
+/* Measure unaligned access speed on all CPUs present at boot in parallel. */
+static int vec_check_unaligned_access_speed_all_cpus(void *unused)
+{
+	schedule_on_each_cpu(check_vector_unaligned_access);
+
+	/*
+	 * Setup hotplug callbacks for any new CPUs that come online or go
+	 * offline.
+	 */
+	cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "riscv:online",
+				  riscv_online_cpu_vec, NULL);
+
+	return 0;
+}
+
 static int check_unaligned_access_all_cpus(void)
 {
 	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
 
-	check_vector_unaligned_access_all_cpus();
+#ifdef CONFIG_RISCV_ISA_V
+	bool all_cpus_vec_supported = check_vector_unaligned_access_all_cpus();
+
+	if (all_cpus_vec_supported) {
+		kthread_run(vec_check_unaligned_access_speed_all_cpus,
+			    NULL, "thebestthread");
+	}
+#endif
 
 	if (!all_cpus_emulated)
 		return check_unaligned_access_speed_all_cpus();
diff --git a/arch/riscv/kernel/vec-copy-unaligned.S b/arch/riscv/kernel/vec-copy-unaligned.S
new file mode 100644
index 000000000000..11522ec8f0a8
--- /dev/null
+++ b/arch/riscv/kernel/vec-copy-unaligned.S
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2024 Rivos Inc. */
+
+#include <linux/linkage.h>
+#include <asm/asm.h>
+#include <linux/args.h>
+
+	.text
+
+#define WORD_EEW 64
+
+#define WORD_SEW CONCATENATE(e, WORD_EEW)
+#define VEC_L CONCATENATE(vle, WORD_EEW).v
+#define VEC_S CONCATENATE(vle, WORD_EEW).v
+
+/* void __riscv_copy_vec_words_unaligned(void *, const void *, size_t) */
+/* Performs a memcpy without aligning buffers, using word loads and stores. */
+/* Note: The size is truncated to a multiple of WORD_EEW */
+SYM_FUNC_START(__riscv_copy_vec_words_unaligned)
+	andi  a4, a2, ~(WORD_EEW-1)
+	beqz  a4, 2f
+	add   a3, a1, a4
+	.option push
+	.option arch, +v
+1:
+	vsetivli t0, 8, WORD_SEW, m8, ta, ma
+	VEC_L v0, (a1)
+	VEC_S v0, (a0)
+	addi  a0, a0, WORD_EEW
+	addi  a1, a1, WORD_EEW
+	bltu  a1, a3, 1b
+
+2:
+	.option pop
+	ret
+SYM_FUNC_END(__riscv_copy_vec_words_unaligned)
+
+/* void __riscv_copy_vec_bytes_unaligned(void *, const void *, size_t) */
+/* Performs a memcpy without aligning buffers, using only byte accesses. */
+/* Note: The size is truncated to a multiple of 8 */
+SYM_FUNC_START(__riscv_copy_vec_bytes_unaligned)
+	andi a4, a2, ~(8-1)
+	beqz a4, 2f
+	add  a3, a1, a4
+	.option push
+	.option arch, +v
+1:
+	vsetivli t0, 8, e8, m8, ta, ma
+	vle8.v v0, (a1)
+	vse8.v v0, (a0)
+	addi a0, a0, 8
+	addi a1, a1, 8
+	bltu a1, a3, 1b
+
+2:
+	.option pop
+	ret
+SYM_FUNC_END(__riscv_copy_vec_bytes_unaligned)
-- 
2.43.0

Re: [PATCH 3/3] RISC-V: Report vector unaligned accesse speed hwprobe

[kernel test robot]

Hi Jesse,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.9]
[cannot apply to akpm-mm/mm-everything linus/master v6.10-rc2 v6.10-rc1 next-20240606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jesse-Taube/RISC-V-Detect-unaligned-vector-accesses-supported/20240607-023434
base:   v6.9
patch link:    https://lore.kernel.org/r/20240606183215.416829-3-jesse%40rivosinc.com
patch subject: [PATCH 3/3] RISC-V: Report vector unaligned accesse speed hwprobe
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20240607/202406070612.Vj1dDiqM-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070612.Vj1dDiqM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406070612.Vj1dDiqM-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   arch/riscv/kernel/unaligned_access_speed.c: In function 'vec_check_unaligned_access_speed_all_cpus':
>> arch/riscv/kernel/unaligned_access_speed.c:370:30: error: 'check_vector_unaligned_access' undeclared (first use in this function); did you mean 'check_unaligned_access'?
     370 |         schedule_on_each_cpu(check_vector_unaligned_access);
         |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                              check_unaligned_access
   arch/riscv/kernel/unaligned_access_speed.c:370:30: note: each undeclared identifier is reported only once for each function it appears in
>> arch/riscv/kernel/unaligned_access_speed.c:377:35: error: 'riscv_online_cpu_vec' undeclared (first use in this function); did you mean 'riscv_online_cpu'?
     377 |                                   riscv_online_cpu_vec, NULL);
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   riscv_online_cpu
   arch/riscv/kernel/unaligned_access_speed.c: At top level:
>> arch/riscv/kernel/unaligned_access_speed.c:368:12: warning: 'vec_check_unaligned_access_speed_all_cpus' defined but not used [-Wunused-function]
     368 | static int vec_check_unaligned_access_speed_all_cpus(void *unused)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +370 arch/riscv/kernel/unaligned_access_speed.c

   366	
   367	/* Measure unaligned access speed on all CPUs present at boot in parallel. */
 > 368	static int vec_check_unaligned_access_speed_all_cpus(void *unused)
   369	{
 > 370		schedule_on_each_cpu(check_vector_unaligned_access);
   371	
   372		/*
   373		 * Setup hotplug callbacks for any new CPUs that come online or go
   374		 * offline.
   375		 */
   376		cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "riscv:online",
 > 377					  riscv_online_cpu_vec, NULL);
   378	
   379		return 0;
   380	}
   381	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/3] RISC-V: Report vector unaligned accesse speed hwprobe

[Charlie Jenkins]

On Thu, Jun 06, 2024 at 02:32:15PM -0400, Jesse Taube wrote:
> Detect if vector misaligned accesses are faster or slower than
> equivalent vector byte accesses. This is useful for usermode to know
> whether vector byte accesses or vector misaligned accesses have a better
> bandwidth for operations like memcpy.
> 
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
>  arch/riscv/kernel/Makefile                 |   3 +
>  arch/riscv/kernel/copy-unaligned.h         |   5 +
>  arch/riscv/kernel/unaligned_access_speed.c | 127 ++++++++++++++++++++-
>  arch/riscv/kernel/vec-copy-unaligned.S     |  58 ++++++++++
>  4 files changed, 192 insertions(+), 1 deletion(-)
>  create mode 100644 arch/riscv/kernel/vec-copy-unaligned.S
> 
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 81d94a8ee10f..61cec0688559 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -65,6 +65,9 @@ obj-$(CONFIG_MMU) += vdso.o vdso/
>  obj-$(CONFIG_RISCV_MISALIGNED)	+= traps_misaligned.o
>  obj-$(CONFIG_RISCV_MISALIGNED)	+= unaligned_access_speed.o
>  obj-$(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)	+= copy-unaligned.o
> +ifeq ($(CONFIG_RISCV_ISA_V), y)
> +obj-$(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS)	+= vec-copy-unaligned.o
> +endif
>  
>  obj-$(CONFIG_FPU)		+= fpu.o
>  obj-$(CONFIG_RISCV_ISA_V)	+= vector.o
> diff --git a/arch/riscv/kernel/copy-unaligned.h b/arch/riscv/kernel/copy-unaligned.h
> index e3d70d35b708..88be070085cb 100644
> --- a/arch/riscv/kernel/copy-unaligned.h
> +++ b/arch/riscv/kernel/copy-unaligned.h
> @@ -10,4 +10,9 @@
>  void __riscv_copy_words_unaligned(void *dst, const void *src, size_t size);
>  void __riscv_copy_bytes_unaligned(void *dst, const void *src, size_t size);
>  
> +#ifdef CONFIG_RISCV_ISA_V
> +void __riscv_copy_vec_words_unaligned(void *dst, const void *src, size_t size);
> +void __riscv_copy_vec_bytes_unaligned(void *dst, const void *src, size_t size);
> +#endif
> +
>  #endif /* __RISCV_KERNEL_COPY_UNALIGNED_H */
> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> index 92a84239beaa..4e6f753b659a 100644
> --- a/arch/riscv/kernel/unaligned_access_speed.c
> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> @@ -8,9 +8,11 @@
>  #include <linux/jump_label.h>
>  #include <linux/mm.h>
>  #include <linux/smp.h>
> +#include <linux/kthread.h>
>  #include <linux/types.h>
>  #include <asm/cpufeature.h>
>  #include <asm/hwprobe.h>
> +#include <asm/vector.h>
>  
>  #include "copy-unaligned.h"
>  
> @@ -128,6 +130,107 @@ static void check_unaligned_access_nonboot_cpu(void *param)
>  		check_unaligned_access(pages[cpu]);
>  }
>  
> +#ifdef CONFIG_RISCV_ISA_V
> +static void check_vector_unaligned_access(struct work_struct *unused)
> +{
> +	int cpu = smp_processor_id();
> +	u64 start_cycles, end_cycles;
> +	u64 word_cycles;
> +	u64 byte_cycles;
> +	int ratio;
> +	unsigned long start_jiffies, now;
> +	struct page *page;
> +	void *dst;
> +	void *src;
> +	long speed = RISCV_HWPROBE_VEC_MISALIGNED_SLOW;
> +
> +	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED)
> +		return;
> +
> +	page = alloc_pages(GFP_KERNEL, MISALIGNED_BUFFER_ORDER);
> +	if (!page) {
> +		pr_warn("Allocation failure, not measuring vector misaligned performance\n");
> +		return;
> +	}
> +
> +	/* Make an unaligned destination buffer. */
> +	dst = (void *)((unsigned long)page_address(page) | 0x1);
> +	/* Unalign src as well, but differently (off by 1 + 2 = 3). */
> +	src = dst + (MISALIGNED_BUFFER_SIZE / 2);
> +	src += 2;
> +	word_cycles = -1ULL;
> +
> +	/* Do a warmup. */
> +	local_irq_enable();
> +	kernel_vector_begin();
> +	__riscv_copy_vec_words_unaligned(dst, src, MISALIGNED_COPY_SIZE);
> +
> +	start_jiffies = jiffies;
> +	while ((now = jiffies) == start_jiffies)
> +		cpu_relax();
> +
> +	/*
> +	 * For a fixed amount of time, repeatedly try the function, and take
> +	 * the best time in cycles as the measurement.
> +	 */
> +	while (time_before(jiffies, now + (1 << MISALIGNED_ACCESS_JIFFIES_LG2))) {
> +		start_cycles = get_cycles64();
> +		/* Ensure the CSR read can't reorder WRT to the copy. */
> +		mb();
> +		__riscv_copy_vec_words_unaligned(dst, src, MISALIGNED_COPY_SIZE);
> +		/* Ensure the copy ends before the end time is snapped. */
> +		mb();
> +		end_cycles = get_cycles64();
> +		if ((end_cycles - start_cycles) < word_cycles)
> +			word_cycles = end_cycles - start_cycles;
> +	}
> +
> +	byte_cycles = -1ULL;
> +	__riscv_copy_vec_bytes_unaligned(dst, src, MISALIGNED_COPY_SIZE);
> +	start_jiffies = jiffies;
> +	while ((now = jiffies) == start_jiffies)
> +		cpu_relax();
> +
> +	while (time_before(jiffies, now + (1 << MISALIGNED_ACCESS_JIFFIES_LG2))) {
> +		start_cycles = get_cycles64();
> +		mb();
> +		__riscv_copy_vec_bytes_unaligned(dst, src, MISALIGNED_COPY_SIZE);
> +		mb();
> +		end_cycles = get_cycles64();
> +		if ((end_cycles - start_cycles) < byte_cycles)
> +			byte_cycles = end_cycles - start_cycles;
> +	}
> +
> +	kernel_vector_end();
> +
> +	/* Don't divide by zero. */
> +	if (!word_cycles || !byte_cycles) {
> +		pr_warn("cpu%d: rdtime lacks granularity needed to measure unaligned vector access speed\n",
> +			cpu);
> +
> +		return;
> +	}
> +
> +	if (word_cycles < byte_cycles)
> +		speed = RISCV_HWPROBE_VEC_MISALIGNED_FAST;
> +
> +	ratio = div_u64((byte_cycles * 100), word_cycles);
> +	pr_info("cpu%d: Ratio of vector byte access time to vector unaligned word access is %d.%02d, unaligned accesses are %s\n",
> +		cpu,
> +		ratio / 100,
> +		ratio % 100,
> +		(speed ==  RISCV_HWPROBE_VEC_MISALIGNED_FAST) ? "fast" : "slow");
> +
> +	per_cpu(vector_misaligned_access, cpu) = speed;
> +}
> +
> +static int riscv_online_cpu_vec(unsigned int cpu)
> +{
> +	check_vector_unaligned_access(NULL);
> +	return 0;
> +}
> +#endif
> +
>  DEFINE_STATIC_KEY_FALSE(fast_unaligned_access_speed_key);
>  
>  static void modify_unaligned_access_branches(cpumask_t *mask, int weight)
> @@ -261,11 +364,33 @@ static int check_unaligned_access_speed_all_cpus(void)
>  	return 0;
>  }
>  
> +/* Measure unaligned access speed on all CPUs present at boot in parallel. */
> +static int vec_check_unaligned_access_speed_all_cpus(void *unused)
> +{
> +	schedule_on_each_cpu(check_vector_unaligned_access);
> +
> +	/*
> +	 * Setup hotplug callbacks for any new CPUs that come online or go
> +	 * offline.
> +	 */
> +	cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "riscv:online",
> +				  riscv_online_cpu_vec, NULL);
> +
> +	return 0;
> +}
> +
>  static int check_unaligned_access_all_cpus(void)
>  {
>  	bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>  
> -	check_vector_unaligned_access_all_cpus();
> +#ifdef CONFIG_RISCV_ISA_V
> +	bool all_cpus_vec_supported = check_vector_unaligned_access_all_cpus();
> +
> +	if (all_cpus_vec_supported) {
> +		kthread_run(vec_check_unaligned_access_speed_all_cpus,

I think it might be better if this is combined with
check_unaligned_access_speed_all_cpus() by leveraging the function
check_unaligned_access_nonboot_cpu(). The idea behind that is that right
now we have both the vector unaligned accesses and the scalar unaligned
access tests being kicked off onto each cpu separately which requires 2
IPIs per hart, but if we run them back to back on a given hart the IPI
only needs to happen once per hart. Having the methods of checking
vector unaligned access and scalar unaligned access be standardized
would be nice as well.

The scalar misaligned access also keeps one cpu back to keep consistency
in timing so I would imagine that would be important to do here as well.

- Charlie

> +			    NULL, "thebestthread");
> +	}
> +#endif
>  
>  	if (!all_cpus_emulated)
>  		return check_unaligned_access_speed_all_cpus();
> diff --git a/arch/riscv/kernel/vec-copy-unaligned.S b/arch/riscv/kernel/vec-copy-unaligned.S
> new file mode 100644
> index 000000000000..11522ec8f0a8
> --- /dev/null
> +++ b/arch/riscv/kernel/vec-copy-unaligned.S
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2024 Rivos Inc. */
> +
> +#include <linux/linkage.h>
> +#include <asm/asm.h>
> +#include <linux/args.h>
> +
> +	.text
> +
> +#define WORD_EEW 64
> +
> +#define WORD_SEW CONCATENATE(e, WORD_EEW)
> +#define VEC_L CONCATENATE(vle, WORD_EEW).v
> +#define VEC_S CONCATENATE(vle, WORD_EEW).v
> +
> +/* void __riscv_copy_vec_words_unaligned(void *, const void *, size_t) */
> +/* Performs a memcpy without aligning buffers, using word loads and stores. */
> +/* Note: The size is truncated to a multiple of WORD_EEW */
> +SYM_FUNC_START(__riscv_copy_vec_words_unaligned)
> +	andi  a4, a2, ~(WORD_EEW-1)
> +	beqz  a4, 2f
> +	add   a3, a1, a4
> +	.option push
> +	.option arch, +v
> +1:
> +	vsetivli t0, 8, WORD_SEW, m8, ta, ma
> +	VEC_L v0, (a1)
> +	VEC_S v0, (a0)
> +	addi  a0, a0, WORD_EEW
> +	addi  a1, a1, WORD_EEW
> +	bltu  a1, a3, 1b
> +
> +2:
> +	.option pop
> +	ret
> +SYM_FUNC_END(__riscv_copy_vec_words_unaligned)
> +
> +/* void __riscv_copy_vec_bytes_unaligned(void *, const void *, size_t) */
> +/* Performs a memcpy without aligning buffers, using only byte accesses. */
> +/* Note: The size is truncated to a multiple of 8 */
> +SYM_FUNC_START(__riscv_copy_vec_bytes_unaligned)
> +	andi a4, a2, ~(8-1)
> +	beqz a4, 2f
> +	add  a3, a1, a4
> +	.option push
> +	.option arch, +v
> +1:
> +	vsetivli t0, 8, e8, m8, ta, ma
> +	vle8.v v0, (a1)
> +	vse8.v v0, (a0)
> +	addi a0, a0, 8
> +	addi a1, a1, 8
> +	bltu a1, a3, 1b
> +
> +2:
> +	.option pop
> +	ret
> +SYM_FUNC_END(__riscv_copy_vec_bytes_unaligned)
> -- 
> 2.43.0
> 

Re: [PATCH 1/3] RISC-V: Add Zicclsm to cpufeature and hwprobe

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3678 bytes --]

On Thu, Jun 06, 2024 at 02:32:13PM -0400, Jesse Taube wrote:
> > Zicclsm Misaligned loads and stores to main memory regions with both
> > the cacheability and coherence PMAs must be supported.
> > Note:
> > This introduces a new extension name for this feature.
> > This requires misaligned support for all regular load and store
> > instructions (including scalar and vector) but not AMOs or other
> > specialized forms of memory access. Even though mandated, misaligned
> > loads and stores might execute extremely slowly. Standard software
> > distributions should assume their existence only for correctness,
> > not for performance.
> 
> Detecing zicclsm allows the kernel to report if the
> hardware supports misaligned accesses even if support wasn't probed.
> 
> This is useful for usermode to know if vector misaligned accesses are
> supported.
> 
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
>  arch/riscv/include/asm/hwcap.h        | 1 +
>  arch/riscv/include/uapi/asm/hwprobe.h | 1 +
>  arch/riscv/kernel/cpufeature.c        | 1 +
>  arch/riscv/kernel/sys_hwprobe.c       | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e17d0078a651..8c0d0b555a8e 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,7 @@
>  #define RISCV_ISA_EXT_ZTSO		72
>  #define RISCV_ISA_EXT_ZACAS		73
>  #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_ZICCLSM		75
>  
>  #define RISCV_ISA_EXT_XLINUXENVCFG	127
>  
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 2902f68dc913..060212331a03 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -59,6 +59,7 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
>  #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
>  #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
> +#define		RISCV_HWPROBE_EXT_ZICCLSM	(1ULL << 36)

Missing an update to hwprobe.rst.

>  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
>  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 3ed2359eae35..863c708f2f2e 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -305,6 +305,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>  	__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
>  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>  	__RISCV_ISA_EXT_DATA(xandespmu, RISCV_ISA_EXT_XANDESPMU),
> +	__RISCV_ISA_EXT_DATA(zicclsm, RISCV_ISA_EXT_ZICCLSM),

Please read the ordering comment above this structure!
Also, you're missing dt-binding documentation for the extension.

>  };
>  
>  const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index 8cae41a502dd..b286b73e763e 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -125,6 +125,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  			EXT_KEY(ZVKT);
>  			EXT_KEY(ZVFH);
>  			EXT_KEY(ZVFHMIN);
> +			EXT_KEY(ZICCLSM);

Order looks off here too, I think this should be added in in the same
order as to riscv_isa_ext, although the requirement isn't hard here,
just that adding to the end of a list means it's annoying to check for
what's missing.

Thanks,
Conor.

>  		}
>  
>  		if (has_fpu()) {
> -- 
> 2.43.0
> 
> 

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

Re: [PATCH 1/3] RISC-V: Add Zicclsm to cpufeature and hwprobe

[Charlie Jenkins]

On Thu, Jun 06, 2024 at 07:43:52PM +0100, Conor Dooley wrote:
> On Thu, Jun 06, 2024 at 02:32:13PM -0400, Jesse Taube wrote:
> > > Zicclsm Misaligned loads and stores to main memory regions with both
> > > the cacheability and coherence PMAs must be supported.
> > > Note:
> > > This introduces a new extension name for this feature.
> > > This requires misaligned support for all regular load and store
> > > instructions (including scalar and vector) but not AMOs or other
> > > specialized forms of memory access. Even though mandated, misaligned
> > > loads and stores might execute extremely slowly. Standard software
> > > distributions should assume their existence only for correctness,
> > > not for performance.
> > 
> > Detecing zicclsm allows the kernel to report if the
> > hardware supports misaligned accesses even if support wasn't probed.
> > 
> > This is useful for usermode to know if vector misaligned accesses are
> > supported.
> > 
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/hwcap.h        | 1 +
> >  arch/riscv/include/uapi/asm/hwprobe.h | 1 +
> >  arch/riscv/kernel/cpufeature.c        | 1 +
> >  arch/riscv/kernel/sys_hwprobe.c       | 1 +
> >  4 files changed, 4 insertions(+)
> > 
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index e17d0078a651..8c0d0b555a8e 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -81,6 +81,7 @@
> >  #define RISCV_ISA_EXT_ZTSO		72
> >  #define RISCV_ISA_EXT_ZACAS		73
> >  #define RISCV_ISA_EXT_XANDESPMU		74
> > +#define RISCV_ISA_EXT_ZICCLSM		75
> >  
> >  #define RISCV_ISA_EXT_XLINUXENVCFG	127
> >  
> > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > index 2902f68dc913..060212331a03 100644
> > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > @@ -59,6 +59,7 @@ struct riscv_hwprobe {
> >  #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
> >  #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
> >  #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
> > +#define		RISCV_HWPROBE_EXT_ZICCLSM	(1ULL << 36)
> 
> Missing an update to hwprobe.rst.
> 

"RISCV_HWPROBE_EXT_ZIHINTPAUSE	(1ULL << 36)" was also defined here in
6.10 so this key needs to be bumped down one.

- Charlie

> >  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
> >  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
> >  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 3ed2359eae35..863c708f2f2e 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -305,6 +305,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> >  	__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
> >  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >  	__RISCV_ISA_EXT_DATA(xandespmu, RISCV_ISA_EXT_XANDESPMU),
> > +	__RISCV_ISA_EXT_DATA(zicclsm, RISCV_ISA_EXT_ZICCLSM),
> 
> Please read the ordering comment above this structure!
> Also, you're missing dt-binding documentation for the extension.
> 
> >  };
> >  
> >  const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
> > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > index 8cae41a502dd..b286b73e763e 100644
> > --- a/arch/riscv/kernel/sys_hwprobe.c
> > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > @@ -125,6 +125,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
> >  			EXT_KEY(ZVKT);
> >  			EXT_KEY(ZVFH);
> >  			EXT_KEY(ZVFHMIN);
> > +			EXT_KEY(ZICCLSM);
> 
> Order looks off here too, I think this should be added in in the same
> order as to riscv_isa_ext, although the requirement isn't hard here,
> just that adding to the end of a list means it's annoying to check for
> what's missing.
> 
> Thanks,
> Conor.
> 
> >  		}
> >  
> >  		if (has_fpu()) {
> > -- 
> > 2.43.0
> > 
> >