Re: [PATCH] KVM: x86: Add support for cmpxchg16b emulation

Paolo Bonzini
2026-03-12T11:46:33Z

On 3/6/26 16:38, Sean Christopherson wrote:
>> - if (ctxt->dst.bytes == 16)
>> + /* Use of the REX.W prefix promotes operation to 128 bits */
>> + if (ctxt->rex_bits & REX_W)
>
> Uh, yeah, and the caller specifically pivoted on this size. I'm a-ok with a
> sanity check, but it should be exactly that, e.g.
>
> if (WARN_ON_ONCE(8 + (ctxt->rex_bits & REX_W) * 8 != ctxt->dst.bytes))
> return X86EMUL_UNHANDLEABLE;

> And then emulator_cmpxchg_emulated() should be taught to do cmpxchg16b itself:
>
> /* guests cmpxchg8b have to be emulated atomically */
> if (bytes > 8 || (bytes & (bytes - 1)))
> goto emul_write;
>
> That might be a big lift, e.g. to get a 16-byte uaccess version. If it's
> unreasonably difficult, we should reject emulation of LOCK CMPXCHG16B.

It's actually pretty easy because a kind soul already wrote the
cmpxchg8b version of the macros. Compile-tested only (and checked
that the asm does have the instruction):

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 3a7755c1a4410..f40e815263233 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -388,14 +388,13 @@ do { \
*_old = __old; \
likely(success); })

-#ifdef CONFIG_X86_32
-#define __try_cmpxchg64_user_asm(_ptr, _pold, _new, label) ({ \
+#define __try_cmpxchg8b_16b_user_asm(_b, _ptr, _pold, _new, label)({ \
bool success; \
__typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
__typeof__(*(_ptr)) __old = *_old; \
__typeof__(*(_ptr)) __new = (_new); \
asm_goto_output("\n" \
- "1: " LOCK_PREFIX "cmpxchg8b %[ptr]\n" \
+ "1: " LOCK_PREFIX "cmpxchg" #_b "b %[ptr]\n" \
_ASM_EXTABLE_UA(1b, %l[label]) \
: CC_OUT(z) (success), \
"+A" (__old), \
@@ -407,7 +406,6 @@ do { \
if (unlikely(!success)) \
*_old = __old; \
likely(success); })
-#endif // CONFIG_X86_32
#else // !CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
#define __try_cmpxchg_user_asm(itype, ltype, _ptr, _pold, _new, label) ({ \
int __err = 0; \
@@ -433,7 +431,6 @@ do { \
*_old = __old; \
likely(success); })

-#ifdef CONFIG_X86_32
/*
* Unlike the normal CMPXCHG, use output GPR for both success/fail and error.
* There are only six GPRs available and four (EAX, EBX, ECX, and EDX) are
@@ -441,13 +438,13 @@ do { \
* both ESI and EDI for the memory operand, compilation will fail if the error
* is an input+output as there will be no register available for input.
*/
-#define __try_cmpxchg64_user_asm(_ptr, _pold, _new, label) ({ \
+#define __try_cmpxchg8b_16b_user_asm(_b, _ptr, _pold, _new, label) ({ \
int __result; \
__typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
__typeof__(*(_ptr)) __old = *_old; \
__typeof__(*(_ptr)) __new = (_new); \
asm volatile("\n" \
- "1: " LOCK_PREFIX "cmpxchg8b %[ptr]\n" \
+ "1: " LOCK_PREFIX "cmpxchg" #_b "b %[ptr]\n" \
"mov $0, %[result]\n\t" \
"setz %b[result]\n" \
"2:\n" \
@@ -464,7 +461,6 @@ do { \
if (unlikely(!__result)) \
*_old = __old; \
likely(__result); })
-#endif // CONFIG_X86_32
#endif // CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT

/* FIXME: this hack is definitely wrong -AK */
@@ -552,9 +549,15 @@ do { \

extern void __try_cmpxchg_user_wrong_size(void);

-#ifndef CONFIG_X86_32
+#ifdef CONFIG_X86_32
+#define __try_cmpxchg64_user_asm(_ptr, _pold, _new, label) \
+ __try_cmpxchg8b_16b_user_asm(8, _ptr, _pold, _new, label)
+#else
#define __try_cmpxchg64_user_asm(_ptr, _oldp, _nval, _label) \
__try_cmpxchg_user_asm("q", "r", (_ptr), (_oldp), (_nval), _label)
+
+#define __try_cmpxchg128_user_asm(_ptr, _pold, _new, label) \
+ __try_cmpxchg8b_16b_user_asm(16, _ptr, _pold, _new, label)
#endif

/*
@@ -581,6 +584,9 @@ extern void __try_cmpxchg_user_wrong_size(void);
case 8: __ret = __try_cmpxchg64_user_asm((__force u64 *)(_ptr), (_oldp),\
(_nval), _label); \
break; \
+ case 16:__ret = __try_cmpxchg128_user_asm((__force u128 *)(_ptr), (_oldp),\
+ (_nval), _label); \
+ break; \
default: __try_cmpxchg_user_wrong_size(); \
} \
__ret; })
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f2db04aa6a17f..3e1bd9cdd75e6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7973,7 +7973,7 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
int r;

/* guests cmpxchg8b have to be emulated atomically */
- if (bytes > 8 || (bytes & (bytes - 1)))
+ if (bytes > 2 * sizeof(unsigned long) || (bytes & (bytes - 1)))
goto emul_write;

gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, NULL);
@@ -8013,6 +8013,11 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
case 8:
r = emulator_try_cmpxchg_user(u64, hva, old, new);
break;
+#ifdef CONFIG_X86_64
+ case 16:
+ r = emulator_try_cmpxchg_user(u128, hva, old, new);
+ break;
+#endif
default:
BUG();
}

(with the uaccess.h part to be split in a separate patch, of course).

Paolo