| Message ID | 20250214173644.22895-1-nsaenz@amazon.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [kvm-unit-tests] x86: Make set/clear_bit() atomic | expand |
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.
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
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"); }
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(-)