kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64()
@ 2020-12-15 18:28 Uros Bizjak
  2020-12-15 18:28 ` [PATCH 1/3] asm-generic/atomic: Add try_cmpxchg64() instrumentation Uros Bizjak
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Uros Bizjak @ 2020-12-15 18:28 UTC (permalink / raw)
  To: x86, kvm, linux-kernel
  Cc: Uros Bizjak, Will Deacon, Peter Zijlstra, Boqun Feng,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel

This patch series introduces try_cmpxchg64() atomic locking function.

try_cmpxchg64() provides the same interface for 64 bit and 32 bit targets,
emits CMPXCHGQ for 64 bit targets and CMPXCHG8B for 32 bit targets,
and provides appropriate fallbacks when CMPXCHG8B is unavailable.

try_cmpxchg64() reuses flags from CMPXCHGQ/CMPXCHG8B instructions and
avoids unneeded CMP for 64 bit targets or XOR/XOR/OR sequence for
32 bit targets.

Cc: Will Deacon <will@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>

Uros Bizjak (3):
  asm-generic/atomic: Add try_cmpxchg64() instrumentation
  locking/atomic/x86: Introduce arch_try_cmpxchg64()
  KVM/VMX: Use try_cmpxchg64() in posted_intr.c

 arch/x86/include/asm/cmpxchg_32.h         | 62 +++++++++++++++++++----
 arch/x86/include/asm/cmpxchg_64.h         |  6 +++
 arch/x86/kvm/vmx/posted_intr.c            |  9 ++--
 include/asm-generic/atomic-instrumented.h | 46 ++++++++++++++++-
 scripts/atomic/gen-atomic-instrumented.sh |  2 +-
 5 files changed, 108 insertions(+), 17 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] asm-generic/atomic: Add try_cmpxchg64() instrumentation
  2020-12-15 18:28 [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Uros Bizjak
@ 2020-12-15 18:28 ` Uros Bizjak
  2020-12-15 18:28 ` [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64() Uros Bizjak
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Uros Bizjak @ 2020-12-15 18:28 UTC (permalink / raw)
  To: x86, kvm, linux-kernel
  Cc: Uros Bizjak, Will Deacon, Peter Zijlstra, Boqun Feng

Instrument try_cmpxchg64() similar to try_cmpxchg().

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Will Deacon <will@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
---
 include/asm-generic/atomic-instrumented.h | 46 ++++++++++++++++++++++-
 scripts/atomic/gen-atomic-instrumented.sh |  2 +-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/atomic-instrumented.h b/include/asm-generic/atomic-instrumented.h
index 888b6cfeed91..396d4484963a 100644
--- a/include/asm-generic/atomic-instrumented.h
+++ b/include/asm-generic/atomic-instrumented.h
@@ -1793,6 +1793,50 @@ atomic64_dec_if_positive(atomic64_t *v)
 })
 #endif
 
+#if !defined(arch_try_cmpxchg64_relaxed) || defined(arch_try_cmpxchg64)
+#define try_cmpxchg64(ptr, oldp, ...) \
+({ \
+	typeof(ptr) __ai_ptr = (ptr); \
+	typeof(oldp) __ai_oldp = (oldp); \
+	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	arch_try_cmpxchg64(__ai_ptr, __ai_oldp, __VA_ARGS__); \
+})
+#endif
+
+#if defined(arch_try_cmpxchg64_acquire)
+#define try_cmpxchg64_acquire(ptr, oldp, ...) \
+({ \
+	typeof(ptr) __ai_ptr = (ptr); \
+	typeof(oldp) __ai_oldp = (oldp); \
+	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	arch_try_cmpxchg64_acquire(__ai_ptr, __ai_oldp, __VA_ARGS__); \
+})
+#endif
+
+#if defined(arch_try_cmpxchg64_release)
+#define try_cmpxchg64_release(ptr, oldp, ...) \
+({ \
+	typeof(ptr) __ai_ptr = (ptr); \
+	typeof(oldp) __ai_oldp = (oldp); \
+	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	arch_try_cmpxchg64_release(__ai_ptr, __ai_oldp, __VA_ARGS__); \
+})
+#endif
+
+#if defined(arch_try_cmpxchg64_relaxed)
+#define try_cmpxchg64_relaxed(ptr, oldp, ...) \
+({ \
+	typeof(ptr) __ai_ptr = (ptr); \
+	typeof(oldp) __ai_oldp = (oldp); \
+	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr)); \
+	instrument_atomic_write(__ai_oldp, sizeof(*__ai_oldp)); \
+	arch_try_cmpxchg64_relaxed(__ai_ptr, __ai_oldp, __VA_ARGS__); \
+})
+#endif
+
 #define cmpxchg_local(ptr, ...) \
 ({ \
 	typeof(ptr) __ai_ptr = (ptr); \
@@ -1830,4 +1874,4 @@ atomic64_dec_if_positive(atomic64_t *v)
 })
 
 #endif /* _ASM_GENERIC_ATOMIC_INSTRUMENTED_H */
-// 4bec382e44520f4d8267e42620054db26a659ea3
+// 28b0549ca119ed58a9b61da6e277ee30de18bdfc
diff --git a/scripts/atomic/gen-atomic-instrumented.sh b/scripts/atomic/gen-atomic-instrumented.sh
index 5766ffcec7c5..ade2e509b5eb 100755
--- a/scripts/atomic/gen-atomic-instrumented.sh
+++ b/scripts/atomic/gen-atomic-instrumented.sh
@@ -186,7 +186,7 @@ grep '^[a-z]' "$1" | while read name meta args; do
 	gen_proto "${meta}" "${name}" "atomic64" "s64" ${args}
 done
 
-for xchg in "xchg" "cmpxchg" "cmpxchg64" "try_cmpxchg"; do
+for xchg in "xchg" "cmpxchg" "cmpxchg64" "try_cmpxchg" "try_cmpxchg64"; do
 	for order in "" "_acquire" "_release" "_relaxed"; do
 		gen_optional_xchg "${xchg}" "${order}"
 	done
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64()
  2020-12-15 18:28 [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Uros Bizjak
  2020-12-15 18:28 ` [PATCH 1/3] asm-generic/atomic: Add try_cmpxchg64() instrumentation Uros Bizjak
