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

[Djordje Todorovic]

HTEC Proprietary

Hi Alex,

> I read the previous comments and everyone seems to agree that an
> implementation using an alternative is the way to go, so can you come
> with a new version doing that?

Yes, sure, we are working on that. I wanted to confirm that it is the way
to go.

Thanks,
Djordje

________________________________________
From: Alexandre Ghiti <alex@xxxxxxxx>
Sent: Tuesday, March 25, 2025 9:58 AM
To: Djordje Todorovic <Djordje.Todorovic@xxxxxxxxxxxxx>; Palmer Dabbelt <palmer@xxxxxxxxxxx>; jrtc27@xxxxxxxxxx <jrtc27@xxxxxxxxxx>
Cc: Charlie Jenkins <charlie@xxxxxxxxxxxx>; samuel.holland@xxxxxxxxxx <samuel.holland@xxxxxxxxxx>; Conor Dooley <conor@xxxxxxxxxx>; arikalo@xxxxxxxxx <arikalo@xxxxxxxxx>; linux-riscv@xxxxxxxxxxxxxxxxxxx <linux-riscv@xxxxxxxxxxxxxxxxxxx>; Paul Walmsley <paul.walmsley@xxxxxxxxxx>; aou@xxxxxxxxxxxxxxxxx <aou@xxxxxxxxxxxxxxxxx>; Will Deacon <will@xxxxxxxxxx>; peterz@xxxxxxxxxxxxx <peterz@xxxxxxxxxxxxx>; boqun.feng@xxxxxxxxx <boqun.feng@xxxxxxxxx>; Mark Rutland <mark.rutland@xxxxxxx>; yury.norov@xxxxxxxxx <yury.norov@xxxxxxxxx>; linux@xxxxxxxxxxxxxxxxxx <linux@xxxxxxxxxxxxxxxxxx>; parri.andrea@xxxxxxxxx <parri.andrea@xxxxxxxxx>; leobras@xxxxxxxxxx <leobras@xxxxxxxxxx>; guoren@xxxxxxxxxx <guoren@xxxxxxxxxx>; ericchancf@xxxxxxxxxx <ericchancf@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic functions

[You don't often get email from alex@xxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.


Hi Djordje,

On 19/03/2025 13:28, Djordje Todorovic wrote:
> HTEC Public
>
> Hi everyone,
>
> Thank you a lot for your comments.
> I am wondering how we should proceed from here.


I read the previous comments and everyone seems to agree that an
implementation using an alternative is the way to go, so can you come
with a new version doing that?

Thanks,

Alex


>
> Best,
> Djordje
>
> ________________________________________
> From: Djordje Todorovic <Djordje.Todorovic@xxxxxxxxxxxxx>
> Sent: Wednesday, February 12, 2025 11:22 AM
> To: Palmer Dabbelt <palmer@xxxxxxxxxxx>; jrtc27@xxxxxxxxxx <jrtc27@xxxxxxxxxx>
> Cc: Charlie Jenkins <charlie@xxxxxxxxxxxx>; samuel.holland@xxxxxxxxxx <samuel.holland@xxxxxxxxxx>; Conor Dooley <conor@xxxxxxxxxx>; arikalo@xxxxxxxxx <arikalo@xxxxxxxxx>; linux-riscv@xxxxxxxxxxxxxxxxxxx <linux-riscv@xxxxxxxxxxxxxxxxxxx>; Paul Walmsley <paul.walmsley@xxxxxxxxxx>; aou@xxxxxxxxxxxxxxxxx <aou@xxxxxxxxxxxxxxxxx>; Will Deacon <will@xxxxxxxxxx>; peterz@xxxxxxxxxxxxx <peterz@xxxxxxxxxxxxx>; boqun.feng@xxxxxxxxx <boqun.feng@xxxxxxxxx>; Mark Rutland <mark.rutland@xxxxxxx>; yury.norov@xxxxxxxxx <yury.norov@xxxxxxxxx>; linux@xxxxxxxxxxxxxxxxxx <linux@xxxxxxxxxxxxxxxxxx>; parri.andrea@xxxxxxxxx <parri.andrea@xxxxxxxxx>; leobras@xxxxxxxxxx <leobras@xxxxxxxxxx>; guoren@xxxxxxxxxx <guoren@xxxxxxxxxx>; ericchancf@xxxxxxxxxx <ericchancf@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic functions
>
> Hi,
>
> The official product page is located here:
> https://mips.com/products/hardware/p8700
>
> The mechanism we use to handle AMO is described in:
> "7.3.10 MIPS Configuration 6 Register (mipsconfig6)"
>
> Best,
> Djordje
>
> ________________________________________
> From: Palmer Dabbelt <palmer@xxxxxxxxxxx>
> Sent: Wednesday, February 5, 2025 4:49 PM
> To: jrtc27@xxxxxxxxxx <jrtc27@xxxxxxxxxx>
> Cc: Charlie Jenkins <charlie@xxxxxxxxxxxx>; samuel.holland@xxxxxxxxxx <samuel.holland@xxxxxxxxxx>; Conor Dooley <conor@xxxxxxxxxx>; arikalo@xxxxxxxxx <arikalo@xxxxxxxxx>; linux-riscv@xxxxxxxxxxxxxxxxxxx <linux-riscv@xxxxxxxxxxxxxxxxxxx>; Paul Walmsley <paul.walmsley@xxxxxxxxxx>; aou@xxxxxxxxxxxxxxxxx <aou@xxxxxxxxxxxxxxxxx>; Will Deacon <will@xxxxxxxxxx>; peterz@xxxxxxxxxxxxx <peterz@xxxxxxxxxxxxx>; boqun.feng@xxxxxxxxx <boqun.feng@xxxxxxxxx>; Mark Rutland <mark.rutland@xxxxxxx>; yury.norov@xxxxxxxxx <yury.norov@xxxxxxxxx>; linux@xxxxxxxxxxxxxxxxxx <linux@xxxxxxxxxxxxxxxxxx>; parri.andrea@xxxxxxxxx <parri.andrea@xxxxxxxxx>; leobras@xxxxxxxxxx <leobras@xxxxxxxxxx>; guoren@xxxxxxxxxx <guoren@xxxxxxxxxx>; ericchancf@xxxxxxxxxx <ericchancf@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; Djordje Todorovic <djordje.todorovic@xxxxxxxxxxxxx>
> Subject: Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic functions
>
> [You don't often get email from palmer@xxxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> 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
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv