Re: [PATCH RESEND v7 6/6] arm64: futex: support futex with FEAT_LSUI

[Yeoreum Yun <yeoreum.yun@arm.com>]

Hi Catalin,

> On Sat, Aug 16, 2025 at 04:19:29PM +0100, Yeoreum Yun wrote:
> > @@ -115,11 +117,137 @@ __llsc_futex_cmpxchg(u32 __user *uaddr, u32 oldval, u32 newval, u32 *oval)
> >  	return ret;
> >  }
> >
> > +#ifdef CONFIG_AS_HAS_LSUI
> > +
> > +#define __LSUI_PREAMBLE	".arch_extension lsui\n"
> > +
> > +#define LSUI_FUTEX_ATOMIC_OP(op, asm_op, mb)				\
> > +static __always_inline int						\
> > +__lsui_futex_atomic_##op(int oparg, u32 __user *uaddr, int *oval)	\
> > +{									\
> > +	int ret = 0;							\
> > +	int oldval;							\
> > +									\
> > +	uaccess_ttbr0_enable();						\
>
> I think we can drop uaccess_ttbr0_*() from these functions. At the
> kconfig level, TTBR0_PAN selects PAN. Hardware with LSUI will also
> have PAN (since 8.1), so the above is an unnecessary branch or nop,
> depending on how the alternatives play out. But add a comment instead.

Thanks to point out this.
I'll change it.

>
> > +static __always_inline int
> > +__lsui_futex_atomic_eor(int oparg, u32 __user *uaddr, int *oval)
> > +{
> > +	unsigned int loops = LLSC_MAX_LOOPS;
> > +	int ret, oldval, tmp;
> > +
> > +	uaccess_ttbr0_enable();
> > +	/*
> > +	 * there are no ldteor/stteor instructions...
> > +	 */
> > +	asm volatile("// __lsui_futex_atomic_eor\n"
> > +	__LSUI_PREAMBLE
> > +"	prfm	pstl1strm, %2\n"
> > +"1:	ldtxr	%w1, %2\n"
> > +"	eor	%w3, %w1, %w5\n"
> > +"2:	stltxr	%w0, %w3, %2\n"
> > +"	cbz	%w0, 3f\n"
> > +"	sub	%w4, %w4, %w0\n"
> > +"	cbnz	%w4, 1b\n"
> > +"	mov	%w0, %w6\n"
> > +"3:\n"
> > +"	dmb	ish\n"
> > +	_ASM_EXTABLE_UACCESS_ERR(1b, 3b, %w0)
> > +	_ASM_EXTABLE_UACCESS_ERR(2b, 3b, %w0)
> > +	: "=&r" (ret), "=&r" (oldval), "+Q" (*uaddr), "=&r" (tmp),
> > +	  "+r" (loops)
> > +	: "r" (oparg), "Ir" (-EAGAIN)
> > +	: "memory");
> > +	uaccess_ttbr0_disable();
> > +
> > +	if (!ret)
> > +		*oval = oldval;
> > +
> > +	return ret;
> > +}
>
> That's an unfortunate omission from the architecture.
>
> > +#define __lsui_llsc_body(op, ...)					\
> > +({									\
> > +	alternative_has_cap_likely(ARM64_HAS_LSUI) ?			\
> > +		__lsui_##op(__VA_ARGS__) : __llsc_##op(__VA_ARGS__);	\
> > +})
> > +
> > +#else	/* CONFIG_AS_HAS_LSUI */
> > +
> > +#define __lsui_llsc_body(op, ...)	__llsc_##op(__VA_ARGS__)
> > +
> > +#endif	/* CONFIG_AS_HAS_LSUI */
> > +
> > +
> >  #define FUTEX_ATOMIC_OP(op)						\
> >  static __always_inline int						\
> >  __futex_atomic_##op(int oparg, u32 __user *uaddr, int *oval)		\
> >  {									\
> > -	return __llsc_futex_atomic_##op(oparg, uaddr, oval);		\
> > +	return __lsui_llsc_body(futex_atomic_##op, oparg, uaddr, oval);	\
> >  }
>
> That's what I got confused about. It looks fine:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks!
--
Sincerely,
Yeoreum Yun