[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aMo/sQR80i7GbpAF@gmail.com>
Date: Wed, 17 Sep 2025 00:57:21 -0400
From: Guo Ren <guoren@...nel.org>
To: Andrea Parri <parri.andrea@...il.com>
Cc: Xu Lu <luxu.kernel@...edance.com>, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, paul.walmsley@...ive.com, palmer@...belt.com,
aou@...s.berkeley.edu, alex@...ti.fr, ajones@...tanamicro.com,
brs@...osinc.com, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
apw@...onical.com, joe@...ches.com
Subject: Re: [PATCH v2 0/4] riscv: Add Zalasr ISA extension support
On Wed, Sep 17, 2025 at 12:01:34AM -0400, Guo Ren wrote:
> On Tue, Sep 02, 2025 at 06:59:15PM +0200, Andrea Parri wrote:
> > > Xu Lu (4):
> > > riscv: add ISA extension parsing for Zalasr
> > > dt-bindings: riscv: Add Zalasr ISA extension description
> > > riscv: Instroduce Zalasr instructions
> > > riscv: Use Zalasr for smp_load_acquire/smp_store_release
> >
> > Informally put, our (Linux) memory consistency model specifies that any
> > sequence
> >
> > spin_unlock(s);
> > spin_lock(t);
> >
> > behaves "as it provides at least FENCE.TSO ordering between operations
> > which precede the UNLOCK+LOCK sequence and operations which follow the
> > sequence". Unless I missing something, the patch set in question breaks
> > such ordering property (on RISC-V): for example, a "release" annotation,
> > .RL (as in spin_unlock() -> smp_store_release(), after patch #4) paired
> > with an "acquire" fence, FENCE R,RW (as could be found in spin_lock() ->
> > atomic_try_cmpxchg_acquire()) do not provide the specified property.
> >
> > I _think some solutions to the issue above include:
> >
> > a) make sure an .RL annotation is always paired with an .AQ annotation
> > and viceversa an .AQ annotation is paired with an .RL annotation
> > (this approach matches the current arm64 approach/implementation);
> >
> > b) on the opposite direction, always pair FENCE R,RW (or occasionally
> > FENCE R,R) with FENCE RW,W (this matches the current approach/the
> > current implementation within riscv);
> >
> > c) mix the previous two solutions (resp., annotations and fences), but
> > make sure to "upgrade" any releases to provide (insert) a FENCE.TSO.
> I prefer option c) at first, it has fewer modification and influence.
>
> asm volatile(ALTERNATIVE("fence rw, w;\t\nsb %0, 0(%1)\t\n", \
> - SB_RL(%0, %1) "\t\nnop\t\n", \
> + SB_RL(%0, %1) "\t\n fence.tso;\t\n", \
"fence rw, rw;\t\nsb %0, 0(%1)\t\n", \
How about enhance fence rw, rw? It's a bit more stronger than .tso.
0, RISCV_ISA_EXT_ZALASR, 1) \
> : : "r" (v), "r" (p) : "memory"); \
>
> I didn't object option a), and I think it could be done in the future.
> Acquire Zalasr extension step by step.
>
> >
> > (a) would align RISC-V and ARM64 (which is a good thing IMO), though it
> > is probably the most invasive approach among the three approaches above
> > (requiring certain changes to arch/riscv/include/asm/{cmpxchg,atomic}.h,
> > which are already relatively messy due to the various ZABHA plus ZACAS
> > switches). Overall, I'm not too exited at the idea of reviewing any of
> > those changes, but if the community opts for it, I'll almost definitely
> > take a closer look with due calm. ;-)
> >
> > Andrea
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
Powered by blists - more mailing lists