@ 2020-12-15 18:28 ` Uros Bizjak
  2020-12-15 20:08   ` Uros Bizjak
  2020-12-16 15:37   ` kernel test robot
  2020-12-15 18:28 ` [PATCH 3/3] KVM/VMX: Use try_cmpxchg64() in posted_intr.c Uros Bizjak
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 9+ messages in thread
From: Uros Bizjak @ 2020-12-15 18:28 UTC (permalink / raw)
  To: x86, kvm, linux-kernel
  Cc: Uros Bizjak, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin

Add arch_try_cmpxchg64(), similar to arch_try_cmpxchg(), that
operates with 64 bit operands. This function provides the same
interface for 32 bit and 64 bit targets.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/include/asm/cmpxchg_32.h | 62 ++++++++++++++++++++++++++-----
 arch/x86/include/asm/cmpxchg_64.h |  6 +++
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/cmpxchg_32.h b/arch/x86/include/asm/cmpxchg_32.h
index 0a7fe0321613..8dcde400244e 100644
--- a/arch/x86/include/asm/cmpxchg_32.h
+++ b/arch/x86/include/asm/cmpxchg_32.h
@@ -35,15 +35,6 @@ static inline void set_64bit(volatile u64 *ptr, u64 value)
 		     : "memory");
 }
 
-#ifdef CONFIG_X86_CMPXCHG64
-#define arch_cmpxchg64(ptr, o, n)					\
-	((__typeof__(*(ptr)))__cmpxchg64((ptr), (unsigned long long)(o), \
-					 (unsigned long long)(n)))
-#define arch_cmpxchg64_local(ptr, o, n)					\
-	((__typeof__(*(ptr)))__cmpxchg64_local((ptr), (unsigned long long)(o), \
-					       (unsigned long long)(n)))
-#endif
-
 static inline u64 __cmpxchg64(volatile u64 *ptr, u64 old, u64 new)
 {
 	u64 prev;
@@ -71,6 +62,39 @@ static inline u64 __cmpxchg64_local(volatile u64 *ptr, u64 old, u64 new)
 }
 
 #ifndef CONFIG_X86_CMPXCHG64
