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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z6Ed6gYzdKhQQQVM@ghost>
Date: Mon, 3 Feb 2025 11:50:02 -0800
From: Charlie Jenkins <charlie@...osinc.com>
To: Samuel Holland <samuel.holland@...ive.com>
Cc: Conor Dooley <conor@...nel.org>, Aleksandar Rikalo <arikalo@...il.com>,
	linux-riscv@...ts.infradead.org,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>, Will Deacon <will@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Boqun Feng <boqun.feng@...il.com>,
	Mark Rutland <mark.rutland@....com>,
	Yury Norov <yury.norov@...il.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Andrea Parri <parri.andrea@...il.com>,
	Leonardo Bras <leobras@...hat.com>, Guo Ren <guoren@...nel.org>,
	Eric Chan <ericchancf@...gle.com>, linux-kernel@...r.kernel.org,
	Djordje Todorovic <djordje.todorovic@...cgroup.com>
Subject: Re: [PATCH v2] riscv: Use Zalrsc extension to implement atomic
 functions

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.

- Charlie

> 
> Regards,
> Samuel
>