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

[Samuel Holland]

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.

Regards,
Samuel