+#define arch_cmpxchg64(ptr, o, n)					\
+	((__typeof__(*(ptr)))__cmpxchg64((ptr), (unsigned long long)(o), \
+					 (unsigned long long)(n)))
+#define arch_cmpxchg64_local(ptr, o, n)					\
+	((__typeof__(*(ptr)))__cmpxchg64_local((ptr), (unsigned long long)(o), \
+
+#define __raw_try_cmpxchg64(_ptr, _pold, _new, lock)		\
+({								\
+	bool success;						\
+	__typeof__(_ptr) _old = (__typeof__(_ptr))(_pold);	\
+	__typeof__(*(_ptr)) __old = *_old;			\
+	__typeof__(*(_ptr)) __new = (_new);			\
+	asm volatile(lock "cmpxchg8b %1"			\
+		     CC_SET(z)					\
+		     : CC_OUT(z) (success),			\
+		       "+m" (*_ptr),				\
+		       "+A" (__old)				\
+		     : "b" ((unsigned int)__new),		\
+		       "c" ((unsigned int)(__new>>32))		\
+		     : "memory");				\
+	if (unlikely(!success))					\
+		*_old = __old;					\
+	likely(success);					\
+})
+
+#define __try_cmpxchg64(ptr, pold, new)				\
+	__raw_try_cmpxchg64((ptr), (pold), (new), LOCK_PREFIX)
+
+#define arch_try_cmpxchg64(ptr, pold, new) 			\
+	__try_cmpxchg64((ptr), (pold), (new))
+
+#else
+
 /*
  * Building a kernel capable running on 80386 and 80486. It may be necessary
  * to simulate the cmpxchg8b on the 80386 and 80486 CPU.
@@ -108,6 +132,26 @@ static inline u64 __cmpxchg64_local(volatile u64 *ptr, u64 old, u64 new)
 		       : "memory");				\
 	__ret; })
 
+#define arch_try_cmpxchg64(ptr, po, n)				\
+({								\
+	bool success;						\
+	__typeof__(ptr) _old = (__typeof__(ptr))(po);		\
+	__typeof__(*(ptr)) __old = *_old;			\
+	__typeof__(*(ptr)) __new = (n);				\
+	alternative_io(LOCK_PREFIX_HERE				\
+			"call cmpxchg8b_emu",			\
+			"lock; cmpxchg8b (%%esi)" ,		\
+		       X86_FEATURE_CX8,				\
+		       "+A" (__old),				\
+		       "S" ((ptr)),				\
+		       "b" ((unsigned int)__new),		\
+		       "c" ((unsigned int)(__new>>32))		\
+		       : "memory");				\
+	success = (__old == *_old);				\
+	if (unlikely(!success))					\
+		*_old = __old;					\
+	likely(success);					\
+})
 #endif
 
 #define system_has_cmpxchg_double() boot_cpu_has(X86_FEATURE_CX8)
diff --git a/arch/x86/include/asm/cmpxchg_64.h b/arch/x86/include/asm/cmpxchg_64.h
index 072e5459fe2f..250187ac8248 100644
--- a/arch/x86/include/asm/cmpxchg_64.h
+++ b/arch/x86/include/asm/cmpxchg_64.h
@@ -19,6 +19,12 @@ static inline void set_64bit(volatile u64 *ptr, u64 val)
 	arch_cmpxchg_local((ptr), (o), (n));				\
 })
 
+#define arch_try_cmpxchg64(ptr, po, n)					\
+({									\
+	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
+	arch_try_cmpxchg((ptr), (po), (n));				\
+})
+
 #define system_has_cmpxchg_double() boot_cpu_has(X86_FEATURE_CX16)
 
 #endif /* _ASM_X86_CMPXCHG_64_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] KVM/VMX: Use try_cmpxchg64() in posted_intr.c
  2020-12-15 18:28 [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Uros Bizjak
  2020-12-15 18:28 ` [PATCH 1/3] asm-generic/atomic: Add try_cmpxchg64() instrumentation Uros Bizjak
  2020-12-15 18:28 ` [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64() Uros Bizjak
@ 2020-12-15 18:28 ` Uros Bizjak
  2021-01-15 19:28   ` Sean Christopherson
  2021-01-15 18:09 ` [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Sean Christopherson
  2021-01-18 18:26 ` Paolo Bonzini
  4 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2020-12-15 18:28 UTC (permalink / raw)
  To: x86, kvm, linux-kernel
  Cc: Uros Bizjak, Paolo Bonzini, Sean Christopherson,
	Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel

Use try_cmpxchg64() instead of cmpxchg64() to reuse flags from
cmpxchg/cmpxchg8b instruction. For 64 bit targets flags reuse
avoids a CMP instruction, while for 32 bit targets flags reuse
avoids XOR/XOR/OR instruction sequence.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 arch/x86/kvm/vmx/posted_intr.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx/posted_intr.c b/arch/x86/kvm/vmx/posted_intr.c
index f02962dcc72c..47b47970fb46 100644
--- a/arch/x86/kvm/vmx/posted_intr.c
+++ b/arch/x86/kvm/vmx/posted_intr.c
@@ -60,8 +60,7 @@ void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu)
 			new.ndst = (dest << 8) & 0xFF00;
 
 		new.sn = 0;
-	} while (cmpxchg64(&pi_desc->control, old.control,
-			   new.control) != old.control);
+	} while (!try_cmpxchg64(&pi_desc->control, &old.control, new.control));
 
 after_clear_sn:
 
@@ -111,8 +110,7 @@ static void __pi_post_block(struct kvm_vcpu *vcpu)
 
 		/* set 'NV' to 'notification vector' */
 		new.nv = POSTED_INTR_VECTOR;
-	} while (cmpxchg64(&pi_desc->control, old.control,
-			   new.control) != old.control);
+	} while (!try_cmpxchg64(&pi_desc->control, &old.control, new.control));
 
 	if (!WARN_ON_ONCE(vcpu->pre_pcpu == -1)) {
 		spin_lock(&per_cpu(blocked_vcpu_on_cpu_lock, vcpu->pre_pcpu));
@@ -181,8 +179,7 @@ int pi_pre_block(struct kvm_vcpu *vcpu)
 
 		/* set 'NV' to 'wakeup vector' */
 		new.nv = POSTED_INTR_WAKEUP_VECTOR;
-	} while (cmpxchg64(&pi_desc->control, old.control,
-			   new.control) != old.control);
+	} while (!try_cmpxchg64(&pi_desc->control, &old.control, new.control));
 
 	/* We should not block the vCPU if an interrupt is posted for it.  */
 	if (pi_test_on(pi_desc) == 1)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64()
  2020-12-15 18:28 ` [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64() Uros Bizjak
@ 2020-12-15 20:08   ` Uros Bizjak
  2020-12-16 15:37   ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: Uros Bizjak @ 2020-12-15 20:08 UTC (permalink / raw)
  To: X86 ML, kvm, LKML
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin

On Tue, Dec 15, 2020 at 7:28 PM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> Add arch_try_cmpxchg64(), similar to arch_try_cmpxchg(), that
> operates with 64 bit operands. This function provides the same
> interface for 32 bit and 64 bit targets.
>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> ---
>  arch/x86/include/asm/cmpxchg_32.h | 62 ++++++++++++++++++++++++++-----
>  arch/x86/include/asm/cmpxchg_64.h |  6 +++
>  2 files changed, 59 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/cmpxchg_32.h b/arch/x86/include/asm/cmpxchg_32.h
> index 0a7fe0321613..8dcde400244e 100644
> --- a/arch/x86/include/asm/cmpxchg_32.h
> +++ b/arch/x86/include/asm/cmpxchg_32.h
> @@ -35,15 +35,6 @@ static inline void set_64bit(volatile u64 *ptr, u64 value)
>                      : "memory");
>  }
>
> -#ifdef CONFIG_X86_CMPXCHG64

Oops, I didn't notice that I had left a reversed #ifdef condition in
the patch (to test 32 bit target without X86_CMPXCHG64).

Obviously, CONFIG_X86_CMPXCHG64 has to be defined to use CMPXCHG8B, so
please use #ifdef here.

Uros.

> -#define arch_cmpxchg64(ptr, o, n)                                      \
> -       ((__typeof__(*(ptr)))__cmpxchg64((ptr), (unsigned long long)(o), \
> -                                        (unsigned long long)(n)))
> -#define arch_cmpxchg64_local(ptr, o, n)                                        \
> -       ((__typeof__(*(ptr)))__cmpxchg64_local((ptr), (unsigned long long)(o), \
> -                                              (unsigned long long)(n)))
> -#endif
> -
>  static inline u64 __cmpxchg64(volatile u64 *ptr, u64 old, u64 new)
>  {
>         u64 prev;
> @@ -71,6 +62,39 @@ static inline u64 __cmpxchg64_local(volatile u64 *ptr, u64 old, u64 new)
>  }
>
>  #ifndef CONFIG_X86_CMPXCHG64
> +#define arch_cmpxchg64(ptr, o, n)                                      \
> +       ((__typeof__(*(ptr)))__cmpxchg64((ptr), (unsigned long long)(o), \
> +                                        (unsigned long long)(n)))
> +#define arch_cmpxchg64_local(ptr, o, n)                                        \
> +       ((__typeof__(*(ptr)))__cmpxchg64_local((ptr), (unsigned long long)(o), \
> +
> +#define __raw_try_cmpxchg64(_ptr, _pold, _new, lock)           \
> +({                                                             \
> +       bool success;                                           \
> +       __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold);      \
> +       __typeof__(*(_ptr)) __old = *_old;                      \
> +       __typeof__(*(_ptr)) __new = (_new);                     \
> +       asm volatile(lock "cmpxchg8b %1"                        \
> +                    CC_SET(z)                                  \
> +                    : CC_OUT(z) (success),                     \
> +                      "+m" (*_ptr),                            \
> +                      "+A" (__old)                             \
> +                    : "b" ((unsigned int)__new),               \
> +                      "c" ((unsigned int)(__new>>32))          \
> +                    : "memory");                               \
> +       if (unlikely(!success))                                 \
> +               *_old = __old;                                  \
> +       likely(success);                                        \
> +})
> +
> +#define __try_cmpxchg64(ptr, pold, new)                                \
> +       __raw_try_cmpxchg64((ptr), (pold), (new), LOCK_PREFIX)
> +
> +#define arch_try_cmpxchg64(ptr, pold, new)                     \
> +       __try_cmpxchg64((ptr), (pold), (new))
> +
> +#else
> +
>  /*
>   * Building a kernel capable running on 80386 and 80486. It may be necessary
>   * to simulate the cmpxchg8b on the 80386 and 80486 CPU.
> @@ -108,6 +132,26 @@ static inline u64 __cmpxchg64_local(volatile u64 *ptr, u64 old, u64 new)
>                        : "memory");                             \
>         __ret; })
>
> +#define arch_try_cmpxchg64(ptr, po, n)                         \
> +({                                                             \
> +       bool success;                                           \
> +       __typeof__(ptr) _old = (__typeof__(ptr))(po);           \
> +       __typeof__(*(ptr)) __old = *_old;                       \
> +       __typeof__(*(ptr)) __new = (n);                         \
> +       alternative_io(LOCK_PREFIX_HERE                         \
> +                       "call cmpxchg8b_emu",                   \
> +                       "lock; cmpxchg8b (%%esi)" ,             \
> +                      X86_FEATURE_CX8,                         \
> +                      "+A" (__old),                            \
> +                      "S" ((ptr)),                             \
> +                      "b" ((unsigned int)__new),               \
> +                      "c" ((unsigned int)(__new>>32))          \
> +                      : "memory");                             \
> +       success = (__old == *_old);                             \
> +       if (unlikely(!success))                                 \
> +               *_old = __old;                                  \
> +       likely(success);                                        \
> +})
>  #endif
>
>  #define system_has_cmpxchg_double() boot_cpu_has(X86_FEATURE_CX8)
> diff --git a/arch/x86/include/asm/cmpxchg_64.h b/arch/x86/include/asm/cmpxchg_64.h
> index 072e5459fe2f..250187ac8248 100644
> --- a/arch/x86/include/asm/cmpxchg_64.h
> +++ b/arch/x86/include/asm/cmpxchg_64.h
> @@ -19,6 +19,12 @@ static inline void set_64bit(volatile u64 *ptr, u64 val)
>         arch_cmpxchg_local((ptr), (o), (n));                            \
>  })
>
> +#define arch_try_cmpxchg64(ptr, po, n)                                 \
> +({                                                                     \
> +       BUILD_BUG_ON(sizeof(*(ptr)) != 8);                              \
> +       arch_try_cmpxchg((ptr), (po), (n));                             \
> +})
> +
>  #define system_has_cmpxchg_double() boot_cpu_has(X86_FEATURE_CX16)
>
>  #endif /* _ASM_X86_CMPXCHG_64_H */
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64()
  2020-12-15 18:28 ` [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64() Uros Bizjak
  2020-12-15 20:08   ` Uros Bizjak
@ 2020-12-16 15:37   ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-12-16 15:37 UTC (permalink / raw)
  To: Uros Bizjak, x86, kvm, linux-kernel
  Cc: kbuild-all, Uros Bizjak, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

Hi Uros,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/master]
[also build test ERROR on next-20201215]
[cannot apply to tip/x86/core kvm/linux-next v5.10]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Uros-Bizjak/x86-KVM-VMX-Introduce-and-use-try_cmpxchg64/20201216-024049
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git d1c29f5debd4633eb0e9ea1bc00aaad48b077a9b
config: i386-tinyconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/60a11e7e63e120b5fd41b5346cf5a05ea71c7cb2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Uros-Bizjak/x86-KVM-VMX-Introduce-and-use-try_cmpxchg64/20201216-024049
        git checkout 60a11e7e63e120b5fd41b5346cf5a05ea71c7cb2
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: arch/x86/events/core.o: in function `x86_perf_event_update':
>> core.c:(.text+0x847): undefined reference to `cmpxchg8b_emu'
   ld: kernel/sched/clock.o: in function `sched_clock_local.constprop.0':
>> clock.c:(.text+0x1b4): undefined reference to `cmpxchg8b_emu'
   ld: kernel/sched/clock.o: in function `sched_clock_cpu':
   clock.c:(.text+0x293): undefined reference to `cmpxchg8b_emu'
