Re: [PATCH v4 4/6] RISC-V: hwprobe: Support probing of misaligned access performance

Heiko Stübner
2023-03-17T10:09:16Z

Hi Evan,

Am Dienstag, 14. März 2023, 19:32:18 CET schrieb Evan Green:
> This allows userspace to select various routines to use based on the
> performance of misaligned access on the target hardware.

I really like this implementation.

Also interesting that T-Head actually has a fast unaligned access.
Maybe that should be part of the commit message (including were
this information comes from)

> Co-developed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> Signed-off-by: Evan Green <evan@rivosinc.com>
>
> ---
>
> Changes in v4:
> - Add newlines to CPUPERF_0 documentation (Conor)
> - Add UNSUPPORTED value (Conor)
> - Switched from DT to alternatives-based probing (Rob)
> - Crispen up cpu index type to always be int (Conor)
>
> Changes in v3:
> - Have hwprobe_misaligned return int instead of long.
> - Constify cpumask pointer in hwprobe_misaligned()
> - Fix warnings in _PERF_O list documentation, use :c:macro:.
> - Move include cpufeature.h to misaligned patch.
> - Fix documentation mismatch for RISCV_HWPROBE_KEY_CPUPERF_0 (Conor)
> - Use for_each_possible_cpu() instead of NR_CPUS (Conor)
> - Break early in misaligned access iteration (Conor)
> - Increase MISALIGNED_MASK from 2 bits to 3 for possible UNSUPPORTED future
> value (Conor)
>
> Changes in v2:
> - Fixed logic error in if(of_property_read_string...) that caused crash
> - Include cpufeature.h in cpufeature.h to avoid undeclared variable
> warning.
> - Added a _MASK define
> - Fix random checkpatch complaints
>
> Documentation/riscv/hwprobe.rst | 21 ++++++++++++++++++++
> arch/riscv/errata/thead/errata.c | 9 +++++++++
> arch/riscv/include/asm/alternative.h | 5 +++++
> arch/riscv/include/asm/cpufeature.h | 2 ++
> arch/riscv/include/asm/hwprobe.h | 2 +-
> arch/riscv/include/uapi/asm/hwprobe.h | 7 +++++++
> arch/riscv/kernel/alternative.c | 19 ++++++++++++++++++
> arch/riscv/kernel/cpufeature.c | 3 +++
> arch/riscv/kernel/smpboot.c | 1 +
> arch/riscv/kernel/sys_riscv.c | 28 +++++++++++++++++++++++++++
> 10 files changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
> index 945d44683c40..9f0dd62dcb5d 100644
> --- a/Documentation/riscv/hwprobe.rst
> +++ b/Documentation/riscv/hwprobe.rst
> @@ -63,3 +63,24 @@ The following keys are defined:
>
> * :c:macro:`RISCV_HWPROBE_IMA_C`: The C extension is supported, as defined
> by version 2.2 of the RISC-V ISA manual.
> +
> +* :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
> + information about the selected set of processors.
> +
> + * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
> + accesses is unknown.
> +
> + * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
> + emulated via software, either in or below the kernel. These accesses are
> + always extremely slow.
> +
> + * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned accesses are supported
> + in hardware, but are slower than the cooresponding aligned accesses
> + sequences.
> +
> + * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned accesses are supported
> + in hardware and are faster than the cooresponding aligned accesses
> + sequences.
> +
> + * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
> + not supported at all and will generate a misaligned address fault.
> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> index fac5742d1c1e..f41a45af5607 100644
> --- a/arch/riscv/errata/thead/errata.c
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -10,7 +10,9 @@
> #include <linux/uaccess.h>
> #include <asm/alternative.h>
> #include <asm/cacheflush.h>
> +#include <asm/cpufeature.h>
> #include <asm/errata_list.h>
> +#include <asm/hwprobe.h>
> #include <asm/patch.h>
> #include <asm/vendorid_list.h>
>
> @@ -108,3 +110,10 @@ void __init_or_module thead_errata_patch_func(struct alt_entry *begin, struct al
> if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> local_flush_icache_all();
> }
> +
> +void thead_feature_probe_func(unsigned int cpu, unsigned long archid,
> + unsigned long impid)
> +{
> + if ((archid == 0) && (impid == 0))
> + per_cpu(misaligned_access_speed, cpu) = RISCV_HWPROBE_MISALIGNED_FAST;

When looking at this function I 'm wondering if we also want to expose
the active erratas somehow (not in this patch of course, just in general)

Heiko