linux-kernel - Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks

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: <ZHZESDWJZvhuJ3Af@andrea>
Date:   Tue, 30 May 2023 20:45:28 +0200
From:   Andrea Parri <parri.andrea@...il.com>
To:     Palmer Dabbelt <palmer@...belt.com>
Cc:     ajones@...tanamicro.com, heiko@...ech.de,
        linux-riscv@...ts.infradead.org,
        Paul Walmsley <paul.walmsley@...ive.com>,
        linux-kernel@...r.kernel.org, christoph.muellner@...ll.eu,
        David.Laight@...lab.com, heiko.stuebner@...ll.eu
Subject: Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks

On Wed, May 24, 2023 at 04:00:43PM -0700, Palmer Dabbelt wrote:
> On Wed, 24 May 2023 10:05:52 PDT (-0700), ajones@...tanamicro.com wrote:

> > I guess this peeling off of the first iteration is because it's expected
> > that the load generated by READ_ONCE() is more efficient than lr.w/d? If
> > we're worried about unnecessary use of lr.w/d, then shouldn't we look
> > for a solution that doesn't issue those instructions when we don't have
> > the Zawrs extension?
> 
> It's actually just a consequence of how the Linux hooks are described:
> they're macros that take a C expression to test in the loop, and we can't
> handle C expressions in LR/SC loops as that'd require compiler support and
> nobody's figured out how to do that correctly yet (there were some patches,
> but they had issues).  So we need to do this awkward bit of checking without
> the reservation and then waiting with the reservation.

I believe Andrew was really just hinting to something like (from
arch/arm64/):

#define smp_cond_load_relaxed(ptr, cond_expr)				\
({									\
	typeof(ptr) __PTR = (ptr);					\
	__unqual_scalar_typeof(*ptr) VAL;				\
	for (;;) {							\
		VAL = READ_ONCE(*__PTR);				\
		if (cond_expr)						\
			break;						\
		__cmpwait_relaxed(__PTR, VAL);				\
	}								\
	(typeof(*ptr))VAL;						\
})

where the __cmpwait_relaxed() would issue NOPs without Zawrs, a
sequence "lr.* ; beq ; wrs.sto" otherwise.  (with the "dangling
reservation" when we branch, similarly to CMPXCHG)?

  Andrea