Messages in this thread | | | From | Evan Green <> | Date | Tue, 27 Feb 2024 10:48:39 -0800 | Subject | Re: [PATCH v4 2/2] riscv: Set unalignment speed at compile time |
| |
On Tue, Feb 27, 2024 at 10:17 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > On Tue, Feb 27, 2024 at 11:39:25AM +0000, Conor Dooley wrote: > > On Fri, Feb 16, 2024 at 12:33:19PM -0800, Charlie Jenkins wrote: > > > Introduce Kconfig options to set the kernel unaligned access support. > > > These options provide a non-portable alternative to the runtime > > > unaligned access probe. > > > > > > To support this, the unaligned access probing code is moved into it's > > > own file and gated behind a new RISCV_PROBE_UNALIGNED_ACCESS_SUPPORT > > > option. > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > --- > > > arch/riscv/Kconfig | 58 +++++- > > > arch/riscv/include/asm/cpufeature.h | 30 +++- > > > arch/riscv/kernel/Makefile | 6 +- > > > arch/riscv/kernel/cpufeature.c | 255 -------------------------- > > > arch/riscv/kernel/misaligned_access_speed.c | 265 ++++++++++++++++++++++++++++ > > > arch/riscv/kernel/probe_emulated_access.c | 64 +++++++ > > > arch/riscv/kernel/sys_hwprobe.c | 25 +++ > > > arch/riscv/kernel/traps_misaligned.c | 54 +----- > > > 8 files changed, 442 insertions(+), 315 deletions(-) > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > index bffbd869a068..3cf700adc43b 100644 > > > --- a/arch/riscv/Kconfig > > > +++ b/arch/riscv/Kconfig > > > @@ -690,25 +690,71 @@ config THREAD_SIZE_ORDER > > > config RISCV_MISALIGNED > > > > > > Why can we not make up our minds on what to call this? The majority of > > users are "unaligned" but the file you add and this config option are > > "misaligned." > > We have both everywhere, maybe we (I?) should go in and standardize the > wording everywhere. I personally prefer "misaligned" which means > "incorrectly aligned" over "unaligned" which means "not aligned" because > a 7-bit alignment is still "aligned" along a 7-bit boundary, but it is > certainly incorrectly aligned. > > > > > > bool "Support misaligned load/store traps for kernel and userspace" > > > select SYSCTL_ARCH_UNALIGN_ALLOW > > > + depends on RISCV_PROBE_UNALIGNED_ACCESS || RISCV_EMULATED_UNALIGNED_ACCESS > > > default y > > > help > > > Say Y here if you want the kernel to embed support for misaligned > > > load/store for both kernel and userspace. When disable, misaligned > > > accesses will generate SIGBUS in userspace and panic in kernel. > > > > > > +choice > > > + prompt "Unaligned Accesses Support" > > > + default RISCV_PROBE_UNALIGNED_ACCESS > > > + help > > > + This selects the hardware support for unaligned accesses. This > > > + information is used by the kernel to perform optimizations. It is also > > > + exposed to user space via the hwprobe syscall. The hardware will be > > > + probed at boot by default. > > > + > > > +config RISCV_PROBE_UNALIGNED_ACCESS > > > + bool "Probe for hardware unaligned access support" > > > + help > > > + During boot, the kernel will run a series of tests to determine the > > > + speed of unaligned accesses. This is the only portable option. This > > > + probing will dynamically determine the speed of unaligned accesses on > > > + the boot hardware. > > > + > > > +config RISCV_EMULATED_UNALIGNED_ACCESS > > > + bool "Assume the CPU expects emulated unaligned memory accesses" > > > + depends on NONPORTABLE > > > > This is portable too, right? > > I guess so? I think I would prefer to have the probing being the only > portable option. > > > > > > + select RISCV_MISALIGNED > > > + help > > > + Assume that the CPU expects emulated unaligned memory accesses. > > > + When enabled, this option notifies the kernel and userspace that > > > + unaligned memory accesses will be emulated by the kernel. > > > > > To enforce > > > + this expectation, RISCV_MISALIGNED is selected by this option. > > > > Drop this IMO, let Kconfig handle displaying the dependencies. > > > > I was debating if Kconfig handling was enough, so I am glad it is, I > will remove this. > > > > + > > > +config RISCV_SLOW_UNALIGNED_ACCESS > > > + bool "Assume the CPU supports slow unaligned memory accesses" > > > + depends on NONPORTABLE > > > + help > > > + Assume that the CPU supports slow unaligned memory accesses. When > > > + enabled, this option improves the performance of the kernel on such > > > + CPUs. > > > > Does it? Are you sure that generating unaligned accesses on systems > > where they are slow is a performance increase? > > That said, I don't really see this option actually doing anything other > > than setting the value for hwprobe, so I don't actually know what the > > effect of this option actually is on the kernel's performance. > > > > Generally I would like to suggest a change from "CPU" to "system" here, > > since the slow cases that exist are mostly because the unaligned access > > is actually emulated in firmware. > > It would be ideal if "emulated" was used for any case of emulated > accesses (firmware or in the kernel). Doing emulated accesses will be > orders of magnitude slower than a processor that "slowly" handles the > accesses. > > So even if the processor performs a "slow" access, it could still be > beneficial for the kernel to do the misaligned access rather than manual > do the alignment. > > Currently there is no place that takes into account this "slow" option > but I wanted to leave it open for future optimizations. > > > > > > However, the kernel will run much more slowly, or will not be > > > + able to run at all, on CPUs that do not support unaligned memory > > > + accesses. > > > + > > > config RISCV_EFFICIENT_UNALIGNED_ACCESS > > > bool "Assume the CPU supports fast unaligned memory accesses" > > > depends on NONPORTABLE > > > select DCACHE_WORD_ACCESS if MMU > > > select HAVE_EFFICIENT_UNALIGNED_ACCESS > > > help > > > - Say Y here if you want the kernel to assume that the CPU supports > > > - efficient unaligned memory accesses. When enabled, this option > > > - improves the performance of the kernel on such CPUs. However, the > > > - kernel will run much more slowly, or will not be able to run at all, > > > - on CPUs that do not support efficient unaligned memory accesses. > > > + Assume that the CPU supports fast unaligned memory accesses. When > > > + enabled, this option improves the performance of the kernel on such > > > + CPUs. However, the kernel will run much more slowly, or will not be > > > + able to run at all, on CPUs that do not support efficient unaligned > > > + memory accesses. > > > + > > > +config RISCV_UNSUPPORTED_UNALIGNED_ACCESS > > > > This option needs to be removed. The uabi states that unaligned access > > is supported in userspace, if the cpu or firmware does not implement > > unaligned access then the kernel must emulate it. > > Should it removed from hwprobe as well then?
We had added it as a hwprobe value in this discussion: https://lore.kernel.org/lkml/Y+1VOXyKDDHEuejJ@spud/
Personally I like it as a possible hwprobe value, even if it is in conflict with the uabi. I can't fully defend it, other than as a very forward looking possibility, and as a nice value for people doing weird things off the beaten path. My preference would be to keep it in hwprobe, but I'm fine with not having a Kconfig for it.
-Evan
| |