[kvm-unit-tests PATCH] x86: Make set/clear_bit() atomic

[kvm-unit-tests PATCH] x86: Make set/clear_bit() atomic

[Nicolas Saenz Julienne]

x86 is the only architecture that defines set/clear_bit() as non-atomic.
This makes it incompatible with arch-agnostic code that might implicitly
require atomicity. And it was observed to corrupt the 'online_cpus'
bitmap, as non BSP CPUs perform RmWs on the bitmap concurrently during
bring up. See:

ap_start64()
  save_id()
    set_bit(apic_id(), online_cpus)

Address this by making set/clear_bit() atomic.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
---
 lib/x86/processor.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index da1ed662..82507787 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -843,13 +843,13 @@ static inline bool is_canonical(u64 addr)
 
 static inline void clear_bit(int bit, u8 *addr)
 {
-	__asm__ __volatile__("btr %1, %0"
+	__asm__ __volatile__("lock; btr %1, %0"
 			     : "+m" (*addr) : "Ir" (bit) : "cc", "memory");
 }
 
 static inline void set_bit(int bit, u8 *addr)
 {
-	__asm__ __volatile__("bts %1, %0"
+	__asm__ __volatile__("lock; bts %1, %0"
 			     : "+m" (*addr) : "Ir" (bit) : "cc", "memory");
 }
 
-- 
2.47.1

Re: [kvm-unit-tests PATCH] x86: Make set/clear_bit() atomic

[Sean Christopherson]

On Fri, Feb 14, 2025, Nicolas Saenz Julienne wrote:
> x86 is the only architecture that defines set/clear_bit() as non-atomic.
> This makes it incompatible with arch-agnostic code that might implicitly
> require atomicity. And it was observed to corrupt the 'online_cpus'
> bitmap, as non BSP CPUs perform RmWs on the bitmap concurrently during
> bring up. See:
> 
> ap_start64()
>   save_id()
>     set_bit(apic_id(), online_cpus)
> 
> Address this by making set/clear_bit() atomic.

OMG, this is arguaby worse than the per-CPU stack/data mess.  *sigh*

I'll grab this, I'm putting together a pull request for a few things.

Re: [kvm-unit-tests PATCH] x86: Make set/clear_bit() atomic

[Sean Christopherson]

On Fri, 14 Feb 2025 17:36:44 +0000, Nicolas Saenz Julienne wrote:
> x86 is the only architecture that defines set/clear_bit() as non-atomic.
> This makes it incompatible with arch-agnostic code that might implicitly
> require atomicity. And it was observed to corrupt the 'online_cpus'
> bitmap, as non BSP CPUs perform RmWs on the bitmap concurrently during
> bring up. See:
> 
> ap_start64()
>   save_id()
>     set_bit(apic_id(), online_cpus)
> 
> [...]

Applied to kvm-x86 next (and now pulled by Paolo), thanks!

[1/1] x86: Make set/clear_bit() atomic
      https://github.com/kvm-x86/kvm-unit-tests/commit/2f3c02862e03

--
https://github.com/kvm-x86/kvm-unit-tests/tree/next