Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic functions
From: Palmer Dabbelt
Date: Wed Feb 05 2025 - 10:50:07 EST
On Mon, 03 Feb 2025 13:34:48 PST (-0800), jrtc27@xxxxxxxxxx wrote:
On 3 Feb 2025, at 19:50, Charlie Jenkins <charlie@xxxxxxxxxxxx> wrote:
On Mon, Feb 03, 2025 at 01:30:48PM -0600, Samuel Holland wrote:
Hi Charlie,
On 2025-02-03 1:12 PM, Charlie Jenkins wrote:
On Sun, Feb 02, 2025 at 08:08:50PM +0000, Conor Dooley wrote:
On Sat, Feb 01, 2025 at 01:04:25PM +0100, Aleksandar Rikalo wrote:
On Fri, Jan 10, 2025 at 4:23 AM Charlie Jenkins <charlie@xxxxxxxxxxxx> wrote:
From: Chao-ying Fu <cfu@xxxxxxxx>
Use only LR/SC instructions to implement atomic functions.
In the previous patch you mention that this is to support MIPS P8700. Can
you expand on why this change is required? The datasheet at [1] says:
"The P8700 core is configured to support the RV64GCZba_Zbb (G = IMAFD)
Standard ISA. It includes the RV64I base ISA, Multiply (M), Atomic (A),
Single-Precision Floating Point (F), Double (D), Compressed (C) RISC-V
extensions, as well as the as well as the bit-manipulation extensions
(Zba) and (Zbb)"
The "A" extension is a part of "G" which is mostly assumed to exist in
the kernel. Additionally, having this be a compilation flag will cause
traps on generic kernels. We generally try to push everything we can
into runtime feature detection since there are so many possible variants
of riscv.
Expressing not being able to perform a feature like this is normally
better expressed as an errata. Then generic kernels will be able to
include this, and anybody who doesn't want to have the extra nops
introduced can disable the errata. A similar approach to what I pointed
out last time should work here too (but with more places to replace)
[2].
[1] https://mips.com/wp-content/uploads/2024/11/P8700_Data_Sheet.pdf
[2] https://lore.kernel.org/lkml/Z2-UNfwcAQYZqVBU@ghost/T/
So far we haven't found a way to do this using errata.
You mean using alternatives? Not implementing A, but instead
implementing Zalrsc, is not an erratum. It's a design decision.
We could do the same thing we do with misaligned access detection and
run some instructions to determine if these instructions are being
emulated. If they are being emulated, patch all of the places to use
zalrsc.
Is the implication here that the riscv,isa-extensions list passed to the kernel
will contain "a", even though the hardware does not support it, because AMOs are
emulated in M-mode?
If that is not the case, there is no need for runtime detection. The alternative
entry can check RISCV_ISA_EXT_ZAAMO (which would be implied by RISCV_ISA_EXT_a)
in the ISA bitmap like normal.
That would be much better! I was under the assumption that the usecase
for this patch was that they were passing in "a" and wanting to only get
zalrsc. We should be able to check
RISCV_ISA_EXT_ZAAMO/RISCV_ISA_EXT_ZALRSC to get the information without
runtime detection.
In LLVM at least -mcpu=mips-p8700 enables the full A extension...
So then I think we need some help from the HW vendor here.
Specifically: do these systems implement A via M-mode traps (ie, with a
performance penalty) or is there something functional erratum in the A
implementation.
If it's just a performance thing then we need some benchmark justifying
the extra work, which means some hardware we can point at to run the
benchmark. Probably also best to shim this in via alternatives, so we
can keep multi-vendor kernels working.
If it's a funtional issue then we need to know what's actually broken.
Either way this should be exposed to userspace.
Jess