diff mbox series

[RFC,2/3] x86/uaccess: Implement unsafe_try_cmpxchg_user()

Message ID 20211214205358.643980924@infradead.org (mailing list archive)
State New
Headers show
Series sched: User Managed Concurrency Groups | expand

Commit Message

Peter Zijlstra Dec. 14, 2021, 8:44 p.m. UTC
Do try_cmpxchg() loops on userspace addresses.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/uaccess.h |   57 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

Comments

Sean Christopherson Dec. 20, 2021, 5:30 p.m. UTC | #1
On Tue, Dec 14, 2021, Peter Zijlstra wrote:
> Do try_cmpxchg() loops on userspace addresses.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> @@ -501,6 +543,21 @@ do {										\
>  } while (0)
>  #endif // CONFIG_CC_HAS_ASM_GOTO_OUTPUT
>  
> +extern void __try_cmpxchg_user_wrong_size(void);
> +
> +#define unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label) ({		\
> +	__typeof__(*(_ptr)) __ret;					\
> +	switch (sizeof(__ret)) {					\
> +	case 4:	__ret = __try_cmpxchg_user_asm("l", (_ptr), (_oldp),	\
> +					       (_nval), _label);	\
> +		break;							\
> +	case 8:	__ret = __try_cmpxchg_user_asm("q", (_ptr), (_oldp),	\
> +					       (_nval), _label);	\
> +		break;							\

Can we add support for 1-byte and 2-byte cmpxchg, and for using cmpxchg8b to handle
8-byte operations in 32-bit mode?  Support for all the flavors (except 16-byte)
would allow KVM to use this in an emulator path that currently kmaps the target.
I'd be more than happy to help test the result.

Thanks!

> +	default: __try_cmpxchg_user_wrong_size();			\
> +	}								\
> +	__ret;						})
> +
>  /*
>   * We want the unsafe accessors to always be inlined and use
>   * the error labels - thus the macro games.
> 
>
Peter Zijlstra Dec. 21, 2021, 11:17 a.m. UTC | #2
On Mon, Dec 20, 2021 at 05:30:05PM +0000, Sean Christopherson wrote:
> On Tue, Dec 14, 2021, Peter Zijlstra wrote:
> > Do try_cmpxchg() loops on userspace addresses.
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> > @@ -501,6 +543,21 @@ do {										\
> >  } while (0)
> >  #endif // CONFIG_CC_HAS_ASM_GOTO_OUTPUT
> >  
> > +extern void __try_cmpxchg_user_wrong_size(void);
> > +
> > +#define unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label) ({		\
> > +	__typeof__(*(_ptr)) __ret;					\
> > +	switch (sizeof(__ret)) {					\
> > +	case 4:	__ret = __try_cmpxchg_user_asm("l", (_ptr), (_oldp),	\
> > +					       (_nval), _label);	\
> > +		break;							\
> > +	case 8:	__ret = __try_cmpxchg_user_asm("q", (_ptr), (_oldp),	\
> > +					       (_nval), _label);	\
> > +		break;							\
> 
> Can we add support for 1-byte and 2-byte cmpxchg, and for using cmpxchg8b to handle
> 8-byte operations in 32-bit mode?  Support for all the flavors (except 16-byte)
> would allow KVM to use this in an emulator path that currently kmaps the target.
> I'd be more than happy to help test the result.

Sure, no problem. I currently still need to audit parts of mm/ and
do the smp-wake-idle bits before I repost -- that and take a xmas break
ofcourse :-) So it'll be a while before I repost this.
diff mbox series

Patch

--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -342,6 +342,24 @@  do {									\
 		     : [umem] "m" (__m(addr))				\
 		     : : label)
 
+#define __try_cmpxchg_user_asm(itype, _ptr, _pold, _new, label)	({	\
+	bool success;							\
+	__typeof__(_ptr) _old = (__typeof__(_ptr))(_pold);		\
+	__typeof__(*(_ptr)) __old = *_old;				\
+	__typeof__(*(_ptr)) __new = (_new);				\
+	asm_volatile_goto("\n"						\
+		     "1: " LOCK_PREFIX "cmpxchg"itype" %[new], %[ptr]\n"\
+		     _ASM_EXTABLE_UA(1b, %l[label])			\
+		     : CC_OUT(z) (success),				\
+		       [ptr] "+m" (*_ptr),				\
+		       [old] "+a" (__old)				\
+		     : [new] "r" (__new)				\
+		     : "memory", "cc"					\
+		     : label);						\
+	if (unlikely(!success))						\
+		*_old = __old;						\
+	likely(success);					})
+
 #else // !CONFIG_CC_HAS_ASM_GOTO_OUTPUT
 
 #ifdef CONFIG_X86_32
@@ -407,6 +425,30 @@  do {									\
 		     : [umem] "m" (__m(addr)),				\
 		       "0" (err))
 
+#define __try_cmpxchg_user_asm(itype, _ptr, _pold, _new, label)	({	\
+	int __err = 0;							\
+	bool success;							\
+	__typeof__(_ptr) _old = (__typeof__(_ptr))(_pold);		\
+	__typeof__(*(_ptr)) __old = *_old;				\
+	__typeof__(*(_ptr)) __new = (_new);				\
+	asm volatile("\n"						\
+		     "1: " LOCK_PREFIX "cmpxchg"itype" %[new], %[ptr]\n"\
+		     CC_SET(z)						\
+		     "2:\n"						\
+		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG,	\
+					   %[errout])			\
+		     : CC_OUT(z) (success),				\
+		       [errout] "+r" (__err),				\
+		       [ptr] "+m" (*_ptr),				\
+		       [old] "+a" (__old)				\
+		     : [new] "r" (__new)				\
+		     : "memory", "cc");					\
+	if (unlikely(__err))						\
+		goto label;						\
+	if (unlikely(!success))						\
+		*_old = __old;						\
+	likely(success);					})
+
 #endif // CONFIG_CC_HAS_ASM_GOTO_OUTPUT
 
 /* FIXME: this hack is definitely wrong -AK */
@@ -501,6 +543,21 @@  do {										\
 } while (0)
 #endif // CONFIG_CC_HAS_ASM_GOTO_OUTPUT
 
+extern void __try_cmpxchg_user_wrong_size(void);
+
+#define unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label) ({		\
+	__typeof__(*(_ptr)) __ret;					\
+	switch (sizeof(__ret)) {					\
+	case 4:	__ret = __try_cmpxchg_user_asm("l", (_ptr), (_oldp),	\
+					       (_nval), _label);	\
+		break;							\
+	case 8:	__ret = __try_cmpxchg_user_asm("q", (_ptr), (_oldp),	\
+					       (_nval), _label);	\
+		break;							\
+	default: __try_cmpxchg_user_wrong_size();			\
+	}								\
+	__ret;						})
+
 /*
  * We want the unsafe accessors to always be inlined and use
  * the error labels - thus the macro games.