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.