>> ld: clock.c:(.text+0x2e3): undefined reference to `cmpxchg8b_emu'
   ld: kernel/events/core.o: in function `perf_swevent_set_period':
   core.c:(.text+0x8cbb): undefined reference to `cmpxchg8b_emu'
   ld: fs/inode.o:inode.c:(.text+0x10ca): more undefined references to `cmpxchg8b_emu' follow

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7282 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64()
  2020-12-15 18:28 [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Uros Bizjak
                   ` (2 preceding siblings ...)
  2020-12-15 18:28 ` [PATCH 3/3] KVM/VMX: Use try_cmpxchg64() in posted_intr.c Uros Bizjak
@ 2021-01-15 18:09 ` Sean Christopherson
  2021-01-18 18:26 ` Paolo Bonzini
  4 siblings, 0 replies; 9+ messages in thread
From: Sean Christopherson @ 2021-01-15 18:09 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: x86, kvm, linux-kernel, Will Deacon, Peter Zijlstra, Boqun Feng,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel

On Tue, Dec 15, 2020, Uros Bizjak wrote:
> This patch series introduces try_cmpxchg64() atomic locking function.
> 
> try_cmpxchg64() provides the same interface for 64 bit and 32 bit targets,
> emits CMPXCHGQ for 64 bit targets and CMPXCHG8B for 32 bit targets,
> and provides appropriate fallbacks when CMPXCHG8B is unavailable.
> 
> try_cmpxchg64() reuses flags from CMPXCHGQ/CMPXCHG8B instructions and
> avoids unneeded CMP for 64 bit targets or XOR/XOR/OR sequence for
> 32 bit targets.
> 
> Cc: Will Deacon <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>
> Cc: Jim Mattson <jmattson@google.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> 
> Uros Bizjak (3):
>   asm-generic/atomic: Add try_cmpxchg64() instrumentation
>   locking/atomic/x86: Introduce arch_try_cmpxchg64()
>   KVM/VMX: Use try_cmpxchg64() in posted_intr.c

For anyone else trying to apply this, it depends on v5.11-rc1 (commit
29f006fdefe6, "asm-generic/atomic: Add try_cmpxchg() fallbacks"), which hasn't
yet been merged into Paolo's tree.

>  arch/x86/include/asm/cmpxchg_32.h         | 62 +++++++++++++++++++----
>  arch/x86/include/asm/cmpxchg_64.h         |  6 +++
>  arch/x86/kvm/vmx/posted_intr.c            |  9 ++--
>  include/asm-generic/atomic-instrumented.h | 46 ++++++++++++++++-
>  scripts/atomic/gen-atomic-instrumented.sh |  2 +-
>  5 files changed, 108 insertions(+), 17 deletions(-)
> 
> -- 
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] KVM/VMX: Use try_cmpxchg64() in posted_intr.c
  2020-12-15 18:28 ` [PATCH 3/3] KVM/VMX: Use try_cmpxchg64() in posted_intr.c Uros Bizjak
@ 2021-01-15 19:28   ` Sean Christopherson
  0 siblings, 0 replies; 9+ messages in thread
