| Message ID | 20240606183215.416829-2-jesse@rivosinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] RISC-V: Add Zicclsm to cpufeature and hwprobe | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote: > Run a unaligned vector access to test if the system supports > vector unaligned access. Add the result to a new key in hwprobe. > This is useful for usermode to know if vector misaligned accesses are > supported and if they are faster or slower than equivalent byte accesses. > > Signed-off-by: Jesse Taube <jesse@rivosinc.com> > --- > arch/riscv/include/asm/cpufeature.h | 2 + > arch/riscv/include/asm/hwprobe.h | 2 +- > arch/riscv/include/asm/vector.h | 1 + > arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ > arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ > arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- > arch/riscv/kernel/unaligned_access_speed.c | 4 ++ > arch/riscv/kernel/vector.c | 2 +- > 8 files changed, 129 insertions(+), 6 deletions(-) > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > index 347805446151..a012c8490a27 100644 > --- a/arch/riscv/include/asm/cpufeature.h > +++ b/arch/riscv/include/asm/cpufeature.h > @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void); > > #if defined(CONFIG_RISCV_MISALIGNED) > bool check_unaligned_access_emulated_all_cpus(void); > +bool check_vector_unaligned_access_all_cpus(void); > void unaligned_emulation_finish(void); > bool unaligned_ctl_available(void); > DECLARE_PER_CPU(long, misaligned_access_speed); > +DECLARE_PER_CPU(long, vector_misaligned_access); > #else > static inline bool unaligned_ctl_available(void) > { > diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h > index 630507dff5ea..150a9877b0af 100644 > --- a/arch/riscv/include/asm/hwprobe.h > +++ b/arch/riscv/include/asm/hwprobe.h > @@ -8,7 +8,7 @@ > > #include <uapi/asm/hwprobe.h> > > -#define RISCV_HWPROBE_MAX_KEY 6 > +#define RISCV_HWPROBE_MAX_KEY 7 > > static inline bool riscv_hwprobe_key_is_valid(__s64 key) > { > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > index 731dcd0ed4de..776af9b37e23 100644 > --- a/arch/riscv/include/asm/vector.h > +++ b/arch/riscv/include/asm/vector.h > @@ -21,6 +21,7 @@ > > extern unsigned long riscv_v_vsize; > int riscv_v_setup_vsize(void); > +bool insn_is_vector(u32 insn_buf); > bool riscv_v_first_use_handler(struct pt_regs *regs); > void kernel_vector_begin(void); > void kernel_vector_end(void); > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h > index 060212331a03..ebacff86f134 100644 > --- a/arch/riscv/include/uapi/asm/hwprobe.h > +++ b/arch/riscv/include/uapi/asm/hwprobe.h > @@ -68,6 +68,12 @@ struct riscv_hwprobe { > #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) > #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) > #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6 > +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF 7 > +#define RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN 0 > +#define RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED 1 > +#define RISCV_HWPROBE_VEC_MISALIGNED_SLOW 2 > +#define RISCV_HWPROBE_VEC_MISALIGNED_FAST 3 > +#define RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED 4 > /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */ > > /* Flags */ > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c > index b286b73e763e..ce641cc6e47a 100644 > --- a/arch/riscv/kernel/sys_hwprobe.c > +++ b/arch/riscv/kernel/sys_hwprobe.c > @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus) > } > #endif > > +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS) > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) > +{ > + int cpu; > + u64 perf = -1ULL; > + > + for_each_cpu(cpu, cpus) { > + int this_perf = per_cpu(vector_misaligned_access, cpu); > + > + if (perf == -1ULL) > + perf = this_perf; > + > + if (perf != this_perf) { > + perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > + break; > + } > + } > + > + if (perf == -1ULL) > + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > + > + return perf; > +} > +#else > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) > +{ > + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > +} > +#endif > + > static void hwprobe_one_pair(struct riscv_hwprobe *pair, > const struct cpumask *cpus) > { > @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, > pair->value = hwprobe_misaligned(cpus); > break; > > + case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF: > + pair->value = hwprobe_vec_misaligned(cpus); > + break; > + > case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE: > pair->value = 0; > if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ)) > diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c > index 2adb7c3e4dd5..8f26c3d92230 100644 > --- a/arch/riscv/kernel/traps_misaligned.c > +++ b/arch/riscv/kernel/traps_misaligned.c > @@ -16,6 +16,7 @@ > #include <asm/entry-common.h> > #include <asm/hwprobe.h> > #include <asm/cpufeature.h> > +#include <asm/vector.h> > > #define INSN_MATCH_LB 0x3 > #define INSN_MASK_LB 0x707f > @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs) > > perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); > > -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > - *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; > -#endif > - > if (!unaligned_enabled) > return -1; > > @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs) > if (get_insn(regs, epc, &insn)) > return -1; > > +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > + if (insn_is_vector(insn) && > + *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { > + *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; > + regs->epc = epc + INSN_LEN(insn); > + return 0; > + } > + > + *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; This unconditionally sets scalar unaligned accesses even if the unaligned access is caused by vector. Scalar unaligned accesses should only be set to emulated if this function is entered from a scalar unaligned load. The rest of this function handles how scalar unaligned accesses are emulated, and the equivalent needs to happen for vector. You need to add routines that manually load the data from the memory address into the vector register. When Clément did this for scalar, he provided a test case to help reviewers [1]. Please add onto these test cases or make your own for vector. Link: https://github.com/clementleger/unaligned_test [1] > +#endif > + > regs->epc = 0; > > if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { > @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu) > return misaligned_emu_detected; > } > > +#ifdef CONFIG_RISCV_ISA_V > +static void check_vector_unaligned_access(struct work_struct *unused) Can you standardize this name with the scalar version by writing emulated in it? "check_vector_unaligned_access_emulated_all_cpus" > +{ > + int cpu = smp_processor_id(); > + long *mas_ptr = this_cpu_ptr(&vector_misaligned_access); > + unsigned long tmp_var; > + > + if (!riscv_isa_extension_available(hart_isa[cpu].isa, v)) > + return; > + > + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > + > + local_irq_enable(); > + kernel_vector_begin(); > + __asm__ __volatile__ ( > + ".balign 4\n\t" > + ".option push\n\t" > + ".option arch, +v\n\t" > + " vsetivli zero, 1, e16, m1, ta, ma\n\t" // Vectors of 16b > + " vle16.v v0, (%[ptr])\n\t" // Load bytes > + ".option pop\n\t" > + : : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory"); memory is being read from, but not written to, so there is no need to have a memory clobber. > + kernel_vector_end(); > + > + if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN) > + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > +} > + > +bool check_vector_unaligned_access_all_cpus(void) > +{ > + int cpu; > + bool ret = true; > + > + for_each_online_cpu(cpu) > + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) zicclsm is not specific to vector so it can be extracted out of this vector specific function. Assuming that hardware properly reports the extension, if zicclsm is present then it is known that both vector and scalar unaligned accesses are supported. > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; Please use the exising UNKNOWN terminology instead of renaming to SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned accesses are supported. > + else > + ret = false; > + > + > + if (ret) > + return true; > + ret = true; > + > + schedule_on_each_cpu(check_vector_unaligned_access); > + > + for_each_online_cpu(cpu) > + if (per_cpu(vector_misaligned_access, cpu) > + != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) > + return false; > + > + return ret; > +} > +#else If CONFIG_RISCV_ISA_V is not set, there is no value in checking if vector unaligned accesses are supported because userspace will not be allowed to use vector instructions anyway. - Charlie > +bool check_vector_unaligned_access_all_cpus(void) > +{ > + int cpu; > + > + for_each_online_cpu(cpu) > + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > + else > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; > + > + return false; > +} > +#endif > + > bool check_unaligned_access_emulated_all_cpus(void) > { > int cpu; > diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c > index a9a6bcb02acf..92a84239beaa 100644 > --- a/arch/riscv/kernel/unaligned_access_speed.c > +++ b/arch/riscv/kernel/unaligned_access_speed.c > @@ -20,6 +20,7 @@ > #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80) > > DEFINE_PER_CPU(long, misaligned_access_speed); > +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > static cpumask_t fast_misaligned_access; > @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void) > { > bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); > > + check_vector_unaligned_access_all_cpus(); > + > if (!all_cpus_emulated) > return check_unaligned_access_speed_all_cpus(); > > @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void) > static int check_unaligned_access_all_cpus(void) > { > check_unaligned_access_emulated_all_cpus(); > + check_vector_unaligned_access_all_cpus(); > > return 0; > } > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > index 6727d1d3b8f2..2cceab739b2c 100644 > --- a/arch/riscv/kernel/vector.c > +++ b/arch/riscv/kernel/vector.c > @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void) > #endif > } > > -static bool insn_is_vector(u32 insn_buf) > +bool insn_is_vector(u32 insn_buf) > { > u32 opcode = insn_buf & __INSN_OPCODE_MASK; > u32 width, csr; > -- > 2.43.0 >
Hi Jesse, kernel test robot noticed the following build errors: [auto build test ERROR on v6.9] [cannot apply to akpm-mm/mm-everything linus/master v6.10-rc2 v6.10-rc1 next-20240606] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jesse-Taube/RISC-V-Detect-unaligned-vector-accesses-supported/20240607-023434 base: v6.9 patch link: https://lore.kernel.org/r/20240606183215.416829-2-jesse%40rivosinc.com patch subject: [PATCH 2/3] RISC-V: Detect unaligned vector accesses supported. config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20240607/202406070508.6UJUx2rO-lkp@intel.com/config) compiler: riscv64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070508.6UJUx2rO-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202406070508.6UJUx2rO-lkp@intel.com/ All errors (new ones prefixed by >>): arch/riscv/kernel/traps_misaligned.c: In function 'handle_misaligned_load': >> arch/riscv/kernel/traps_misaligned.c:427:13: error: implicit declaration of function 'insn_is_vector' [-Werror=implicit-function-declaration] 427 | if (insn_is_vector(insn) && | ^~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +/insn_is_vector +427 arch/riscv/kernel/traps_misaligned.c 406 407 int handle_misaligned_load(struct pt_regs *regs) 408 { 409 union reg_data val; 410 unsigned long epc = regs->epc; 411 unsigned long insn; 412 unsigned long addr = regs->badaddr; 413 int i, fp = 0, shift = 0, len = 0; 414 415 perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); 416 417 if (!unaligned_enabled) 418 return -1; 419 420 if (user_mode(regs) && (current->thread.align_ctl & PR_UNALIGN_SIGBUS)) 421 return -1; 422 423 if (get_insn(regs, epc, &insn)) 424 return -1; 425 426 #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > 427 if (insn_is_vector(insn) && 428 *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { 429 *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; 430 regs->epc = epc + INSN_LEN(insn); 431 return 0; 432 } 433 434 *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; 435 #endif 436 437 regs->epc = 0; 438 439 if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { 440 len = 4; 441 shift = 8 * (sizeof(unsigned long) - len); 442 #if defined(CONFIG_64BIT) 443 } else if ((insn & INSN_MASK_LD) == INSN_MATCH_LD) { 444 len = 8; 445 shift = 8 * (sizeof(unsigned long) - len); 446 } else if ((insn & INSN_MASK_LWU) == INSN_MATCH_LWU) { 447 len = 4; 448 #endif 449 } else if ((insn & INSN_MASK_FLD) == INSN_MATCH_FLD) { 450 fp = 1; 451 len = 8; 452 } else if ((insn & INSN_MASK_FLW) == INSN_MATCH_FLW) { 453 fp = 1; 454 len = 4; 455 } else if ((insn & INSN_MASK_LH) == INSN_MATCH_LH) { 456 len = 2; 457 shift = 8 * (sizeof(unsigned long) - len); 458 } else if ((insn & INSN_MASK_LHU) == INSN_MATCH_LHU) { 459 len = 2; 460 #if defined(CONFIG_64BIT) 461 } else if ((insn & INSN_MASK_C_LD) == INSN_MATCH_C_LD) { 462 len = 8; 463 shift = 8 * (sizeof(unsigned long) - len); 464 insn = RVC_RS2S(insn) << SH_RD; 465 } else if ((insn & INSN_MASK_C_LDSP) == INSN_MATCH_C_LDSP && 466 ((insn >> SH_RD) & 0x1f)) { 467 len = 8; 468 shift = 8 * (sizeof(unsigned long) - len); 469 #endif 470 } else if ((insn & INSN_MASK_C_LW) == INSN_MATCH_C_LW) { 471 len = 4; 472 shift = 8 * (sizeof(unsigned long) - len); 473 insn = RVC_RS2S(insn) << SH_RD; 474 } else if ((insn & INSN_MASK_C_LWSP) == INSN_MATCH_C_LWSP && 475 ((insn >> SH_RD) & 0x1f)) { 476 len = 4; 477 shift = 8 * (sizeof(unsigned long) - len); 478 } else if ((insn & INSN_MASK_C_FLD) == INSN_MATCH_C_FLD) { 479 fp = 1; 480 len = 8; 481 insn = RVC_RS2S(insn) << SH_RD; 482 } else if ((insn & INSN_MASK_C_FLDSP) == INSN_MATCH_C_FLDSP) { 483 fp = 1; 484 len = 8; 485 #if defined(CONFIG_32BIT) 486 } else if ((insn & INSN_MASK_C_FLW) == INSN_MATCH_C_FLW) { 487 fp = 1; 488 len = 4; 489 insn = RVC_RS2S(insn) << SH_RD; 490 } else if ((insn & INSN_MASK_C_FLWSP) == INSN_MATCH_C_FLWSP) { 491 fp = 1; 492 len = 4; 493 #endif 494 } else { 495 regs->epc = epc; 496 return -1; 497 } 498 499 if (!IS_ENABLED(CONFIG_FPU) && fp) 500 return -EOPNOTSUPP; 501 502 val.data_u64 = 0; 503 for (i = 0; i < len; i++) { 504 if (load_u8(regs, (void *)(addr + i), &val.data_bytes[i])) 505 return -1; 506 } 507 508 if (!fp) 509 SET_RD(insn, regs, val.data_ulong << shift >> shift); 510 else if (len == 8) 511 set_f64_rd(insn, regs, val.data_u64); 512 else 513 set_f32_rd(insn, regs, val.data_ulong); 514 515 regs->epc = epc + INSN_LEN(insn); 516 517 return 0; 518 } 519
On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote: > On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote: > > Run a unaligned vector access to test if the system supports > > vector unaligned access. Add the result to a new key in hwprobe. > > This is useful for usermode to know if vector misaligned accesses are > > supported and if they are faster or slower than equivalent byte accesses. > > > > Signed-off-by: Jesse Taube <jesse@rivosinc.com> > > --- > > arch/riscv/include/asm/cpufeature.h | 2 + > > arch/riscv/include/asm/hwprobe.h | 2 +- > > arch/riscv/include/asm/vector.h | 1 + > > arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ > > arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ > > arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- > > arch/riscv/kernel/unaligned_access_speed.c | 4 ++ > > arch/riscv/kernel/vector.c | 2 +- > > 8 files changed, 129 insertions(+), 6 deletions(-) > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > index 347805446151..a012c8490a27 100644 > > --- a/arch/riscv/include/asm/cpufeature.h > > +++ b/arch/riscv/include/asm/cpufeature.h > > @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void); > > > > #if defined(CONFIG_RISCV_MISALIGNED) > > bool check_unaligned_access_emulated_all_cpus(void); > > +bool check_vector_unaligned_access_all_cpus(void); > > void unaligned_emulation_finish(void); > > bool unaligned_ctl_available(void); > > DECLARE_PER_CPU(long, misaligned_access_speed); > > +DECLARE_PER_CPU(long, vector_misaligned_access); > > #else > > static inline bool unaligned_ctl_available(void) > > { > > diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h > > index 630507dff5ea..150a9877b0af 100644 > > --- a/arch/riscv/include/asm/hwprobe.h > > +++ b/arch/riscv/include/asm/hwprobe.h > > @@ -8,7 +8,7 @@ > > > > #include <uapi/asm/hwprobe.h> > > > > -#define RISCV_HWPROBE_MAX_KEY 6 > > +#define RISCV_HWPROBE_MAX_KEY 7 > > > > static inline bool riscv_hwprobe_key_is_valid(__s64 key) > > { > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > > index 731dcd0ed4de..776af9b37e23 100644 > > --- a/arch/riscv/include/asm/vector.h > > +++ b/arch/riscv/include/asm/vector.h > > @@ -21,6 +21,7 @@ > > > > extern unsigned long riscv_v_vsize; > > int riscv_v_setup_vsize(void); > > +bool insn_is_vector(u32 insn_buf); > > bool riscv_v_first_use_handler(struct pt_regs *regs); > > void kernel_vector_begin(void); > > void kernel_vector_end(void); > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h > > index 060212331a03..ebacff86f134 100644 > > --- a/arch/riscv/include/uapi/asm/hwprobe.h > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h > > @@ -68,6 +68,12 @@ struct riscv_hwprobe { > > #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) > > #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) > > #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6 > > +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF 7 > > +#define RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN 0 > > +#define RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED 1 > > +#define RISCV_HWPROBE_VEC_MISALIGNED_SLOW 2 > > +#define RISCV_HWPROBE_VEC_MISALIGNED_FAST 3 > > +#define RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED 4 > > /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */ > > > > /* Flags */ > > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c > > index b286b73e763e..ce641cc6e47a 100644 > > --- a/arch/riscv/kernel/sys_hwprobe.c > > +++ b/arch/riscv/kernel/sys_hwprobe.c > > @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus) > > } > > #endif > > > > +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS) > > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) > > +{ > > + int cpu; > > + u64 perf = -1ULL; > > + > > + for_each_cpu(cpu, cpus) { > > + int this_perf = per_cpu(vector_misaligned_access, cpu); > > + > > + if (perf == -1ULL) > > + perf = this_perf; > > + > > + if (perf != this_perf) { > > + perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > + break; > > + } > > + } > > + > > + if (perf == -1ULL) > > + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > + > > + return perf; > > +} > > +#else > > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) > > +{ I meant to mention this in my last message! The scalar version has cutouts for configs here like: if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS)) return RISCV_HWPROBE_MISALIGNED_FAST; Having this functionality on vector as well would be much appreciated. I don't think it's valid to assume that vector and scalar have the same speed, so this would require a vector version of the RISCV_MISALIGNED tree in arch/riscv/Kconfig. - Charlie > > + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > +} > > +#endif > > + > > static void hwprobe_one_pair(struct riscv_hwprobe *pair, > > const struct cpumask *cpus) > > { > > @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, > > pair->value = hwprobe_misaligned(cpus); > > break; > > > > + case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF: > > + pair->value = hwprobe_vec_misaligned(cpus); > > + break; > > + > > case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE: > > pair->value = 0; > > if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ)) > > diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c > > index 2adb7c3e4dd5..8f26c3d92230 100644 > > --- a/arch/riscv/kernel/traps_misaligned.c > > +++ b/arch/riscv/kernel/traps_misaligned.c > > @@ -16,6 +16,7 @@ > > #include <asm/entry-common.h> > > #include <asm/hwprobe.h> > > #include <asm/cpufeature.h> > > +#include <asm/vector.h> > > > > #define INSN_MATCH_LB 0x3 > > #define INSN_MASK_LB 0x707f > > @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs) > > > > perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); > > > > -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > > - *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; > > -#endif > > - > > if (!unaligned_enabled) > > return -1; > > > > @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs) > > if (get_insn(regs, epc, &insn)) > > return -1; > > > > +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > > + if (insn_is_vector(insn) && > > + *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { > > + *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; > > + regs->epc = epc + INSN_LEN(insn); > > + return 0; > > + } > > + > > + *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; > > This unconditionally sets scalar unaligned accesses even if the > unaligned access is caused by vector. Scalar unaligned accesses should > only be set to emulated if this function is entered from a scalar > unaligned load. > > The rest of this function handles how scalar unaligned accesses are > emulated, and the equivalent needs to happen for vector. You need to add > routines that manually load the data from the memory address into the > vector register. When Clément did this for scalar, he provided a test > case to help reviewers [1]. Please add onto these test cases or make > your own for vector. > > Link: https://github.com/clementleger/unaligned_test [1] > > > +#endif > > + > > regs->epc = 0; > > > > if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { > > @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu) > > return misaligned_emu_detected; > > } > > > > +#ifdef CONFIG_RISCV_ISA_V > > +static void check_vector_unaligned_access(struct work_struct *unused) > > Can you standardize this name with the scalar version by writing > emulated in it? > > "check_vector_unaligned_access_emulated_all_cpus" > > > +{ > > + int cpu = smp_processor_id(); > > + long *mas_ptr = this_cpu_ptr(&vector_misaligned_access); > > + unsigned long tmp_var; > > + > > + if (!riscv_isa_extension_available(hart_isa[cpu].isa, v)) > > + return; > > + > > + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > + > > + local_irq_enable(); > > + kernel_vector_begin(); > > + __asm__ __volatile__ ( > > + ".balign 4\n\t" > > + ".option push\n\t" > > + ".option arch, +v\n\t" > > + " vsetivli zero, 1, e16, m1, ta, ma\n\t" // Vectors of 16b > > + " vle16.v v0, (%[ptr])\n\t" // Load bytes > > + ".option pop\n\t" > > + : : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory"); > > memory is being read from, but not written to, so there is no need to > have a memory clobber. > > > + kernel_vector_end(); > > + > > + if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN) > > + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > +} > > + > > +bool check_vector_unaligned_access_all_cpus(void) > > +{ > > + int cpu; > > + bool ret = true; > > + > > + for_each_online_cpu(cpu) > > + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) > > zicclsm is not specific to vector so it can be extracted out of this > vector specific function. Assuming that hardware properly reports the > extension, if zicclsm is present then it is known that both vector and > scalar unaligned accesses are supported. > > > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > Please use the exising UNKNOWN terminology instead of renaming to > SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned > accesses are supported. > > > + else > > + ret = false; > > + > > + > > + if (ret) > > + return true; > > + ret = true; > > + > > + schedule_on_each_cpu(check_vector_unaligned_access); > > + > > + for_each_online_cpu(cpu) > > + if (per_cpu(vector_misaligned_access, cpu) > > + != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) > > + return false; > > + > > + return ret; > > +} > > +#else > > If CONFIG_RISCV_ISA_V is not set, there is no value in checking if > vector unaligned accesses are supported because userspace will not be > allowed to use vector instructions anyway. > > - Charlie > > > +bool check_vector_unaligned_access_all_cpus(void) > > +{ > > + int cpu; > > + > > + for_each_online_cpu(cpu) > > + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) > > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > + else > > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; > > + > > + return false; > > +} > > +#endif > > + > > bool check_unaligned_access_emulated_all_cpus(void) > > { > > int cpu; > > diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c > > index a9a6bcb02acf..92a84239beaa 100644 > > --- a/arch/riscv/kernel/unaligned_access_speed.c > > +++ b/arch/riscv/kernel/unaligned_access_speed.c > > @@ -20,6 +20,7 @@ > > #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80) > > > > DEFINE_PER_CPU(long, misaligned_access_speed); > > +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > > > #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > > static cpumask_t fast_misaligned_access; > > @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void) > > { > > bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); > > > > + check_vector_unaligned_access_all_cpus(); > > + > > if (!all_cpus_emulated) > > return check_unaligned_access_speed_all_cpus(); > > > > @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void) > > static int check_unaligned_access_all_cpus(void) > > { > > check_unaligned_access_emulated_all_cpus(); > > + check_vector_unaligned_access_all_cpus(); > > > > return 0; > > } > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > > index 6727d1d3b8f2..2cceab739b2c 100644 > > --- a/arch/riscv/kernel/vector.c > > +++ b/arch/riscv/kernel/vector.c > > @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void) > > #endif > > } > > > > -static bool insn_is_vector(u32 insn_buf) > > +bool insn_is_vector(u32 insn_buf) > > { > > u32 opcode = insn_buf & __INSN_OPCODE_MASK; > > u32 width, csr; > > -- > > 2.43.0 > >
On 6/6/24 19:13, Charlie Jenkins wrote: > On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote: >> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote: >>> Run a unaligned vector access to test if the system supports >>> vector unaligned access. Add the result to a new key in hwprobe. >>> This is useful for usermode to know if vector misaligned accesses are >>> supported and if they are faster or slower than equivalent byte accesses. >>> >>> Signed-off-by: Jesse Taube <jesse@rivosinc.com> >>> --- >>> arch/riscv/include/asm/cpufeature.h | 2 + >>> arch/riscv/include/asm/hwprobe.h | 2 +- >>> arch/riscv/include/asm/vector.h | 1 + >>> arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ >>> arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ >>> arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- >>> arch/riscv/kernel/unaligned_access_speed.c | 4 ++ >>> arch/riscv/kernel/vector.c | 2 +- >>> 8 files changed, 129 insertions(+), 6 deletions(-) >>> >>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h >>> index 347805446151..a012c8490a27 100644 >>> --- a/arch/riscv/include/asm/cpufeature.h >>> +++ b/arch/riscv/include/asm/cpufeature.h >>> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void); >>> >>> #if defined(CONFIG_RISCV_MISALIGNED) >>> bool check_unaligned_access_emulated_all_cpus(void); >>> +bool check_vector_unaligned_access_all_cpus(void); >>> void unaligned_emulation_finish(void); >>> bool unaligned_ctl_available(void); >>> DECLARE_PER_CPU(long, misaligned_access_speed); >>> +DECLARE_PER_CPU(long, vector_misaligned_access); >>> #else >>> static inline bool unaligned_ctl_available(void) >>> { >>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h >>> index 630507dff5ea..150a9877b0af 100644 >>> --- a/arch/riscv/include/asm/hwprobe.h >>> +++ b/arch/riscv/include/asm/hwprobe.h >>> @@ -8,7 +8,7 @@ >>> >>> #include <uapi/asm/hwprobe.h> >>> >>> -#define RISCV_HWPROBE_MAX_KEY 6 >>> +#define RISCV_HWPROBE_MAX_KEY 7 >>> >>> static inline bool riscv_hwprobe_key_is_valid(__s64 key) >>> { >>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h >>> index 731dcd0ed4de..776af9b37e23 100644 >>> --- a/arch/riscv/include/asm/vector.h >>> +++ b/arch/riscv/include/asm/vector.h >>> @@ -21,6 +21,7 @@ >>> >>> extern unsigned long riscv_v_vsize; >>> int riscv_v_setup_vsize(void); >>> +bool insn_is_vector(u32 insn_buf); >>> bool riscv_v_first_use_handler(struct pt_regs *regs); >>> void kernel_vector_begin(void); >>> void kernel_vector_end(void); >>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h >>> index 060212331a03..ebacff86f134 100644 >>> --- a/arch/riscv/include/uapi/asm/hwprobe.h >>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h >>> @@ -68,6 +68,12 @@ struct riscv_hwprobe { >>> #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) >>> #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) >>> #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6 >>> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF 7 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN 0 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED 1 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_SLOW 2 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_FAST 3 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED 4 >>> /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */ >>> >>> /* Flags */ >>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c >>> index b286b73e763e..ce641cc6e47a 100644 >>> --- a/arch/riscv/kernel/sys_hwprobe.c >>> +++ b/arch/riscv/kernel/sys_hwprobe.c >>> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus) >>> } >>> #endif >>> >>> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS) >>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >>> +{ >>> + int cpu; >>> + u64 perf = -1ULL; >>> + >>> + for_each_cpu(cpu, cpus) { >>> + int this_perf = per_cpu(vector_misaligned_access, cpu); >>> + >>> + if (perf == -1ULL) >>> + perf = this_perf; >>> + >>> + if (perf != this_perf) { >>> + perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> + break; >>> + } >>> + } >>> + >>> + if (perf == -1ULL) >>> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> + >>> + return perf; >>> +} >>> +#else >>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >>> +{ > > I meant to mention this in my last message! > > The scalar version has cutouts for configs here like: > > if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS)) > return RISCV_HWPROBE_MISALIGNED_FAST; > Will add > Having this functionality on vector as well would be much appreciated. > I don't think it's valid to assume that vector and scalar have the same > speed, so this would require a vector version of the RISCV_MISALIGNED > tree in arch/riscv/Kconfig. > > - Charlie > >>> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> +} >>> +#endif >>> + >>> static void hwprobe_one_pair(struct riscv_hwprobe *pair, >>> const struct cpumask *cpus) >>> { >>> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, >>> pair->value = hwprobe_misaligned(cpus); >>> break; >>> >>> + case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF: >>> + pair->value = hwprobe_vec_misaligned(cpus); >>> + break; >>> + >>> case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE: >>> pair->value = 0; >>> if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ)) >>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c >>> index 2adb7c3e4dd5..8f26c3d92230 100644 >>> --- a/arch/riscv/kernel/traps_misaligned.c >>> +++ b/arch/riscv/kernel/traps_misaligned.c >>> @@ -16,6 +16,7 @@ >>> #include <asm/entry-common.h> >>> #include <asm/hwprobe.h> >>> #include <asm/cpufeature.h> >>> +#include <asm/vector.h> >>> >>> #define INSN_MATCH_LB 0x3 >>> #define INSN_MASK_LB 0x707f >>> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs) >>> >>> perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); >>> >>> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>> - *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; >>> -#endif >>> - >>> if (!unaligned_enabled) >>> return -1; >>> >>> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs) >>> if (get_insn(regs, epc, &insn)) >>> return -1; >>> >>> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>> + if (insn_is_vector(insn) && >>> + *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { >>> + *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >>> + regs->epc = epc + INSN_LEN(insn); >>> + return 0; There is a return before scalar speed is set. >>> + } >>> + >>> + *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; >> >> This unconditionally sets scalar unaligned accesses even if the >> unaligned access is caused by vector. Scalar unaligned accesses should >> only be set to emulated if this function is entered from a scalar >> unaligned load. >> >> The rest of this function handles how scalar unaligned accesses are >> emulated, and the equivalent needs to happen for vector. You need to add >> routines that manually load the data from the memory address into the >> vector register. When Clément did this for scalar, he provided a test >> case to help reviewers [1]. Please add onto these test cases or make >> your own for vector. I wansnt planing on adding emulation in this patch. I can if needed. >> >> Link: https://github.com/clementleger/unaligned_test [1] >> >>> +#endif >>> + >>> regs->epc = 0; >>> >>> if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { >>> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu) >>> return misaligned_emu_detected; >>> } >>> >>> +#ifdef CONFIG_RISCV_ISA_V >>> +static void check_vector_unaligned_access(struct work_struct *unused) >> >> Can you standardize this name with the scalar version by writing >> emulated in it? We dont emulate it so that wouldn't make sence. >> >> "check_vector_unaligned_access_emulated_all_cpus" >> >>> +{ >>> + int cpu = smp_processor_id(); >>> + long *mas_ptr = this_cpu_ptr(&vector_misaligned_access); >>> + unsigned long tmp_var; >>> + >>> + if (!riscv_isa_extension_available(hart_isa[cpu].isa, v)) >>> + return; >>> + >>> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>> + >>> + local_irq_enable(); >>> + kernel_vector_begin(); >>> + __asm__ __volatile__ ( >>> + ".balign 4\n\t" >>> + ".option push\n\t" >>> + ".option arch, +v\n\t" >>> + " vsetivli zero, 1, e16, m1, ta, ma\n\t" // Vectors of 16b >>> + " vle16.v v0, (%[ptr])\n\t" // Load bytes >>> + ".option pop\n\t" >>> + : : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory"); >> >> memory is being read from, but not written to, so there is no need to >> have a memory clobber. fixed. >> >>> + kernel_vector_end(); >>> + >>> + if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN) >>> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>> +} >>> + >>> +bool check_vector_unaligned_access_all_cpus(void) >>> +{ >>> + int cpu; >>> + bool ret = true; >>> + >>> + for_each_online_cpu(cpu) >>> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) >> >> zicclsm is not specific to vector so it can be extracted out of this >> vector specific function. Assuming that hardware properly reports the >> extension, if zicclsm is present then it is known that both vector and >> scalar unaligned accesses are supported. Added so we don't need to waste cycles testing support. >>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >> >> Please use the exising UNKNOWN terminology instead of renaming to >> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned >> accesses are supported. Conor didnt like using UNKNOWN a proxy for "SUPPORTED" Having SUPPORTED is better then assuing the speed to be slow. >> >>> + else >>> + ret = false; >>> + >>> + >>> + if (ret) >>> + return true; >>> + ret = true; >>> + >>> + schedule_on_each_cpu(check_vector_unaligned_access); >>> + >>> + for_each_online_cpu(cpu) >>> + if (per_cpu(vector_misaligned_access, cpu) >>> + != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) >>> + return false; >>> + >>> + return ret; >>> +} >>> +#else >> >> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if >> vector unaligned accesses are supported because userspace will not be >> allowed to use vector instructions anyway. Oh I'm silly meant to be seting to all UNSUPPORTED. Thanks, Jesse Taube >> >> - Charlie >> >>> +bool check_vector_unaligned_access_all_cpus(void) >>> +{ >>> + int cpu; >>> + >>> + for_each_online_cpu(cpu) >>> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) >>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>> + else >>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >>> + >>> + return false; >>> +} >>> +#endif >>> + >>> bool check_unaligned_access_emulated_all_cpus(void) >>> { >>> int cpu; >>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c >>> index a9a6bcb02acf..92a84239beaa 100644 >>> --- a/arch/riscv/kernel/unaligned_access_speed.c >>> +++ b/arch/riscv/kernel/unaligned_access_speed.c >>> @@ -20,6 +20,7 @@ >>> #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80) >>> >>> DEFINE_PER_CPU(long, misaligned_access_speed); >>> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> >>> #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>> static cpumask_t fast_misaligned_access; >>> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void) >>> { >>> bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); >>> >>> + check_vector_unaligned_access_all_cpus(); >>> + >>> if (!all_cpus_emulated) >>> return check_unaligned_access_speed_all_cpus(); >>> >>> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void) >>> static int check_unaligned_access_all_cpus(void) >>> { >>> check_unaligned_access_emulated_all_cpus(); >>> + check_vector_unaligned_access_all_cpus(); >>> >>> return 0; >>> } >>> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c >>> index 6727d1d3b8f2..2cceab739b2c 100644 >>> --- a/arch/riscv/kernel/vector.c >>> +++ b/arch/riscv/kernel/vector.c >>> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void) >>> #endif >>> } >>> >>> -static bool insn_is_vector(u32 insn_buf) >>> +bool insn_is_vector(u32 insn_buf) >>> { >>> u32 opcode = insn_buf & __INSN_OPCODE_MASK; >>> u32 width, csr; >>> -- >>> 2.43.0 >>>
On 6/7/24 15:53, Jesse Taube wrote: > > > On 6/6/24 19:13, Charlie Jenkins wrote: >> On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote: >>> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote: >>>> Run a unaligned vector access to test if the system supports >>>> vector unaligned access. Add the result to a new key in hwprobe. >>>> This is useful for usermode to know if vector misaligned accesses are >>>> supported and if they are faster or slower than equivalent byte accesses. >>>> >>>> Signed-off-by: Jesse Taube <jesse@rivosinc.com> >>>> --- >>>> arch/riscv/include/asm/cpufeature.h | 2 + >>>> arch/riscv/include/asm/hwprobe.h | 2 +- >>>> arch/riscv/include/asm/vector.h | 1 + >>>> arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ >>>> arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ >>>> arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- >>>> arch/riscv/kernel/unaligned_access_speed.c | 4 ++ >>>> arch/riscv/kernel/vector.c | 2 +- >>>> 8 files changed, 129 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h >>>> index 347805446151..a012c8490a27 100644 >>>> --- a/arch/riscv/include/asm/cpufeature.h >>>> +++ b/arch/riscv/include/asm/cpufeature.h >>>> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void); >>>> >>>> #if defined(CONFIG_RISCV_MISALIGNED) >>>> bool check_unaligned_access_emulated_all_cpus(void); >>>> +bool check_vector_unaligned_access_all_cpus(void); >>>> void unaligned_emulation_finish(void); >>>> bool unaligned_ctl_available(void); >>>> DECLARE_PER_CPU(long, misaligned_access_speed); >>>> +DECLARE_PER_CPU(long, vector_misaligned_access); >>>> #else >>>> static inline bool unaligned_ctl_available(void) >>>> { >>>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h >>>> index 630507dff5ea..150a9877b0af 100644 >>>> --- a/arch/riscv/include/asm/hwprobe.h >>>> +++ b/arch/riscv/include/asm/hwprobe.h >>>> @@ -8,7 +8,7 @@ >>>> >>>> #include <uapi/asm/hwprobe.h> >>>> >>>> -#define RISCV_HWPROBE_MAX_KEY 6 >>>> +#define RISCV_HWPROBE_MAX_KEY 7 >>>> >>>> static inline bool riscv_hwprobe_key_is_valid(__s64 key) >>>> { >>>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h >>>> index 731dcd0ed4de..776af9b37e23 100644 >>>> --- a/arch/riscv/include/asm/vector.h >>>> +++ b/arch/riscv/include/asm/vector.h >>>> @@ -21,6 +21,7 @@ >>>> >>>> extern unsigned long riscv_v_vsize; >>>> int riscv_v_setup_vsize(void); >>>> +bool insn_is_vector(u32 insn_buf); >>>> bool riscv_v_first_use_handler(struct pt_regs *regs); >>>> void kernel_vector_begin(void); >>>> void kernel_vector_end(void); >>>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h >>>> index 060212331a03..ebacff86f134 100644 >>>> --- a/arch/riscv/include/uapi/asm/hwprobe.h >>>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h >>>> @@ -68,6 +68,12 @@ struct riscv_hwprobe { >>>> #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) >>>> #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) >>>> #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6 >>>> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF 7 >>>> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN 0 >>>> +#define RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED 1 >>>> +#define RISCV_HWPROBE_VEC_MISALIGNED_SLOW 2 >>>> +#define RISCV_HWPROBE_VEC_MISALIGNED_FAST 3 >>>> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED 4 >>>> /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */ >>>> >>>> /* Flags */ >>>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c >>>> index b286b73e763e..ce641cc6e47a 100644 >>>> --- a/arch/riscv/kernel/sys_hwprobe.c >>>> +++ b/arch/riscv/kernel/sys_hwprobe.c >>>> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus) >>>> } >>>> #endif >>>> >>>> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS) >>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >>>> +{ >>>> + int cpu; >>>> + u64 perf = -1ULL; >>>> + >>>> + for_each_cpu(cpu, cpus) { >>>> + int this_perf = per_cpu(vector_misaligned_access, cpu); >>>> + >>>> + if (perf == -1ULL) >>>> + perf = this_perf; >>>> + >>>> + if (perf != this_perf) { >>>> + perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>>> + break; >>>> + } >>>> + } >>>> + >>>> + if (perf == -1ULL) >>>> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>>> + >>>> + return perf; >>>> +} >>>> +#else >>>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >>>> +{ >> >> I meant to mention this in my last message! >> >> The scalar version has cutouts for configs here like: >> >> if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS)) >> return RISCV_HWPROBE_MISALIGNED_FAST; >> > For both scalar and vector CONFIG_RISCV_PROBE_UNALIGNED_ACCESS probes the speed not support. So misaligned_access_speed will have a valid value. ``` if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_VEC_UNALIGNED_ACCESS)) return RISCV_HWPROBE_MISALIGNED_FAST; if(IS_ENABLED(CONFIG_RISCV_SLOW_VEC_UNALIGNED_ACCESS)) return RISCV_HWPROBE_VEC_MISALIGNED_SLOW; if(IS_ENABLED(CONFIG_RISCV_VEC_UNALIGNED_ACCESS)) /* Sence we cant gurentee that we do have UNALIGNED_ACCESS we can return the result from the test */ return per_cpu(vector_misaligned_access, cpu); return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; ``` > Will add > >> Having this functionality on vector as well would be much appreciated. >> I don't think it's valid to assume that vector and scalar have the same >> speed, so this would require a vector version of the RISCV_MISALIGNED >> tree in arch/riscv/Kconfig. >> >> - Charlie >> >>>> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>>> +} >>>> +#endif >>>> + >>>> static void hwprobe_one_pair(struct riscv_hwprobe *pair, >>>> const struct cpumask *cpus) >>>> { >>>> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, >>>> pair->value = hwprobe_misaligned(cpus); >>>> break; >>>> >>>> + case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF: >>>> + pair->value = hwprobe_vec_misaligned(cpus); >>>> + break; >>>> + >>>> case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE: >>>> pair->value = 0; >>>> if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ)) >>>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c >>>> index 2adb7c3e4dd5..8f26c3d92230 100644 >>>> --- a/arch/riscv/kernel/traps_misaligned.c >>>> +++ b/arch/riscv/kernel/traps_misaligned.c >>>> @@ -16,6 +16,7 @@ >>>> #include <asm/entry-common.h> >>>> #include <asm/hwprobe.h> >>>> #include <asm/cpufeature.h> >>>> +#include <asm/vector.h> >>>> >>>> #define INSN_MATCH_LB 0x3 >>>> #define INSN_MASK_LB 0x707f >>>> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs) >>>> >>>> perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); >>>> >>>> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>>> - *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; >>>> -#endif >>>> - >>>> if (!unaligned_enabled) >>>> return -1; >>>> >>>> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs) >>>> if (get_insn(regs, epc, &insn)) >>>> return -1; >>>> >>>> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>>> + if (insn_is_vector(insn) && >>>> + *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { >>>> + *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >>>> + regs->epc = epc + INSN_LEN(insn); >>>> + return 0; > > There is a return before scalar speed is set. > >>>> + } >>>> + >>>> + *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; >>> >>> This unconditionally sets scalar unaligned accesses even if the >>> unaligned access is caused by vector. Scalar unaligned accesses should >>> only be set to emulated if this function is entered from a scalar >>> unaligned load. >>> >>> The rest of this function handles how scalar unaligned accesses are >>> emulated, and the equivalent needs to happen for vector. You need to add >>> routines that manually load the data from the memory address into the >>> vector register. When Clément did this for scalar, he provided a test >>> case to help reviewers [1]. Please add onto these test cases or make >>> your own for vector. > > I wansnt planing on adding emulation in this patch. I can if needed. > >>> >>> Link: https://github.com/clementleger/unaligned_test [1] >>> >>>> +#endif >>>> + >>>> regs->epc = 0; >>>> >>>> if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { >>>> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu) >>>> return misaligned_emu_detected; >>>> } >>>> >>>> +#ifdef CONFIG_RISCV_ISA_V >>>> +static void check_vector_unaligned_access(struct work_struct *unused) >>> >>> Can you standardize this name with the scalar version by writing >>> emulated in it? > > We dont emulate it so that wouldn't make sence. > >>> >>> "check_vector_unaligned_access_emulated_all_cpus" >>> >>>> +{ >>>> + int cpu = smp_processor_id(); >>>> + long *mas_ptr = this_cpu_ptr(&vector_misaligned_access); >>>> + unsigned long tmp_var; >>>> + >>>> + if (!riscv_isa_extension_available(hart_isa[cpu].isa, v)) >>>> + return; >>>> + >>>> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>>> + >>>> + local_irq_enable(); >>>> + kernel_vector_begin(); >>>> + __asm__ __volatile__ ( >>>> + ".balign 4\n\t" >>>> + ".option push\n\t" >>>> + ".option arch, +v\n\t" >>>> + " vsetivli zero, 1, e16, m1, ta, ma\n\t" // Vectors of 16b >>>> + " vle16.v v0, (%[ptr])\n\t" // Load bytes >>>> + ".option pop\n\t" >>>> + : : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory"); >>> >>> memory is being read from, but not written to, so there is no need to >>> have a memory clobber. > > fixed. > >>> >>>> + kernel_vector_end(); >>>> + >>>> + if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN) >>>> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>>> +} >>>> + >>>> +bool check_vector_unaligned_access_all_cpus(void) >>>> +{ >>>> + int cpu; >>>> + bool ret = true; >>>> + >>>> + for_each_online_cpu(cpu) >>>> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) >>> >>> zicclsm is not specific to vector so it can be extracted out of this >>> vector specific function. Assuming that hardware properly reports the >>> extension, if zicclsm is present then it is known that both vector and >>> scalar unaligned accesses are supported. > > Added so we don't need to waste cycles testing support. > >>>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>> >>> Please use the exising UNKNOWN terminology instead of renaming to >>> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned >>> accesses are supported. > > Conor didnt like using UNKNOWN a proxy for "SUPPORTED" > Having SUPPORTED is better then assuing the speed to be slow. > >>> >>>> + else >>>> + ret = false; >>>> + >>>> + >>>> + if (ret) >>>> + return true; >>>> + ret = true; >>>> + >>>> + schedule_on_each_cpu(check_vector_unaligned_access); >>>> + >>>> + for_each_online_cpu(cpu) >>>> + if (per_cpu(vector_misaligned_access, cpu) >>>> + != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) >>>> + return false; >>>> + >>>> + return ret; >>>> +} >>>> +#else >>> >>> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if >>> vector unaligned accesses are supported because userspace will not be >>> allowed to use vector instructions anyway. > > Oh I'm silly meant to be seting to all UNSUPPORTED. > > > Thanks, > Jesse Taube > >>> >>> - Charlie >>> >>>> +bool check_vector_unaligned_access_all_cpus(void) >>>> +{ >>>> + int cpu; >>>> + >>>> + for_each_online_cpu(cpu) >>>> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) >>>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>>> + else >>>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >>>> + >>>> + return false; >>>> +} >>>> +#endif >>>> + >>>> bool check_unaligned_access_emulated_all_cpus(void) >>>> { >>>> int cpu; >>>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c >>>> index a9a6bcb02acf..92a84239beaa 100644 >>>> --- a/arch/riscv/kernel/unaligned_access_speed.c >>>> +++ b/arch/riscv/kernel/unaligned_access_speed.c >>>> @@ -20,6 +20,7 @@ >>>> #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80) >>>> >>>> DEFINE_PER_CPU(long, misaligned_access_speed); >>>> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>>> >>>> #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>>> static cpumask_t fast_misaligned_access; >>>> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void) >>>> { >>>> bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); >>>> >>>> + check_vector_unaligned_access_all_cpus(); >>>> + >>>> if (!all_cpus_emulated) >>>> return check_unaligned_access_speed_all_cpus(); >>>> >>>> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void) >>>> static int check_unaligned_access_all_cpus(void) >>>> { >>>> check_unaligned_access_emulated_all_cpus(); >>>> + check_vector_unaligned_access_all_cpus(); >>>> >>>> return 0; >>>> } >>>> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c >>>> index 6727d1d3b8f2..2cceab739b2c 100644 >>>> --- a/arch/riscv/kernel/vector.c >>>> +++ b/arch/riscv/kernel/vector.c >>>> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void) >>>> #endif >>>> } >>>> >>>> -static bool insn_is_vector(u32 insn_buf) >>>> +bool insn_is_vector(u32 insn_buf) >>>> { >>>> u32 opcode = insn_buf & __INSN_OPCODE_MASK; >>>> u32 width, csr; >>>> -- >>>> 2.43.0 >>>>
On Fri, Jun 07, 2024 at 03:53:23PM -0400, Jesse Taube wrote: > > > On 6/6/24 19:13, Charlie Jenkins wrote: > > On Thu, Jun 06, 2024 at 02:29:23PM -0700, Charlie Jenkins wrote: > > > On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote: > > > > Run a unaligned vector access to test if the system supports > > > > vector unaligned access. Add the result to a new key in hwprobe. > > > > This is useful for usermode to know if vector misaligned accesses are > > > > supported and if they are faster or slower than equivalent byte accesses. > > > > > > > > Signed-off-by: Jesse Taube <jesse@rivosinc.com> > > > > --- > > > > arch/riscv/include/asm/cpufeature.h | 2 + > > > > arch/riscv/include/asm/hwprobe.h | 2 +- > > > > arch/riscv/include/asm/vector.h | 1 + > > > > arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ > > > > arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ > > > > arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- > > > > arch/riscv/kernel/unaligned_access_speed.c | 4 ++ > > > > arch/riscv/kernel/vector.c | 2 +- > > > > 8 files changed, 129 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > > > index 347805446151..a012c8490a27 100644 > > > > --- a/arch/riscv/include/asm/cpufeature.h > > > > +++ b/arch/riscv/include/asm/cpufeature.h > > > > @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void); > > > > #if defined(CONFIG_RISCV_MISALIGNED) > > > > bool check_unaligned_access_emulated_all_cpus(void); > > > > +bool check_vector_unaligned_access_all_cpus(void); > > > > void unaligned_emulation_finish(void); > > > > bool unaligned_ctl_available(void); > > > > DECLARE_PER_CPU(long, misaligned_access_speed); > > > > +DECLARE_PER_CPU(long, vector_misaligned_access); > > > > #else > > > > static inline bool unaligned_ctl_available(void) > > > > { > > > > diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h > > > > index 630507dff5ea..150a9877b0af 100644 > > > > --- a/arch/riscv/include/asm/hwprobe.h > > > > +++ b/arch/riscv/include/asm/hwprobe.h > > > > @@ -8,7 +8,7 @@ > > > > #include <uapi/asm/hwprobe.h> > > > > -#define RISCV_HWPROBE_MAX_KEY 6 > > > > +#define RISCV_HWPROBE_MAX_KEY 7 > > > > static inline bool riscv_hwprobe_key_is_valid(__s64 key) > > > > { > > > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > > > > index 731dcd0ed4de..776af9b37e23 100644 > > > > --- a/arch/riscv/include/asm/vector.h > > > > +++ b/arch/riscv/include/asm/vector.h > > > > @@ -21,6 +21,7 @@ > > > > extern unsigned long riscv_v_vsize; > > > > int riscv_v_setup_vsize(void); > > > > +bool insn_is_vector(u32 insn_buf); > > > > bool riscv_v_first_use_handler(struct pt_regs *regs); > > > > void kernel_vector_begin(void); > > > > void kernel_vector_end(void); > > > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h > > > > index 060212331a03..ebacff86f134 100644 > > > > --- a/arch/riscv/include/uapi/asm/hwprobe.h > > > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h > > > > @@ -68,6 +68,12 @@ struct riscv_hwprobe { > > > > #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) > > > > #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) > > > > #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6 > > > > +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF 7 > > > > +#define RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN 0 > > > > +#define RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED 1 > > > > +#define RISCV_HWPROBE_VEC_MISALIGNED_SLOW 2 > > > > +#define RISCV_HWPROBE_VEC_MISALIGNED_FAST 3 > > > > +#define RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED 4 > > > > /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */ > > > > /* Flags */ > > > > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c > > > > index b286b73e763e..ce641cc6e47a 100644 > > > > --- a/arch/riscv/kernel/sys_hwprobe.c > > > > +++ b/arch/riscv/kernel/sys_hwprobe.c > > > > @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus) > > > > } > > > > #endif > > > > +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS) > > > > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) > > > > +{ > > > > + int cpu; > > > > + u64 perf = -1ULL; > > > > + > > > > + for_each_cpu(cpu, cpus) { > > > > + int this_perf = per_cpu(vector_misaligned_access, cpu); > > > > + > > > > + if (perf == -1ULL) > > > > + perf = this_perf; > > > > + > > > > + if (perf != this_perf) { > > > > + perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > > > + break; > > > > + } > > > > + } > > > > + > > > > + if (perf == -1ULL) > > > > + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > > > + > > > > + return perf; > > > > +} > > > > +#else > > > > +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) > > > > +{ > > > > I meant to mention this in my last message! > > > > The scalar version has cutouts for configs here like: > > > > if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS)) > > return RISCV_HWPROBE_MISALIGNED_FAST; > > > > Will add > > > Having this functionality on vector as well would be much appreciated. > > I don't think it's valid to assume that vector and scalar have the same > > speed, so this would require a vector version of the RISCV_MISALIGNED > > tree in arch/riscv/Kconfig. > > > > - Charlie > > > > > > + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > > > +} > > > > +#endif > > > > + > > > > static void hwprobe_one_pair(struct riscv_hwprobe *pair, > > > > const struct cpumask *cpus) > > > > { > > > > @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, > > > > pair->value = hwprobe_misaligned(cpus); > > > > break; > > > > + case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF: > > > > + pair->value = hwprobe_vec_misaligned(cpus); > > > > + break; > > > > + > > > > case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE: > > > > pair->value = 0; > > > > if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ)) > > > > diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c > > > > index 2adb7c3e4dd5..8f26c3d92230 100644 > > > > --- a/arch/riscv/kernel/traps_misaligned.c > > > > +++ b/arch/riscv/kernel/traps_misaligned.c > > > > @@ -16,6 +16,7 @@ > > > > #include <asm/entry-common.h> > > > > #include <asm/hwprobe.h> > > > > #include <asm/cpufeature.h> > > > > +#include <asm/vector.h> > > > > #define INSN_MATCH_LB 0x3 > > > > #define INSN_MASK_LB 0x707f > > > > @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs) > > > > perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); > > > > -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > > > > - *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; > > > > -#endif > > > > - > > > > if (!unaligned_enabled) > > > > return -1; > > > > @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs) > > > > if (get_insn(regs, epc, &insn)) > > > > return -1; > > > > +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > > > > + if (insn_is_vector(insn) && > > > > + *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { > > > > + *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; > > > > + regs->epc = epc + INSN_LEN(insn); > > > > + return 0; > > There is a return before scalar speed is set. Aww I missed that. > > > > > + } > > > > + > > > > + *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; > > > > > > This unconditionally sets scalar unaligned accesses even if the > > > unaligned access is caused by vector. Scalar unaligned accesses should > > > only be set to emulated if this function is entered from a scalar > > > unaligned load. > > > > > > The rest of this function handles how scalar unaligned accesses are > > > emulated, and the equivalent needs to happen for vector. You need to add > > > routines that manually load the data from the memory address into the > > > vector register. When Clément did this for scalar, he provided a test > > > case to help reviewers [1]. Please add onto these test cases or make > > > your own for vector. > > I wansnt planing on adding emulation in this patch. I can if needed. Emulation is required. Right now vector unaligned accesses trap into this function and just return without setting the value in the vector register so the value in the vector register is incorrect but the program doesn't know because the trap is swallowed by this function. > > > > > > > Link: https://github.com/clementleger/unaligned_test [1] > > > > > > > +#endif > > > > + > > > > regs->epc = 0; > > > > if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { > > > > @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu) > > > > return misaligned_emu_detected; > > > > } > > > > +#ifdef CONFIG_RISCV_ISA_V > > > > +static void check_vector_unaligned_access(struct work_struct *unused) > > > > > > Can you standardize this name with the scalar version by writing > > > emulated in it? > > We dont emulate it so that wouldn't make sence. > > > > > > > "check_vector_unaligned_access_emulated_all_cpus" > > > > > > > +{ > > > > + int cpu = smp_processor_id(); > > > > + long *mas_ptr = this_cpu_ptr(&vector_misaligned_access); > > > > + unsigned long tmp_var; > > > > + > > > > + if (!riscv_isa_extension_available(hart_isa[cpu].isa, v)) > > > > + return; > > > > + > > > > + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > > > + > > > > + local_irq_enable(); > > > > + kernel_vector_begin(); > > > > + __asm__ __volatile__ ( > > > > + ".balign 4\n\t" > > > > + ".option push\n\t" > > > > + ".option arch, +v\n\t" > > > > + " vsetivli zero, 1, e16, m1, ta, ma\n\t" // Vectors of 16b > > > > + " vle16.v v0, (%[ptr])\n\t" // Load bytes > > > > + ".option pop\n\t" > > > > + : : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory"); > > > > > > memory is being read from, but not written to, so there is no need to > > > have a memory clobber. > > fixed. > > > > > > > > + kernel_vector_end(); > > > > + > > > > + if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN) > > > > + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > > > +} > > > > + > > > > +bool check_vector_unaligned_access_all_cpus(void) > > > > +{ > > > > + int cpu; > > > > + bool ret = true; > > > > + > > > > + for_each_online_cpu(cpu) > > > > + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) > > > > > > zicclsm is not specific to vector so it can be extracted out of this > > > vector specific function. Assuming that hardware properly reports the > > > extension, if zicclsm is present then it is known that both vector and > > > scalar unaligned accesses are supported. > > Added so we don't need to waste cycles testing support. > Yes exactly, that is why it should be added to the scalar version as well :) > > > > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > > > > > Please use the exising UNKNOWN terminology instead of renaming to > > > SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned > > > accesses are supported. > > Conor didnt like using UNKNOWN a proxy for "SUPPORTED" > Having SUPPORTED is better then assuing the speed to be slow. The HWPROBE key is about misaligned access performance. UNKNOWN means that the performance is unknown. The scalar and vector names need to match up. UNKNOWN was already merged and is supported by linux so if you want to use SUPPORTED here then you need to add a scalar SUPPORTED key that is an alias of the UNKNOWN key. I would rather keep UNKNOWN as it is, but that's up to you. > > > > > > > > + else > > > > + ret = false; > > > > + > > > > + > > > > + if (ret) > > > > + return true; > > > > + ret = true; > > > > + > > > > + schedule_on_each_cpu(check_vector_unaligned_access); > > > > + > > > > + for_each_online_cpu(cpu) > > > > + if (per_cpu(vector_misaligned_access, cpu) > > > > + != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) > > > > + return false; > > > > + > > > > + return ret; > > > > +} > > > > +#else > > > > > > If CONFIG_RISCV_ISA_V is not set, there is no value in checking if > > > vector unaligned accesses are supported because userspace will not be > > > allowed to use vector instructions anyway. > > Oh I'm silly meant to be seting to all UNSUPPORTED. > > > Thanks, > Jesse Taube > > > > > > > - Charlie > > > > > > > +bool check_vector_unaligned_access_all_cpus(void) > > > > +{ > > > > + int cpu; > > > > + > > > > + for_each_online_cpu(cpu) > > > > + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) > > > > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > > > + else > > > > + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; > > > > + > > > > + return false; > > > > +} > > > > +#endif > > > > + > > > > bool check_unaligned_access_emulated_all_cpus(void) > > > > { > > > > int cpu; > > > > diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c > > > > index a9a6bcb02acf..92a84239beaa 100644 > > > > --- a/arch/riscv/kernel/unaligned_access_speed.c > > > > +++ b/arch/riscv/kernel/unaligned_access_speed.c > > > > @@ -20,6 +20,7 @@ > > > > #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80) > > > > DEFINE_PER_CPU(long, misaligned_access_speed); > > > > +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; > > > > #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS > > > > static cpumask_t fast_misaligned_access; > > > > @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void) > > > > { > > > > bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); > > > > + check_vector_unaligned_access_all_cpus(); > > > > + > > > > if (!all_cpus_emulated) > > > > return check_unaligned_access_speed_all_cpus(); > > > > @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void) > > > > static int check_unaligned_access_all_cpus(void) > > > > { > > > > check_unaligned_access_emulated_all_cpus(); > > > > + check_vector_unaligned_access_all_cpus(); > > > > return 0; > > > > } > > > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > > > > index 6727d1d3b8f2..2cceab739b2c 100644 > > > > --- a/arch/riscv/kernel/vector.c > > > > +++ b/arch/riscv/kernel/vector.c > > > > @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void) > > > > #endif > > > > } > > > > -static bool insn_is_vector(u32 insn_buf) > > > > +bool insn_is_vector(u32 insn_buf) > > > > { > > > > u32 opcode = insn_buf & __INSN_OPCODE_MASK; > > > > u32 width, csr; > > > > -- > > > > 2.43.0 > > > >
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 <jesse@rivosinc.com> >> --- >> arch/riscv/include/asm/cpufeature.h | 2 + >> arch/riscv/include/asm/hwprobe.h | 2 +- >> arch/riscv/include/asm/vector.h | 1 + >> arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ >> arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ >> arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- >> arch/riscv/kernel/unaligned_access_speed.c | 4 ++ >> arch/riscv/kernel/vector.c | 2 +- >> 8 files changed, 129 insertions(+), 6 deletions(-) >> >> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h >> index 347805446151..a012c8490a27 100644 >> --- a/arch/riscv/include/asm/cpufeature.h >> +++ b/arch/riscv/include/asm/cpufeature.h >> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void); >> >> #if defined(CONFIG_RISCV_MISALIGNED) >> bool check_unaligned_access_emulated_all_cpus(void); >> +bool check_vector_unaligned_access_all_cpus(void); >> void unaligned_emulation_finish(void); >> bool unaligned_ctl_available(void); >> DECLARE_PER_CPU(long, misaligned_access_speed); >> +DECLARE_PER_CPU(long, vector_misaligned_access); >> #else >> static inline bool unaligned_ctl_available(void) >> { >> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h >> index 630507dff5ea..150a9877b0af 100644 >> --- a/arch/riscv/include/asm/hwprobe.h >> +++ b/arch/riscv/include/asm/hwprobe.h >> @@ -8,7 +8,7 @@ >> >> #include <uapi/asm/hwprobe.h> >> >> -#define RISCV_HWPROBE_MAX_KEY 6 >> +#define RISCV_HWPROBE_MAX_KEY 7 >> >> static inline bool riscv_hwprobe_key_is_valid(__s64 key) >> { >> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h >> index 731dcd0ed4de..776af9b37e23 100644 >> --- a/arch/riscv/include/asm/vector.h >> +++ b/arch/riscv/include/asm/vector.h >> @@ -21,6 +21,7 @@ >> >> extern unsigned long riscv_v_vsize; >> int riscv_v_setup_vsize(void); >> +bool insn_is_vector(u32 insn_buf); >> bool riscv_v_first_use_handler(struct pt_regs *regs); >> void kernel_vector_begin(void); >> void kernel_vector_end(void); >> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h >> index 060212331a03..ebacff86f134 100644 >> --- a/arch/riscv/include/uapi/asm/hwprobe.h >> +++ b/arch/riscv/include/uapi/asm/hwprobe.h >> @@ -68,6 +68,12 @@ struct riscv_hwprobe { >> #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) >> #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) >> #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6 >> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF 7 >> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN 0 >> +#define RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED 1 >> +#define RISCV_HWPROBE_VEC_MISALIGNED_SLOW 2 >> +#define RISCV_HWPROBE_VEC_MISALIGNED_FAST 3 >> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED 4 >> /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */ >> >> /* Flags */ >> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c >> index b286b73e763e..ce641cc6e47a 100644 >> --- a/arch/riscv/kernel/sys_hwprobe.c >> +++ b/arch/riscv/kernel/sys_hwprobe.c >> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus) >> } >> #endif >> >> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS) >> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >> +{ >> + int cpu; >> + u64 perf = -1ULL; >> + >> + for_each_cpu(cpu, cpus) { >> + int this_perf = per_cpu(vector_misaligned_access, cpu); >> + >> + if (perf == -1ULL) >> + perf = this_perf; >> + >> + if (perf != this_perf) { >> + perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >> + break; >> + } >> + } >> + >> + if (perf == -1ULL) >> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >> + >> + return perf; >> +} >> +#else >> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >> +{ >> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >> +} >> +#endif >> + >> static void hwprobe_one_pair(struct riscv_hwprobe *pair, >> const struct cpumask *cpus) >> { >> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, >> pair->value = hwprobe_misaligned(cpus); >> break; >> >> + case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF: >> + pair->value = hwprobe_vec_misaligned(cpus); >> + break; >> + >> case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE: >> pair->value = 0; >> if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ)) >> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c >> index 2adb7c3e4dd5..8f26c3d92230 100644 >> --- a/arch/riscv/kernel/traps_misaligned.c >> +++ b/arch/riscv/kernel/traps_misaligned.c >> @@ -16,6 +16,7 @@ >> #include <asm/entry-common.h> >> #include <asm/hwprobe.h> >> #include <asm/cpufeature.h> >> +#include <asm/vector.h> >> >> #define INSN_MATCH_LB 0x3 >> #define INSN_MASK_LB 0x707f >> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs) >> >> perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); >> >> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >> - *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; >> -#endif >> - >> if (!unaligned_enabled) >> return -1; >> >> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs) >> if (get_insn(regs, epc, &insn)) >> return -1; >> >> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >> + if (insn_is_vector(insn) && >> + *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { >> + *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >> + regs->epc = epc + INSN_LEN(insn); >> + return 0; >> + } >> + >> + *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; > > This unconditionally sets scalar unaligned accesses even if the > unaligned access is caused by vector. Scalar unaligned accesses should > only be set to emulated if this function is entered from a scalar > unaligned load. > > The rest of this function handles how scalar unaligned accesses are > emulated, and the equivalent needs to happen for vector. You need to add > routines that manually load the data from the memory address into the > vector register. When Clément did this for scalar, he provided a test > case to help reviewers [1]. Please add onto these test cases or make > your own for vector. > > Link: https://github.com/clementleger/unaligned_test [1] Hi Jesse, I already mentionned that in a previous message and got no answer [1]. Please address all feedback before sending another version. Adding a cover letter for such series would also be appreciated. Thanks, Clément Link :https://lore.kernel.org/all/2c355b76-c768-46b1-9f37 fd1991a54bfd@rivosinc.com/ [1] > >> +#endif >> + >> regs->epc = 0; >> >> if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { >> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu) >> return misaligned_emu_detected; >> } >> >> +#ifdef CONFIG_RISCV_ISA_V >> +static void check_vector_unaligned_access(struct work_struct *unused) > > Can you standardize this name with the scalar version by writing > emulated in it? > > "check_vector_unaligned_access_emulated_all_cpus" > >> +{ >> + int cpu = smp_processor_id(); >> + long *mas_ptr = this_cpu_ptr(&vector_misaligned_access); >> + unsigned long tmp_var; >> + >> + if (!riscv_isa_extension_available(hart_isa[cpu].isa, v)) >> + return; >> + >> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >> + >> + local_irq_enable(); >> + kernel_vector_begin(); >> + __asm__ __volatile__ ( >> + ".balign 4\n\t" >> + ".option push\n\t" >> + ".option arch, +v\n\t" >> + " vsetivli zero, 1, e16, m1, ta, ma\n\t" // Vectors of 16b >> + " vle16.v v0, (%[ptr])\n\t" // Load bytes >> + ".option pop\n\t" >> + : : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory"); > > memory is being read from, but not written to, so there is no need to > have a memory clobber. > >> + kernel_vector_end(); >> + >> + if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN) >> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >> +} >> + >> +bool check_vector_unaligned_access_all_cpus(void) >> +{ >> + int cpu; >> + bool ret = true; >> + >> + for_each_online_cpu(cpu) >> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) > > zicclsm is not specific to vector so it can be extracted out of this > vector specific function. Assuming that hardware properly reports the > extension, if zicclsm is present then it is known that both vector and > scalar unaligned accesses are supported. > >> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; > > Please use the exising UNKNOWN terminology instead of renaming to > SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned > accesses are supported. > >> + else >> + ret = false; >> + >> + >> + if (ret) >> + return true; >> + ret = true; >> + >> + schedule_on_each_cpu(check_vector_unaligned_access); >> + >> + for_each_online_cpu(cpu) >> + if (per_cpu(vector_misaligned_access, cpu) >> + != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) >> + return false; >> + >> + return ret; >> +} >> +#else > > If CONFIG_RISCV_ISA_V is not set, there is no value in checking if > vector unaligned accesses are supported because userspace will not be > allowed to use vector instructions anyway. > > - Charlie > >> +bool check_vector_unaligned_access_all_cpus(void) >> +{ >> + int cpu; >> + >> + for_each_online_cpu(cpu) >> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) >> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >> + else >> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >> + >> + return false; >> +} >> +#endif >> + >> bool check_unaligned_access_emulated_all_cpus(void) >> { >> int cpu; >> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c >> index a9a6bcb02acf..92a84239beaa 100644 >> --- a/arch/riscv/kernel/unaligned_access_speed.c >> +++ b/arch/riscv/kernel/unaligned_access_speed.c >> @@ -20,6 +20,7 @@ >> #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80) >> >> DEFINE_PER_CPU(long, misaligned_access_speed); >> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >> >> #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >> static cpumask_t fast_misaligned_access; >> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void) >> { >> bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); >> >> + check_vector_unaligned_access_all_cpus(); >> + >> if (!all_cpus_emulated) >> return check_unaligned_access_speed_all_cpus(); >> >> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void) >> static int check_unaligned_access_all_cpus(void) >> { >> check_unaligned_access_emulated_all_cpus(); >> + check_vector_unaligned_access_all_cpus(); >> >> return 0; >> } >> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c >> index 6727d1d3b8f2..2cceab739b2c 100644 >> --- a/arch/riscv/kernel/vector.c >> +++ b/arch/riscv/kernel/vector.c >> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void) >> #endif >> } >> >> -static bool insn_is_vector(u32 insn_buf) >> +bool insn_is_vector(u32 insn_buf) >> { >> u32 opcode = insn_buf & __INSN_OPCODE_MASK; >> u32 width, csr; >> -- >> 2.43.0 >>
On 6/10/24 04:23, Clément Léger wrote: > > > On 06/06/2024 23:29, Charlie Jenkins wrote: >> On Thu, Jun 06, 2024 at 02:32:14PM -0400, Jesse Taube wrote: >>> Run a unaligned vector access to test if the system supports >>> vector unaligned access. Add the result to a new key in hwprobe. >>> This is useful for usermode to know if vector misaligned accesses are >>> supported and if they are faster or slower than equivalent byte accesses. >>> >>> Signed-off-by: Jesse Taube <jesse@rivosinc.com> >>> --- >>> arch/riscv/include/asm/cpufeature.h | 2 + >>> arch/riscv/include/asm/hwprobe.h | 2 +- >>> arch/riscv/include/asm/vector.h | 1 + >>> arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ >>> arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ >>> arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- >>> arch/riscv/kernel/unaligned_access_speed.c | 4 ++ >>> arch/riscv/kernel/vector.c | 2 +- >>> 8 files changed, 129 insertions(+), 6 deletions(-) >>> >>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h >>> index 347805446151..a012c8490a27 100644 >>> --- a/arch/riscv/include/asm/cpufeature.h >>> +++ b/arch/riscv/include/asm/cpufeature.h >>> @@ -35,9 +35,11 @@ void riscv_user_isa_enable(void); >>> >>> #if defined(CONFIG_RISCV_MISALIGNED) >>> bool check_unaligned_access_emulated_all_cpus(void); >>> +bool check_vector_unaligned_access_all_cpus(void); >>> void unaligned_emulation_finish(void); >>> bool unaligned_ctl_available(void); >>> DECLARE_PER_CPU(long, misaligned_access_speed); >>> +DECLARE_PER_CPU(long, vector_misaligned_access); >>> #else >>> static inline bool unaligned_ctl_available(void) >>> { >>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h >>> index 630507dff5ea..150a9877b0af 100644 >>> --- a/arch/riscv/include/asm/hwprobe.h >>> +++ b/arch/riscv/include/asm/hwprobe.h >>> @@ -8,7 +8,7 @@ >>> >>> #include <uapi/asm/hwprobe.h> >>> >>> -#define RISCV_HWPROBE_MAX_KEY 6 >>> +#define RISCV_HWPROBE_MAX_KEY 7 >>> >>> static inline bool riscv_hwprobe_key_is_valid(__s64 key) >>> { >>> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h >>> index 731dcd0ed4de..776af9b37e23 100644 >>> --- a/arch/riscv/include/asm/vector.h >>> +++ b/arch/riscv/include/asm/vector.h >>> @@ -21,6 +21,7 @@ >>> >>> extern unsigned long riscv_v_vsize; >>> int riscv_v_setup_vsize(void); >>> +bool insn_is_vector(u32 insn_buf); >>> bool riscv_v_first_use_handler(struct pt_regs *regs); >>> void kernel_vector_begin(void); >>> void kernel_vector_end(void); >>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h >>> index 060212331a03..ebacff86f134 100644 >>> --- a/arch/riscv/include/uapi/asm/hwprobe.h >>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h >>> @@ -68,6 +68,12 @@ struct riscv_hwprobe { >>> #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) >>> #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) >>> #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6 >>> +#define RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF 7 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN 0 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED 1 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_SLOW 2 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_FAST 3 >>> +#define RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED 4 >>> /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */ >>> >>> /* Flags */ >>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c >>> index b286b73e763e..ce641cc6e47a 100644 >>> --- a/arch/riscv/kernel/sys_hwprobe.c >>> +++ b/arch/riscv/kernel/sys_hwprobe.c >>> @@ -184,6 +184,36 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus) >>> } >>> #endif >>> >>> +#if defined(CONFIG_RISCV_PROBE_UNALIGNED_ACCESS) >>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >>> +{ >>> + int cpu; >>> + u64 perf = -1ULL; >>> + >>> + for_each_cpu(cpu, cpus) { >>> + int this_perf = per_cpu(vector_misaligned_access, cpu); >>> + >>> + if (perf == -1ULL) >>> + perf = this_perf; >>> + >>> + if (perf != this_perf) { >>> + perf = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> + break; >>> + } >>> + } >>> + >>> + if (perf == -1ULL) >>> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> + >>> + return perf; >>> +} >>> +#else >>> +static u64 hwprobe_vec_misaligned(const struct cpumask *cpus) >>> +{ >>> + return RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> +} >>> +#endif >>> + >>> static void hwprobe_one_pair(struct riscv_hwprobe *pair, >>> const struct cpumask *cpus) >>> { >>> @@ -211,6 +241,10 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair, >>> pair->value = hwprobe_misaligned(cpus); >>> break; >>> >>> + case RISCV_HWPROBE_VEC_KEY_MISALIGNED_PERF: >>> + pair->value = hwprobe_vec_misaligned(cpus); >>> + break; >>> + >>> case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE: >>> pair->value = 0; >>> if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ)) >>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c >>> index 2adb7c3e4dd5..8f26c3d92230 100644 >>> --- a/arch/riscv/kernel/traps_misaligned.c >>> +++ b/arch/riscv/kernel/traps_misaligned.c >>> @@ -16,6 +16,7 @@ >>> #include <asm/entry-common.h> >>> #include <asm/hwprobe.h> >>> #include <asm/cpufeature.h> >>> +#include <asm/vector.h> >>> >>> #define INSN_MATCH_LB 0x3 >>> #define INSN_MASK_LB 0x707f >>> @@ -413,10 +414,6 @@ int handle_misaligned_load(struct pt_regs *regs) >>> >>> perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); >>> >>> -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>> - *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; >>> -#endif >>> - >>> if (!unaligned_enabled) >>> return -1; >>> >>> @@ -426,6 +423,17 @@ int handle_misaligned_load(struct pt_regs *regs) >>> if (get_insn(regs, epc, &insn)) >>> return -1; >>> >>> +#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>> + if (insn_is_vector(insn) && >>> + *this_cpu_ptr(&vector_misaligned_access) == RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) { >>> + *this_cpu_ptr(&vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >>> + regs->epc = epc + INSN_LEN(insn); >>> + return 0; >>> + } I'm going to add + /* If vector instruction we don't emulate it yet */ + if (insn_is_vector(insn)) { + regs->epc = epc; + return -1; + } >>> + >>> + *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED; >> >> This unconditionally sets scalar unaligned accesses even if the >> unaligned access is caused by vector. Scalar unaligned accesses should >> only be set to emulated if this function is entered from a scalar >> unaligned load. >> Im sorry I missunderstood what you were reffering to. The check above does return, but you are saying when a misaligned vector does come here. If a misaligned vector load trap does happen when we arent checking it will still cause a fault as `vector_misaligned_access != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED`. Basicaly the return 0 above will only ever be posible duing check_vector_unaligned_access. >> The rest of this function handles how scalar unaligned accesses are >> emulated, and the equivalent needs to happen for vector. I am hesitant to do so because I have very little knowlage of how RVV works. It will maintain the previous behavoir of trapping if VEC_MISALIGNED_UNSUPPORTED. > You need to add >> routines that manually load the data from the memory address into the >> vector register. When Clément did this for scalar, he provided a test >> case to help reviewers [1]. Please add onto these test cases or make >> your own for vector. >> >> Link: https://github.com/clementleger/unaligned_test [1] I will add a test. > > Hi Jesse, > > I already mentionned that in a previous message and got no answer [1]. > Please address all feedback before sending another version. Adding a > cover letter for such series would also be appreciated. Sorry I didnt understand and thought your question was the same as charlies. Thanks, Jesse Taube > > Thanks, > > Clément > > Link :https://lore.kernel.org/all/2c355b76-c768-46b1-9f37 > fd1991a54bfd@rivosinc.com/ [1] > >> >>> +#endif >>> + >>> regs->epc = 0; >>> >>> if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) { >>> @@ -625,6 +633,74 @@ static bool check_unaligned_access_emulated(int cpu) >>> return misaligned_emu_detected; >>> } >>> >>> +#ifdef CONFIG_RISCV_ISA_V >>> +static void check_vector_unaligned_access(struct work_struct *unused) >> >> Can you standardize this name with the scalar version by writing >> emulated in it? >> >> "check_vector_unaligned_access_emulated_all_cpus" >> >>> +{ >>> + int cpu = smp_processor_id(); >>> + long *mas_ptr = this_cpu_ptr(&vector_misaligned_access); >>> + unsigned long tmp_var; >>> + >>> + if (!riscv_isa_extension_available(hart_isa[cpu].isa, v)) >>> + return; >>> + >>> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>> + >>> + local_irq_enable(); >>> + kernel_vector_begin(); >>> + __asm__ __volatile__ ( >>> + ".balign 4\n\t" >>> + ".option push\n\t" >>> + ".option arch, +v\n\t" >>> + " vsetivli zero, 1, e16, m1, ta, ma\n\t" // Vectors of 16b >>> + " vle16.v v0, (%[ptr])\n\t" // Load bytes >>> + ".option pop\n\t" >>> + : : [ptr] "r" ((u8 *)&tmp_var + 1) : "v0", "memory"); >> >> memory is being read from, but not written to, so there is no need to >> have a memory clobber. >> >>> + kernel_vector_end(); >>> + >>> + if (*mas_ptr == RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN) >>> + *mas_ptr = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>> +} >>> + >>> +bool check_vector_unaligned_access_all_cpus(void) >>> +{ >>> + int cpu; >>> + bool ret = true; >>> + >>> + for_each_online_cpu(cpu) >>> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) >> >> zicclsm is not specific to vector so it can be extracted out of this >> vector specific function. Assuming that hardware properly reports the >> extension, if zicclsm is present then it is known that both vector and >> scalar unaligned accesses are supported. >> >>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >> >> Please use the exising UNKNOWN terminology instead of renaming to >> SUPPORTED. Any option that is not UNSUPPORTED implies that unaligned >> accesses are supported. >> >>> + else >>> + ret = false; >>> + >>> + >>> + if (ret) >>> + return true; >>> + ret = true; >>> + >>> + schedule_on_each_cpu(check_vector_unaligned_access); >>> + >>> + for_each_online_cpu(cpu) >>> + if (per_cpu(vector_misaligned_access, cpu) >>> + != RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED) >>> + return false; >>> + >>> + return ret; >>> +} >>> +#else >> >> If CONFIG_RISCV_ISA_V is not set, there is no value in checking if >> vector unaligned accesses are supported because userspace will not be >> allowed to use vector instructions anyway. >> >> - Charlie >> >>> +bool check_vector_unaligned_access_all_cpus(void) >>> +{ >>> + int cpu; >>> + >>> + for_each_online_cpu(cpu) >>> + if (riscv_isa_extension_available(hart_isa[cpu].isa, ZICCLSM)) >>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_SUPPORTED; >>> + else >>> + per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_VEC_MISALIGNED_UNSUPPORTED; >>> + >>> + return false; >>> +} >>> +#endif >>> + >>> bool check_unaligned_access_emulated_all_cpus(void) >>> { >>> int cpu; >>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c >>> index a9a6bcb02acf..92a84239beaa 100644 >>> --- a/arch/riscv/kernel/unaligned_access_speed.c >>> +++ b/arch/riscv/kernel/unaligned_access_speed.c >>> @@ -20,6 +20,7 @@ >>> #define MISALIGNED_COPY_SIZE ((MISALIGNED_BUFFER_SIZE / 2) - 0x80) >>> >>> DEFINE_PER_CPU(long, misaligned_access_speed); >>> +DEFINE_PER_CPU(long, vector_misaligned_access) = RISCV_HWPROBE_VEC_MISALIGNED_UNKNOWN; >>> >>> #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS >>> static cpumask_t fast_misaligned_access; >>> @@ -264,6 +265,8 @@ static int check_unaligned_access_all_cpus(void) >>> { >>> bool all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); >>> >>> + check_vector_unaligned_access_all_cpus(); >>> + >>> if (!all_cpus_emulated) >>> return check_unaligned_access_speed_all_cpus(); >>> >>> @@ -273,6 +276,7 @@ static int check_unaligned_access_all_cpus(void) >>> static int check_unaligned_access_all_cpus(void) >>> { >>> check_unaligned_access_emulated_all_cpus(); >>> + check_vector_unaligned_access_all_cpus(); >>> >>> return 0; >>> } >>> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c >>> index 6727d1d3b8f2..2cceab739b2c 100644 >>> --- a/arch/riscv/kernel/vector.c >>> +++ b/arch/riscv/kernel/vector.c >>> @@ -66,7 +66,7 @@ void __init riscv_v_setup_ctx_cache(void) >>> #endif >>> } >>> >>> -static bool insn_is_vector(u32 insn_buf) >>> +bool insn_is_vector(u32 insn_buf) >>> { >>> u32 opcode = insn_buf & __INSN_OPCODE_MASK; >>> u32 width, csr; >>> -- >>> 2.43.0 >>>
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;
Run a unaligned vector access to test if the system supports vector unaligned access. Add the result to a new key in hwprobe. This is useful for usermode to know if vector misaligned accesses are supported and if they are faster or slower than equivalent byte accesses. Signed-off-by: Jesse Taube <jesse@rivosinc.com> --- arch/riscv/include/asm/cpufeature.h | 2 + arch/riscv/include/asm/hwprobe.h | 2 +- arch/riscv/include/asm/vector.h | 1 + arch/riscv/include/uapi/asm/hwprobe.h | 6 ++ arch/riscv/kernel/sys_hwprobe.c | 34 +++++++++ arch/riscv/kernel/traps_misaligned.c | 84 ++++++++++++++++++++-- arch/riscv/kernel/unaligned_access_speed.c | 4 ++ arch/riscv/kernel/vector.c | 2 +- 8 files changed, 129 insertions(+), 6 deletions(-)