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

[Jessica Clarke]

On 2 Feb 2025, at 20:08, Conor Dooley <conor@xxxxxxxxxx> 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.
> 
>> There's no way
>> to patch one instruction with multiple ones,
> 
> Have you looked at how the alternatives work? There's no requirement
> that the sequences have the same number of instructions, padding is
> allowed.
> 
>> and we also need an extra
>> (temporary) register.
>> 
>> A CPU can implement the Zalrsc extension [1] instead of "complete A"
>> (which P8700 does).
>>> From "Zaamo and Zalrsc Extensions" spec:
> 
> Why does this statement differ from the P8700 datasheet linked above?

In an LLVM patch several weeks ago it was said that Zaamo is
implemented in firmware by trapping and emulating with LR/SC.

Jess