From: Sean Christopherson @ 2021-01-15 19:28 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: x86, kvm, linux-kernel, Paolo Bonzini, Vitaly Kuznetsov,
	Wanpeng Li, Jim Mattson, Joerg Roedel

On Tue, Dec 15, 2020, Uros Bizjak wrote:
> Use try_cmpxchg64() instead of cmpxchg64() to reuse flags from
> cmpxchg/cmpxchg8b instruction. For 64 bit targets flags reuse
> avoids a CMP instruction,

It ends up doing way more (in a good way) than eliminate the CMP, at least with
gcc-10.  There's a ripple effect and the compiler ends up generating the loop
in-line, whereas without the "try" version the loop is put out-of-line.

> while for 32 bit targets flags reuse avoids XOR/XOR/OR instruction sequence.
> 
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>

Reviewed-by: Sean Christopherson <seanjc@google.com> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64()
  2020-12-15 18:28 [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Uros Bizjak
                   ` (3 preceding siblings ...)
  2021-01-15 18:09 ` [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Sean Christopherson
@ 2021-01-18 18:26 ` Paolo Bonzini
  4 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2021-01-18 18:26 UTC (permalink / raw)
  To: Uros Bizjak, x86, kvm, linux-kernel
  Cc: Will Deacon, Peter Zijlstra, Boqun Feng, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel

On 15/12/20 19:28, Uros Bizjak wrote:
> This patch series introduces try_cmpxchg64() atomic locking function.
> 
> try_cmpxchg64() provides the same interface for 64 bit and 32 bit targets,
> emits CMPXCHGQ for 64 bit targets and CMPXCHG8B for 32 bit targets,
> and provides appropriate fallbacks when CMPXCHG8B is unavailable.
> 
> try_cmpxchg64() reuses flags from CMPXCHGQ/CMPXCHG8B instructions and
> avoids unneeded CMP for 64 bit targets or XOR/XOR/OR sequence for
> 32 bit targets.
> 
> Cc: Will Deacon <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>
> Cc: Jim Mattson <jmattson@google.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> 
> Uros Bizjak (3):
>    asm-generic/atomic: Add try_cmpxchg64() instrumentation
>    locking/atomic/x86: Introduce arch_try_cmpxchg64()
>    KVM/VMX: Use try_cmpxchg64() in posted_intr.c
> 
>   arch/x86/include/asm/cmpxchg_32.h         | 62 +++++++++++++++++++----
>   arch/x86/include/asm/cmpxchg_64.h         |  6 +++
>   arch/x86/kvm/vmx/posted_intr.c            |  9 ++--
>   include/asm-generic/atomic-instrumented.h | 46 ++++++++++++++++-
>   scripts/atomic/gen-atomic-instrumented.sh |  2 +-
>   5 files changed, 108 insertions(+), 17 deletions(-)
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-01-18 18:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 18:28 [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Uros Bizjak
2020-12-15 18:28 ` [PATCH 1/3] asm-generic/atomic: Add try_cmpxchg64() instrumentation Uros Bizjak
2020-12-15 18:28 ` [PATCH 2/3] locking/atomic/x86: Introduce arch_try_cmpxchg64() Uros Bizjak
2020-12-15 20:08   ` Uros Bizjak
2020-12-16 15:37   ` kernel test robot
2020-12-15 18:28 ` [PATCH 3/3] KVM/VMX: Use try_cmpxchg64() in posted_intr.c Uros Bizjak
2021-01-15 19:28   ` Sean Christopherson
2021-01-15 18:09 ` [PATCH 0/3] x86/KVM/VMX: Introduce and use try_cmpxchg64() Sean Christopherson
2021-01-18 18:26 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).