> 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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
>
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
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/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
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 <[email protected]>
> > ---
> > 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
> >
> >
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/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
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 <[email protected]>
> > ---
> > 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
> >
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 <[email protected]>
> ---
> 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
>
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 <[email protected]>
>>> ---
>>> 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
>>>
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 <[email protected]>
>>>> ---
>>>> 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
>>>>
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 <[email protected]>
> > > > ---
> > > > 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
> > > >
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.
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.
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 <[email protected]>
>> ---
>> 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
[email protected]/ [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
>>
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 <[email protected]>
>>> ---
>>> 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
> [email protected]/ [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
>>>