Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic functions

[Charlie Jenkins <charlie@rivosinc.com>]

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@rivosinc.com> wrote:
> >>>
> >>>>> From: Chao-ying Fu <cfu@mips.com>
> >>>>>
> >>>>> 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.

- Charlie

> 
> Regards,
> Samuel
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv