Thread (51 messages) 51 messages, 4 authors, 2025-09-17

Re: [PATCH RESEND v7 4/6] arm64: futex: refactor futex atomic operation

From: Mark Rutland <mark.rutland@arm.com>
Date: 2025-09-16 13:45:38
Also in: kvmarm, lkml

On Tue, Sep 16, 2025 at 02:27:37PM +0100, Yeoreum Yun wrote:
Hi Mark,

[...]
quoted
quoted
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index 1d6d9f856ac5..0aeda7ced2c0 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -126,6 +126,60 @@ LSUI_FUTEX_ATOMIC_OP(or, ldtset, al)
 LSUI_FUTEX_ATOMIC_OP(andnot, ldtclr, al)
 LSUI_FUTEX_ATOMIC_OP(set, swpt, al)

+
+#define LSUI_CMPXCHG_HELPER(suffix, start_bit)                                 \
+static __always_inline int                                                     \
+__lsui_cmpxchg_helper_##suffix(u64 __user *uaddr, u32 oldval, u32 newval)      \
+{                                                                              \
+       int ret = 0;                                                            \
+       u64 oval, nval, tmp;                                                    \
+                                                                               \
+       asm volatile("//__lsui_cmpxchg_helper_" #suffix "\n"                    \
+       __LSUI_PREAMBLE                                                         \
+"      prfm    pstl1strm, %2\n"                                                \
+"1:    ldtr    %x1, %2\n"                                                      \
+"      mov     %x3, %x1\n"                                                     \
+"      bfi     %x1, %x5, #" #start_bit ", #32\n"                               \
+"      bfi     %x3, %x6, #" #start_bit ", #32\n"                               \
+"      mov     %x4, %x1\n"                                                     \
+"2:    caslt   %x1, %x3, %2\n"                                                 \
+"      sub     %x1, %x1, %x4\n"                                                \
+"      cbz     %x1, 3f\n"                                                      \
+"      mov     %w0, %w7\n"                                                     \
+"3:\n"                                                                         \
+"      dmb     ish\n"                                                          \
+"4:\n"                                                                         \
+       _ASM_EXTABLE_UACCESS_ERR(1b, 4b, %w0)                                   \
+       _ASM_EXTABLE_UACCESS_ERR(2b, 4b, %w0)                                   \
+       : "+r" (ret), "=&r" (oval), "+Q" (*uaddr), "=&r" (nval), "=&r" (tmp)    \
+       : "r" (oldval), "r" (newval), "Ir" (-EAGAIN)                            \
+       : "memory");                                                            \
+                                                                               \
+       return ret;                                                             \
+}
+
+LSUI_CMPXCHG_HELPER(lo, 0)
+LSUI_CMPXCHG_HELPER(hi, 32)
+
+static __always_inline int
+__lsui_cmpxchg_helper(u32 __user *uaddr, u32 oldval, u32 newval, u32 *oval)
+{
+       int ret;
+       unsigned long uaddr_al;
+
+       uaddr_al = ALIGN_DOWN((unsigned long)uaddr, sizeof(u64));
+
+       if (uaddr_al != (unsigned long)uaddr)
+               ret = __lsui_cmpxchg_helper_hi((u64 __user *)uaddr_al, oldval, newval);
+       else
+               ret = __lsui_cmpxchg_helper_lo((u64 __user *)uaddr_al, oldval, newval);
+
+       if (!ret)
+               *oval = oldval;
+
+       return ret;
+}
I think Will expects that you do more of this in C, e.g. have a basic
user cmpxchg on a 64-bit type, e.g.

/*
 * NOTE: *oldp is NOT updated if a fault is taken.
 */
static __always_inline int
user_cmpxchg64_release(u64 __usr *addr, u64 *oldp, u64 new)
{
	int err = 0;

	asm volatile(
	__LSUI_PREAMBLE
	"1:	caslt	%x[old], %x[new], %[addr]\n"
	"2:\n"
	_ASM_EXTABLE_UACCESS_ERR(1b, 4b, %w0)
	: [addr] "+Q" (addr),
	  [old] "+r" (*oldp)
	: [new] "r" (new)
	: "memory"
	);

	return err;
}

That should be the *only* assembly you need to implement.

Atop that, have a wrapper that uses get_user() and that helper above to
implement the 32-bit user cmpxchg, with all the bit manipulation in C:
Thanks for your suggestion. But small question.
I think it's enough to use usafe_get_user() instead of get_user() in here
since when FEAT_LSUI enabled, it doeesn't need to call
uaccess_ttbr0_enable()/disable().
Regardless of uaccess_ttbr0_enable() and uaccess_ttbr0_disable()
specifically, API-wise unsafe_get_user() is only supposed to be called
between user_access_begin() and user_access_end(), and there's some
stuff we probably want to add there (e.g. might_fault(), which
unsafe_get_user() lacks today).

Do we call those?

Mark.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help