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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <mhng-50508e34-b5cc-4dad-b4d4-7d2bac5a74d8@palmer-ri-x1c9>
Date: Wed, 05 Feb 2025 07:49:55 -0800 (PST)
From: Palmer Dabbelt <palmer@...belt.com>
To: jrtc27@...c27.com
CC: Charlie Jenkins <charlie@...osinc.com>, samuel.holland@...ive.com,
  Conor Dooley <conor@...nel.org>, arikalo@...il.com, linux-riscv@...ts.infradead.org,
  Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu, Will Deacon <will@...nel.org>, peterz@...radead.org,
  boqun.feng@...il.com, Mark Rutland <mark.rutland@....com>, yury.norov@...il.com,
  linux@...musvillemoes.dk, parri.andrea@...il.com, leobras@...hat.com, guoren@...nel.org,
  ericchancf@...gle.com, linux-kernel@...r.kernel.org, djordje.todorovic@...cgroup.com
Subject:     Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic functions

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