Message-ID: <29c9eeff-6146-4f3b-b1e1-e7ed20fa98a3@ghiti.fr>
Date: Tue, 25 Mar 2025 09:58:31 +0100
From: Alexandre Ghiti <alex@...ti.fr>
To: Djordje Todorovic <Djordje.Todorovic@...cgroup.com>,
 Palmer Dabbelt <palmer@...belt.com>, "jrtc27@...c27.com" <jrtc27@...c27.com>
Cc: Charlie Jenkins <charlie@...osinc.com>,
 "samuel.holland@...ive.com" <samuel.holland@...ive.com>,
 Conor Dooley <conor@...nel.org>, "arikalo@...il.com" <arikalo@...il.com>,
 "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
 Paul Walmsley <paul.walmsley@...ive.com>,
 "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
 Will Deacon <will@...nel.org>, "peterz@...radead.org"
 <peterz@...radead.org>, "boqun.feng@...il.com" <boqun.feng@...il.com>,
 Mark Rutland <mark.rutland@....com>,
 "yury.norov@...il.com" <yury.norov@...il.com>,
 "linux@...musvillemoes.dk" <linux@...musvillemoes.dk>,
 "parri.andrea@...il.com" <parri.andrea@...il.com>,
 "leobras@...hat.com" <leobras@...hat.com>,
 "guoren@...nel.org" <guoren@...nel.org>,
 "ericchancf@...gle.com" <ericchancf@...gle.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic
 functions

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@...cgroup.com>
> Sent: Wednesday, February 12, 2025 11:22 AM
> To: Palmer Dabbelt <palmer@...belt.com>; jrtc27@...c27.com <jrtc27@...c27.com>
> Cc: Charlie Jenkins <charlie@...osinc.com>; samuel.holland@...ive.com <samuel.holland@...ive.com>; Conor Dooley <conor@...nel.org>; arikalo@...il.com <arikalo@...il.com>; linux-riscv@...ts.infradead.org <linux-riscv@...ts.infradead.org>; Paul Walmsley <paul.walmsley@...ive.com>; aou@...s.berkeley.edu <aou@...s.berkeley.edu>; Will Deacon <will@...nel.org>; peterz@...radead.org <peterz@...radead.org>; boqun.feng@...il.com <boqun.feng@...il.com>; Mark Rutland <mark.rutland@....com>; yury.norov@...il.com <yury.norov@...il.com>; linux@...musvillemoes.dk <linux@...musvillemoes.dk>; parri.andrea@...il.com <parri.andrea@...il.com>; leobras@...hat.com <leobras@...hat.com>; guoren@...nel.org <guoren@...nel.org>; ericchancf@...gle.com <ericchancf@...gle.com>; linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>
> 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@...belt.com>
> Sent: Wednesday, February 5, 2025 4:49 PM
> To: jrtc27@...c27.com <jrtc27@...c27.com>
> Cc: Charlie Jenkins <charlie@...osinc.com>; samuel.holland@...ive.com <samuel.holland@...ive.com>; Conor Dooley <conor@...nel.org>; arikalo@...il.com <arikalo@...il.com>; linux-riscv@...ts.infradead.org <linux-riscv@...ts.infradead.org>; Paul Walmsley <paul.walmsley@...ive.com>; aou@...s.berkeley.edu <aou@...s.berkeley.edu>; Will Deacon <will@...nel.org>; peterz@...radead.org <peterz@...radead.org>; boqun.feng@...il.com <boqun.feng@...il.com>; Mark Rutland <mark.rutland@....com>; yury.norov@...il.com <yury.norov@...il.com>; linux@...musvillemoes.dk <linux@...musvillemoes.dk>; parri.andrea@...il.com <parri.andrea@...il.com>; leobras@...hat.com <leobras@...hat.com>; guoren@...nel.org <guoren@...nel.org>; ericchancf@...gle.com <ericchancf@...gle.com>; linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>; Djordje Todorovic <djordje.todorovic@...cgroup.com>
> Subject: Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic functions
>
> [You don't often get email from palmer@...belt.com. 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@...c27.com wrote:
>> On 3 Feb 2025, at 19:50, Charlie Jenkins <charlie@...osinc.com> 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@...osinc.com> wrote:
>>>>>>>
>>>>>>>>> From: Chao-ying Fu <cfu@...s.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.
>> 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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists