[PATCH v3 0/2] Add support for cmpxchg16b emulation

[PATCH v3 0/2] Add support for cmpxchg16b emulation

[Sairaj Kodilkar]

This series adds 128-bit compare-exchange support needed for KVM to
emulate guest cmpxchg16b instructions.

The AMD IOMMU driver updates 256-bit device table entries with two
128-bit cmpxchg operations.  For hardware-accelerated vIOMMU, QEMU traps
those DTE accesses to install nested page tables [1].  KVM must emulate
guest cmpxchg16b on the trapped MMIO regions; without that, DTE access
emulation fails.

Patch 1: extends the x86 user CMPXCHG helpers with
__try_cmpxchg128_user_asm() (cmpxchg16b on x86-64), wired into
unsafe_try_cmpxchg_user().

Patch 2: extends KVM CMPXCHG8B emulation to 16-byte operands (REX.W=1) and
uses the new helper for atomic 16-byte compare-exchange on guest RAM via
emulator_cmpxchg_emulated().

You can find the KUT to test this series at [2].

[1] https://github.com/AMDESE/qemu-iommu/blob/wip/for_iommufd_hw_queue-v8_amd_viommu_20260106/hw/i386/amd_viommu.c#L517
[2] https://lore.kernel.org/kvm/20260706062153.346-1-sarunkod@amd.com/T/#u

Changes since v2:
https://lore.kernel.org/all/20260706063035.1139-1-sarunkod@amd.com/
Patch 1: Trigger build failure if __try_cmpxchg128_user_asm is called on 32 bit
	 machines [Sean]

Sairaj Kodilkar (2):
  x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands
  KVM: x86: Add support for cmpxchg16b emulation

 arch/x86/include/asm/uaccess.h | 56 +++++++++++++++++++++++++++++++++-
 arch/x86/kvm/emulate.c         | 50 ++++++++++++++++++++----------
 arch/x86/kvm/kvm_emulate.h     |  6 ++++
 arch/x86/kvm/x86.c             |  7 ++++-
 4 files changed, 101 insertions(+), 18 deletions(-)


base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
-- 
2.34.1

[PATCH v3 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[Sairaj Kodilkar]

Extend the existing user CMPXCHG helpers to support 16-byte operands on
x86-64, using LOCK_PREFIX "cmpxchg16b".  This mirrors the existing
__try_cmpxchg64_user_asm() / cmpxchg8b path provided for 32-bit kernels,
where KVM needs an atomic compare-exchange wider than the generic
cmpxchg helper can provide.

On 32-bit kernels, stub the helper to always return failure because cmpxchg16b
requires 64-bit GPRs and is not available.

KVM uses this to atomically emulate guest cmpxchg16b on guest RAM mapped
via userspace addresses.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 arch/x86/include/asm/uaccess.h | 56 +++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 3a0dd3c2b233..39bcf327664e 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -407,6 +407,25 @@ do {									\
 	if (unlikely(!success))						\
 		*_old = __old;						\
 	likely(success);					})
+#else // !CONFIG_X86_32
+#define __try_cmpxchg128_user_asm(_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 "cmpxchg16b %[ptr]\n"		\
+		     _ASM_EXTABLE_UA(1b, %l[label])			\
+		     : "=@ccz" (success),				\
+		       "+A" (__old),					\
+		       [ptr] "+m" (*_ptr)				\
+		     : "b" ((u64)__new),				\
+		       "c" ((u64)((u128)__new >> 64))			\
+		     : "memory"						\
+		     : label);						\
+	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)	({ \
@@ -463,6 +482,30 @@ do {									\
 	if (unlikely(!__result))					\
 		*_old = __old;						\
 	likely(__result);					})
+#else //!CONFIG_X86_32
+#define __try_cmpxchg128_user_asm(_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 "cmpxchg16b %[ptr]\n"		\
+		     "mov $0, %[result]\n\t"				\
+		     "setz %b[result]\n"				\
+		     "2:\n"						\
+		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG,	\
+					   %[result])			\
+		     : [result] "=q" (__result),			\
+		       "+A" (__old),					\
+		       [ptr] "+m" (*_ptr)				\
+		     : "b" ((u64)__new),				\
+		       "c" ((u64)((u128)__new >> 64))			\
+		     : "memory", "cc");					\
+	if (unlikely(__result < 0))					\
+		goto label;						\
+	if (unlikely(!__result))					\
+		*_old = __old;						\
+	likely(__result);					})
 #endif // CONFIG_X86_32
 #endif // CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
 
@@ -551,11 +594,18 @@ do {										\
 
 extern void __try_cmpxchg_user_wrong_size(void);
 
-#ifndef CONFIG_X86_32
+#ifdef CONFIG_X86_32
+/* Always fail on 32 bit arch as it do not support 128 cmpxchg (i.e. cmpxchg16b
+ * instruction).
+ */
+#define __try_cmpxchg128_user_asm(_ptr, _pold, _new, label) ({ BUILD_BUG_ON(1); 0; })
+#else
 #define __try_cmpxchg64_user_asm(_ptr, _oldp, _nval, _label)		\
 	__try_cmpxchg_user_asm("q", "r", (_ptr), (_oldp), (_nval), _label)
+
 #endif
 
+
 /*
  * Force the pointer to u<size> to match the size expected by the asm helper.
  * clang/LLVM compiles all cases and only discards the unused paths after
@@ -580,6 +630,10 @@ 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;						})
-- 
2.34.1

[PATCH v3 2/2] KVM: x86: Add support for cmpxchg16b emulation

[Sairaj Kodilkar]

AMD and Intel both provides support for 128 bit cmpxchg operands using
cmpxchg8b/cmpxchg16b instructions (opcode 0FC7). However, kvm does not
support emulating cmpxchg16b (i.e when destination memory is 128 bit and
REX.W = 1) which causes emulation failure when QEMU guest performs a
cmpxchg16b on a memory region setup as a IO.

This failure is seen on the AMD IOMMU driver which writes 256-bit device
table entries with two 128-bit cmpxchg operations. For guests using
hardware-accelerated vIOMMU, QEMU traps device table accesses to set up
nested page tables (see [1]).  Without 128-bit cmpxchg emulation, KVM
cannot handle these traps and DTE access emulation fails.

Hence extend cmpxchg8b to perform cmpxchg16b when the destination memory
is 128 bit.

[1] https://github.com/AMDESE/qemu-iommu/blob/wip/for_iommufd_hw_queue-v8_amd_viommu_20260106/hw/i386/amd_viommu.c#L517

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 arch/x86/kvm/emulate.c     | 50 ++++++++++++++++++++++++++------------
 arch/x86/kvm/kvm_emulate.h |  6 +++++
 arch/x86/kvm/x86.c         |  7 +++++-
 3 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c1b21282187f..f38c4f1a99c3 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2184,24 +2184,36 @@ static int em_call_near_abs(struct x86_emulate_ctxt *ctxt)
 	return rc;
 }
 
+#define em_cmpxchg8b_16b(__c, rbits, mbits)\
+do {												\
+	u##mbits old = __c->dst.orig_val##mbits;						\
+												\
+	BUILD_BUG_ON(rbits * 2 != mbits);							\
+												\
+	if (((u##rbits) (old >> 0) != (u##rbits) reg_read(ctxt, VCPU_REGS_RAX)) ||		\
+	    ((u##rbits) (old >> rbits) != (u##rbits) reg_read(ctxt, VCPU_REGS_RDX))) {		\
+		*reg_write(ctxt, VCPU_REGS_RAX) = (u##rbits) (old >> 0);			\
+		*reg_write(ctxt, VCPU_REGS_RDX) = (u##rbits) (old >> rbits);			\
+		ctxt->eflags &= ~X86_EFLAGS_ZF;							\
+	} else {										\
+		ctxt->dst.val##mbits = ((u##mbits)reg_read(ctxt, VCPU_REGS_RCX) << rbits) |	\
+					(u##rbits) reg_read(ctxt, VCPU_REGS_RBX);		\
+												\
+		ctxt->eflags |= X86_EFLAGS_ZF;							\
+	}											\
+} while(0)
+
 static int em_cmpxchg8b(struct x86_emulate_ctxt *ctxt)
 {
-	u64 old = ctxt->dst.orig_val64;
-
-	if (ctxt->dst.bytes == 16)
+	if (WARN_ON_ONCE(8 + !!(ctxt->rex_bits & REX_W) * 8 != ctxt->dst.bytes))
 		return X86EMUL_UNHANDLEABLE;
 
-	if (((u32) (old >> 0) != (u32) reg_read(ctxt, VCPU_REGS_RAX)) ||
-	    ((u32) (old >> 32) != (u32) reg_read(ctxt, VCPU_REGS_RDX))) {
-		*reg_write(ctxt, VCPU_REGS_RAX) = (u32) (old >> 0);
-		*reg_write(ctxt, VCPU_REGS_RDX) = (u32) (old >> 32);
-		ctxt->eflags &= ~X86_EFLAGS_ZF;
-	} else {
-		ctxt->dst.val64 = ((u64)reg_read(ctxt, VCPU_REGS_RCX) << 32) |
-			(u32) reg_read(ctxt, VCPU_REGS_RBX);
-
-		ctxt->eflags |= X86_EFLAGS_ZF;
-	}
+	if (!(ctxt->rex_bits & REX_W))
+		em_cmpxchg8b_16b(ctxt, 32, 64);
+#ifdef CONFIG_X86_64
+	else
+		em_cmpxchg8b_16b(ctxt, 64, 128);
+#endif
 	return X86EMUL_CONTINUE;
 }
 
@@ -5414,8 +5426,14 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt, bool check_intercepts)
 			goto done;
 		}
 	}
-	/* Copy full 64-bit value for CMPXCHG8B.  */
-	ctxt->dst.orig_val64 = ctxt->dst.val64;
+	/* Copy full 64/128-bit value for CMPXCHG8B.  */
+
+#ifdef CONFIG_X86_64
+	if (ctxt->dst.bytes == 16)
+		ctxt->dst.orig_val128 = ctxt->dst.val128;
+	else
+#endif
+		ctxt->dst.orig_val64 = ctxt->dst.val64;
 
 special_insn:
 
diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 3e375af15c03..89911845233d 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -263,6 +263,9 @@ struct operand {
 	union {
 		unsigned long orig_val;
 		u64 orig_val64;
+#ifdef CONFIG_X86_64
+		u128 orig_val128;
+#endif
 	};
 	union {
 		unsigned long *reg;
@@ -276,6 +279,9 @@ struct operand {
 	union {
 		unsigned long val;
 		u64 val64;
+#ifdef CONFIG_X86_64
+		u128 val128;
+#endif
 		char valptr[sizeof(avx256_t)];
 		sse128_t vec_val;
 		avx256_t vec_val2;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 79468ddfe473..60be16f05d79 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5271,7 +5271,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);
@@ -5311,6 +5311,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();
 	}
-- 
2.34.1

Re: [PATCH v3 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The inline assembly for 128-bit cmpxchg uses the unreliable `+A` constraint on x86-64, risking silent memory corruption or build failures.
- [Medium] The 32-bit stub for `__try_cmpxchg128_user_asm` relies on `BUILD_BUG_ON(1)`, which will break the 32-bit build for all callers of `unsafe_try_cmpxchg_user`.
--

commit 778924f40bf568ddfaeffc09239d3c0e2a09bba6
Author: Sairaj Kodilkar <sarunkod@amd.com>

x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

This commit extends the existing user CMPXCHG helpers to support 16-byte
operands on x86-64 using the "cmpxchg16b" instruction. On 32-bit kernels,
it stubs the helper to fail, as cmpxchg16b requires 64-bit registers. KVM
uses this to atomically emulate guest cmpxchg16b on guest RAM mapped via
userspace addresses.

> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index 3a0dd3c2b233e..39bcf327664ea 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -407,6 +407,25 @@ do {									\
[ ... ]
> +#else // !CONFIG_X86_32
> +#define __try_cmpxchg128_user_asm(_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 "cmpxchg16b %[ptr]\n"		\
> +		     _ASM_EXTABLE_UA(1b, %l[label])			\
> +		     : "=@ccz" (success),				\
> +		       "+A" (__old),					\

[Severity: High]
Does using the "+A" constraint for a 128-bit value provide the strict
rdx:rax register ordering required by the cmpxchg16b instruction?

The kernel documents in arch/x86/include/asm/asm.h that the "A" constraint
on x86-64 means rax or rdx, rather than the rdx:rax pair. If the compiler
does not guarantee the rdx:rax ordering for __int128/u128 with "+A", could
this lead to silent memory corruption with swapped operands, or outright
build failures on compilers like Clang?

The standard pattern used elsewhere (like __arch_cmpxchg128 in
arch/x86/include/asm/cmpxchg_64.h) splits the 128-bit value into a union
of two 64-bit halves and explicitly binds them to "+a" (rax) and "+d" (rdx).
Would it be safer to use that pattern here?

> +		       [ptr] "+m" (*_ptr)				\
> +		     : "b" ((u64)__new),				\
> +		       "c" ((u64)((u128)__new >> 64))			\
> +		     : "memory"						\
> +		     : label);						\
> +	if (unlikely(!success))						\
> +		*_old = __old;						\
> +	likely(success);					})
>  #endif // CONFIG_X86_32
>  #else  // !CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
[ ... ]
> +#else //!CONFIG_X86_32
> +#define __try_cmpxchg128_user_asm(_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 "cmpxchg16b %[ptr]\n"		\
> +		     "mov $0, %[result]\n\t"				\
> +		     "setz %b[result]\n"				\
> +		     "2:\n"						\
> +		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG,	\
> +					   %[result])			\
> +		     : [result] "=q" (__result),			\
> +		       "+A" (__old),					\

[Severity: High]
Similar to the asm_goto_output block above, this fallback path also binds
the 128-bit __old variable to the "+A" constraint.

> +		       [ptr] "+m" (*_ptr)				\
> +		     : "b" ((u64)__new),				\
> +		       "c" ((u64)((u128)__new >> 64))			\
> +		     : "memory", "cc");					\
> +	if (unlikely(__result < 0))					\
> +		goto label;						\
> +	if (unlikely(!__result))					\
> +		*_old = __old;						\
> +	likely(__result);					})
>  #endif // CONFIG_X86_32
>  #endif // CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
[ ... ]
> @@ -551,11 +594,18 @@ do {										\
>  
>  extern void __try_cmpxchg_user_wrong_size(void);
>  
> -#ifndef CONFIG_X86_32
> +#ifdef CONFIG_X86_32
> +/* Always fail on 32 bit arch as it do not support 128 cmpxchg (i.e. cmpxchg16b
> + * instruction).
> + */
> +#define __try_cmpxchg128_user_asm(_ptr, _pold, _new, label) ({ BUILD_BUG_ON(1); 0; })

[Severity: Medium]
Will this BUILD_BUG_ON(1) break 32-bit x86 builds that call the
unsafe_try_cmpxchg_user() macro for perfectly valid 32-bit or 64-bit sizes?

Because unsafe_try_cmpxchg_user() dispatches via a switch(sizeof(*(_ptr)))
statement, compilers like Clang/LLVM eagerly evaluate all branches during
semantic analysis, including the 16-byte branch. If evaluated on 32-bit
builds, BUILD_BUG_ON(1) will trigger a compile-time error regardless of
whether the 16-byte path is actually reachable.

Since the commit message says "stub the helper to always return failure",
would it be better to return a runtime error code here instead of using
a build bug?

> +#else
>  #define __try_cmpxchg64_user_asm(_ptr, _oldp, _nval, _label)		\
>  	__try_cmpxchg_user_asm("q", "r", (_ptr), (_oldp), (_nval), _label)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826070004.8100-1-sarunkod@amd.com?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Emulation of CMPXCHG16B lacks the mandatory 16-byte alignment check, leading to a guest-triggerable VM crash.
- [Low] The macro `em_cmpxchg8b_16b` incorrectly captures the variable `ctxt` from the surrounding scope instead of solely using its argument `__c`.

Pre-existing issues:
- [Critical] TOCTOU vulnerability in atomic CMPXCHG emulation leads to silent data corruption in the guest when a native compare fails.
--

commit f6a8a67f38b03c4522f119ac54d63d97691e1289
Author: Sairaj Kodilkar <sarunkod@amd.com>

KVM: x86: Add support for cmpxchg16b emulation

This patch adds support for emulating cmpxchg16b when the destination memory
is 128-bit and REX.W = 1. This prevents QEMU guests from hitting emulation
failures during 256-bit device table entries accesses on AMD vIOMMU.

> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index c1b21282187f3..f38c4f1a99c31 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c

[ ... ]

> @@ -2184,24 +2184,36 @@ static int em_call_near_abs(struct x86_emulate_ctxt *ctxt)
>  	return rc;
>  }
>  
> +#define em_cmpxchg8b_16b(__c, rbits, mbits)\
> +do {												\
> +	u##mbits old = __c->dst.orig_val##mbits;						\
> +												\
> +	BUILD_BUG_ON(rbits * 2 != mbits);							\
> +												\
> +	if (((u##rbits) (old >> 0) != (u##rbits) reg_read(ctxt, VCPU_REGS_RAX)) ||		\

[Severity: Low]
Does this macro intentionally reference the unpassed variable ctxt instead of
the provided argument __c?

This uses ctxt to read registers, which makes the macro rely on the specific
variable name ctxt in the caller's scope instead of its parameter.

> +	    ((u##rbits) (old >> rbits) != (u##rbits) reg_read(ctxt, VCPU_REGS_RDX))) {		\
> +		*reg_write(ctxt, VCPU_REGS_RAX) = (u##rbits) (old >> 0);			\
> +		*reg_write(ctxt, VCPU_REGS_RDX) = (u##rbits) (old >> rbits);			\
> +		ctxt->eflags &= ~X86_EFLAGS_ZF;							\
> +	} else {										\
> +		ctxt->dst.val##mbits = ((u##mbits)reg_read(ctxt, VCPU_REGS_RCX) << rbits) |	\
> +					(u##rbits) reg_read(ctxt, VCPU_REGS_RBX);		\
> +												\
> +		ctxt->eflags |= X86_EFLAGS_ZF;							\
> +	}											\
> +} while(0)
> +
>  static int em_cmpxchg8b(struct x86_emulate_ctxt *ctxt)
>  {
> -	u64 old = ctxt->dst.orig_val64;
> -
> -	if (ctxt->dst.bytes == 16)
> +	if (WARN_ON_ONCE(8 + !!(ctxt->rex_bits & REX_W) * 8 != ctxt->dst.bytes))
>  		return X86EMUL_UNHANDLEABLE;
>  
> -	if (((u32) (old >> 0) != (u32) reg_read(ctxt, VCPU_REGS_RAX)) ||
> -	    ((u32) (old >> 32) != (u32) reg_read(ctxt, VCPU_REGS_RDX))) {
> -		*reg_write(ctxt, VCPU_REGS_RAX) = (u32) (old >> 0);
> -		*reg_write(ctxt, VCPU_REGS_RDX) = (u32) (old >> 32);
> -		ctxt->eflags &= ~X86_EFLAGS_ZF;
> -	} else {
> -		ctxt->dst.val64 = ((u64)reg_read(ctxt, VCPU_REGS_RCX) << 32) |
> -			(u32) reg_read(ctxt, VCPU_REGS_RBX);
> -
> -		ctxt->eflags |= X86_EFLAGS_ZF;
> -	}
> +	if (!(ctxt->rex_bits & REX_W))
> +		em_cmpxchg8b_16b(ctxt, 32, 64);
> +#ifdef CONFIG_X86_64
> +	else
> +		em_cmpxchg8b_16b(ctxt, 64, 128);
> +#endif

[Severity: High]
Are we missing a 16-byte alignment check for the destination memory when
executing CMPXCHG16B here?

The x86 architecture strictly mandates that the memory operand for CMPXCHG16B
must be 16-byte aligned. If it is not, a #GP(0) exception is generated
unconditionally.

Without this check, KVM will later attempt a native unaligned lock cmpxchg16b,
which will raise a host #GP(0). The host exception table catches this and
returns -EFAULT, translating to X86EMUL_UNHANDLEABLE, which immediately
crashes the VM via KVM_EXIT_INTERNAL_ERROR. Can this regression be used by a
guest to trigger a denial of service?

>  	return X86EMUL_CONTINUE;
>  }

[ ... ]

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 79468ddfe4736..60be16f05d794 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c

[ ... ]

> @@ -5311,6 +5311,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

[Severity: Critical]
This is a pre-existing issue, but does the emulation of atomic CMPXCHG suffer
from a TOCTOU race leading to silent data corruption in the guest?

When emulator_try_cmpxchg_user() fails due to concurrent memory modification,
emulator_cmpxchg_emulated() returns X86EMUL_CMPXCHG_FAILED.

However, in x86_emulate_insn(), this failure appears to be silently swallowed:

arch/x86/kvm/emulate.c:x86_emulate_insn() {
    ...
    if (rc == X86EMUL_CONTINUE)
        writeback_registers(ctxt);

    return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
}

Because rc is not X86EMUL_CONTINUE, writeback_registers() is skipped and the
guest registers are not updated. Because EMULATION_OK is returned, KVM retires
the instruction as successful, advancing RIP and leaving the guest with a
stale ZF=1 flag. Doesn't this cause the guest to mistakenly believe it
acquired a lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826070004.8100-1-sarunkod@amd.com?part=2

Re: [PATCH v3 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[Sairaj Kodilkar]

On 8/26/2026 12:53 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The inline assembly for 128-bit cmpxchg uses the unreliable `+A` constraint on x86-64, risking silent memory corruption or build failures.
> - [Medium] The 32-bit stub for `__try_cmpxchg128_user_asm` relies on `BUILD_BUG_ON(1)`, which will break the 32-bit build for all callers of `unsafe_try_cmpxchg_user`.
> --
> 
> commit 778924f40bf568ddfaeffc09239d3c0e2a09bba6
> Author: Sairaj Kodilkar <sarunkod@amd.com>
> 
> x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands
> 
> This commit extends the existing user CMPXCHG helpers to support 16-byte
> operands on x86-64 using the "cmpxchg16b" instruction. On 32-bit kernels,
> it stubs the helper to fail, as cmpxchg16b requires 64-bit registers. KVM
> uses this to atomically emulate guest cmpxchg16b on guest RAM mapped via
> userspace addresses.
> 
>> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
>> index 3a0dd3c2b233e..39bcf327664ea 100644
>> --- a/arch/x86/include/asm/uaccess.h
>> +++ b/arch/x86/include/asm/uaccess.h
>> @@ -407,6 +407,25 @@ do {									\
> [ ... ]
>> +#else // !CONFIG_X86_32
>> +#define __try_cmpxchg128_user_asm(_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 "cmpxchg16b %[ptr]\n"		\
>> +		     _ASM_EXTABLE_UA(1b, %l[label])			\
>> +		     : "=@ccz" (success),				\
>> +		       "+A" (__old),					\
> 
> [Severity: High]
> Does using the "+A" constraint for a 128-bit value provide the strict
> rdx:rax register ordering required by the cmpxchg16b instruction?
> 
> The kernel documents in arch/x86/include/asm/asm.h that the "A" constraint
> on x86-64 means rax or rdx, rather than the rdx:rax pair. If the compiler
> does not guarantee the rdx:rax ordering for __int128/u128 with "+A", could
> this lead to silent memory corruption with swapped operands, or outright
> build failures on compilers like Clang?
> 
> The standard pattern used elsewhere (like __arch_cmpxchg128 in
> arch/x86/include/asm/cmpxchg_64.h) splits the 128-bit value into a union
> of two 64-bit halves and explicitly binds them to "+a" (rax) and "+d" (rdx).
> Would it be safer to use that pattern here?

It is easy to misread/misunderstand the official gcc documentations [1].
It specifically mentions that
"Single word values will be allocated either in ax or dx".

Here on x86_64 arch, the __old is 128 bit (i.e. it cannot fit in single
register) so the compiler will always split it in rdx:rax pair.


>> +		       [ptr] "+m" (*_ptr)				\
>> +		     : "b" ((u64)__new),				\
>> +		       "c" ((u64)((u128)__new >> 64))			\
>> +		     : "memory"						\
>> +		     : label);						\
>> +	if (unlikely(!success))						\
>> +		*_old = __old;						\
>> +	likely(success);					})
>>  #endif // CONFIG_X86_32
>>  #else  // !CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
> [ ... ]
>> +#else //!CONFIG_X86_32
>> +#define __try_cmpxchg128_user_asm(_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 "cmpxchg16b %[ptr]\n"		\
>> +		     "mov $0, %[result]\n\t"				\
>> +		     "setz %b[result]\n"				\
>> +		     "2:\n"						\
>> +		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG,	\
>> +					   %[result])			\
>> +		     : [result] "=q" (__result),			\
>> +		       "+A" (__old),					\
> 
> [Severity: High]
> Similar to the asm_goto_output block above, this fallback path also binds
> the 128-bit __old variable to the "+A" constraint.
> 
>> +		       [ptr] "+m" (*_ptr)				\
>> +		     : "b" ((u64)__new),				\
>> +		       "c" ((u64)((u128)__new >> 64))			\
>> +		     : "memory", "cc");					\
>> +	if (unlikely(__result < 0))					\
>> +		goto label;						\
>> +	if (unlikely(!__result))					\
>> +		*_old = __old;						\
>> +	likely(__result);					})
>>  #endif // CONFIG_X86_32
>>  #endif // CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
> [ ... ]
>> @@ -551,11 +594,18 @@ do {										\
>>  
>>  extern void __try_cmpxchg_user_wrong_size(void);
>>  
>> -#ifndef CONFIG_X86_32
>> +#ifdef CONFIG_X86_32
>> +/* Always fail on 32 bit arch as it do not support 128 cmpxchg (i.e. cmpxchg16b
>> + * instruction).
>> + */
>> +#define __try_cmpxchg128_user_asm(_ptr, _pold, _new, label) ({ BUILD_BUG_ON(1); 0; })
> 
> [Severity: Medium]
> Will this BUILD_BUG_ON(1) break 32-bit x86 builds that call the
> unsafe_try_cmpxchg_user() macro for perfectly valid 32-bit or 64-bit sizes?
> 
> Because unsafe_try_cmpxchg_user() dispatches via a switch(sizeof(*(_ptr)))
> statement, compilers like Clang/LLVM eagerly evaluate all branches during
> semantic analysis, including the 16-byte branch. If evaluated on 32-bit
> builds, BUILD_BUG_ON(1) will trigger a compile-time error regardless of
> whether the 16-byte path is actually reachable.
> 
> Since the commit message says "stub the helper to always return failure",
> would it be better to return a runtime error code here instead of using
> a build bug?
> 

The BUILD_BUG_ON(1) will only cause compile failure if there the code is
getting used. As sean mentioned in the previous version, compiler
elimiates this code if it is not used.

Thanks
Sairaj

>> +#else
>>  #define __try_cmpxchg64_user_asm(_ptr, _oldp, _nval, _label)		\
>>  	__try_cmpxchg_user_asm("q", "r", (_ptr), (_oldp), (_nval), _label)
> 

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

[Sairaj Kodilkar]

On 8/26/2026 1:01 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Emulation of CMPXCHG16B lacks the mandatory 16-byte alignment check, leading to a guest-triggerable VM crash.
> - [Low] The macro `em_cmpxchg8b_16b` incorrectly captures the variable `ctxt` from the surrounding scope instead of solely using its argument `__c`.
> 
> Pre-existing issues:
> - [Critical] TOCTOU vulnerability in atomic CMPXCHG emulation leads to silent data corruption in the guest when a native compare fails.
> --
> 
> commit f6a8a67f38b03c4522f119ac54d63d97691e1289
> Author: Sairaj Kodilkar <sarunkod@amd.com>
> 
> KVM: x86: Add support for cmpxchg16b emulation
> 
> This patch adds support for emulating cmpxchg16b when the destination memory
> is 128-bit and REX.W = 1. This prevents QEMU guests from hitting emulation
> failures during 256-bit device table entries accesses on AMD vIOMMU.
> 
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index c1b21282187f3..f38c4f1a99c31 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
> 
> [ ... ]
> 
>> @@ -2184,24 +2184,36 @@ static int em_call_near_abs(struct x86_emulate_ctxt *ctxt)
>>  	return rc;
>>  }
>>  
>> +#define em_cmpxchg8b_16b(__c, rbits, mbits)\
>> +do {												\
>> +	u##mbits old = __c->dst.orig_val##mbits;						\
>> +												\
>> +	BUILD_BUG_ON(rbits * 2 != mbits);							\
>> +												\
>> +	if (((u##rbits) (old >> 0) != (u##rbits) reg_read(ctxt, VCPU_REGS_RAX)) ||		\
> 
> [Severity: Low]
> Does this macro intentionally reference the unpassed variable ctxt instead of
> the provided argument __c?
> 
> This uses ctxt to read registers, which makes the macro rely on the specific
> variable name ctxt in the caller's scope instead of its parameter.
> 

Its a bug, the macro should use __c instead of ctxt everywhere.

>> +	    ((u##rbits) (old >> rbits) != (u##rbits) reg_read(ctxt, VCPU_REGS_RDX))) {		\
>> +		*reg_write(ctxt, VCPU_REGS_RAX) = (u##rbits) (old >> 0);			\
>> +		*reg_write(ctxt, VCPU_REGS_RDX) = (u##rbits) (old >> rbits);			\
>> +		ctxt->eflags &= ~X86_EFLAGS_ZF;							\
>> +	} else {										\
>> +		ctxt->dst.val##mbits = ((u##mbits)reg_read(ctxt, VCPU_REGS_RCX) << rbits) |	\
>> +					(u##rbits) reg_read(ctxt, VCPU_REGS_RBX);		\
>> +												\
>> +		ctxt->eflags |= X86_EFLAGS_ZF;							\
>> +	}											\
>> +} while(0)
>> +
>>  static int em_cmpxchg8b(struct x86_emulate_ctxt *ctxt)
>>  {
>> -	u64 old = ctxt->dst.orig_val64;
>> -
>> -	if (ctxt->dst.bytes == 16)
>> +	if (WARN_ON_ONCE(8 + !!(ctxt->rex_bits & REX_W) * 8 != ctxt->dst.bytes))
>>  		return X86EMUL_UNHANDLEABLE;
>>  
>> -	if (((u32) (old >> 0) != (u32) reg_read(ctxt, VCPU_REGS_RAX)) ||
>> -	    ((u32) (old >> 32) != (u32) reg_read(ctxt, VCPU_REGS_RDX))) {
>> -		*reg_write(ctxt, VCPU_REGS_RAX) = (u32) (old >> 0);
>> -		*reg_write(ctxt, VCPU_REGS_RDX) = (u32) (old >> 32);
>> -		ctxt->eflags &= ~X86_EFLAGS_ZF;
>> -	} else {
>> -		ctxt->dst.val64 = ((u64)reg_read(ctxt, VCPU_REGS_RCX) << 32) |
>> -			(u32) reg_read(ctxt, VCPU_REGS_RBX);
>> -
>> -		ctxt->eflags |= X86_EFLAGS_ZF;
>> -	}
>> +	if (!(ctxt->rex_bits & REX_W))
>> +		em_cmpxchg8b_16b(ctxt, 32, 64);
>> +#ifdef CONFIG_X86_64
>> +	else
>> +		em_cmpxchg8b_16b(ctxt, 64, 128);
>> +#endif
> 
> [Severity: High]
> Are we missing a 16-byte alignment check for the destination memory when
> executing CMPXCHG16B here?
> 
> The x86 architecture strictly mandates that the memory operand for CMPXCHG16B
> must be 16-byte aligned. If it is not, a #GP(0) exception is generated
> unconditionally.
> 
> Without this check, KVM will later attempt a native unaligned lock cmpxchg16b,
> which will raise a host #GP(0). The host exception table catches this and
> returns -EFAULT, translating to X86EMUL_UNHANDLEABLE, which immediately
> crashes the VM via KVM_EXIT_INTERNAL_ERROR. Can this regression be used by a
> guest to trigger a denial of service?

insn_alignment checks for any alignment issues. Its comment explicitly
mentions that

"Also included is CMPXCHG16B which is not a vector instruction, yet it
is subject to the same check.  FXSAVE and FXRSTOR are checked here too
as their 512 bytes of data must be aligned to a 16 byte boundary."

Hence kvm will raise an #GP(0) to guest before it performs cmpxchg8b.

Thanks
Sairaj

> 
>>  	return X86EMUL_CONTINUE;
>>  }
> 
> [ ... ]
> 
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 79468ddfe4736..60be16f05d794 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
> 
> [ ... ]
> 
>> @@ -5311,6 +5311,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
> 
> [Severity: Critical]
> This is a pre-existing issue, but does the emulation of atomic CMPXCHG suffer
> from a TOCTOU race leading to silent data corruption in the guest?
> 
> When emulator_try_cmpxchg_user() fails due to concurrent memory modification,
> emulator_cmpxchg_emulated() returns X86EMUL_CMPXCHG_FAILED.
> 
> However, in x86_emulate_insn(), this failure appears to be silently swallowed:
> 
> arch/x86/kvm/emulate.c:x86_emulate_insn() {
>     ...
>     if (rc == X86EMUL_CONTINUE)
>         writeback_registers(ctxt);
> 
>     return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
> }
> 
> Because rc is not X86EMUL_CONTINUE, writeback_registers() is skipped and the
> guest registers are not updated. Because EMULATION_OK is returned, KVM retires
> the instruction as successful, advancing RIP and leaving the guest with a
> stale ZF=1 flag. Doesn't this cause the guest to mistakenly believe it
> acquired a lock?
> 

Re: [PATCH v3 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[Dave Hansen]

On 8/26/26 00:00, Sairaj Kodilkar wrote:
> Extend the existing user CMPXCHG helpers to support 16-byte operands on
> x86-64, using LOCK_PREFIX "cmpxchg16b".  This mirrors the existing
> __try_cmpxchg64_user_asm() / cmpxchg8b path provided for 32-bit kernels,
> where KVM needs an atomic compare-exchange wider than the generic
> cmpxchg helper can provide.

Please take a good look at the Sashiko review:

https://sashiko.dev/#/patchset/20260826070004.8100-2-sarunkod%40amd.com

It looks like the "A" constraint isn't one that you can cleanly mirror
from cmpxchg8b => cmpxchg16b.

Uros, any chance you can give these a good once-over? This seems to be
just the kind of thing you've been fixing up lately. It would be nice to
get them right the first time.

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

[Mathieu Desnoyers]

On 2026-08-26 03:00, Sairaj Kodilkar wrote:
> AMD and Intel both provides support for 128 bit cmpxchg operands using
> cmpxchg8b/cmpxchg16b instructions (opcode 0FC7). However, kvm does not
> support emulating cmpxchg16b (i.e when destination memory is 128 bit and
> REX.W = 1) which causes emulation failure when QEMU guest performs a
> cmpxchg16b on a memory region setup as a IO.
> 
> This failure is seen on the AMD IOMMU driver which writes 256-bit device
> table entries with two 128-bit cmpxchg operations. For guests using
> hardware-accelerated vIOMMU, QEMU traps device table accesses to set up
> nested page tables (see [1]).  Without 128-bit cmpxchg emulation, KVM
> cannot handle these traps and DTE access emulation fails.
> 
> Hence extend cmpxchg8b to perform cmpxchg16b when the destination memory
> is 128 bit.
> 
> [1] https://github.com/AMDESE/qemu-iommu/blob/wip/for_iommufd_hw_queue-v8_amd_viommu_20260106/hw/i386/amd_viommu.c#L517
> 

You may want to test for the X86_FEATURE_CX16 cpu feature and only
enable this on 64-bit x86 CPUs implementing this instruction.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: [PATCH v3 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[Uros Bizjak]

On Wed, Aug 26, 2026 at 2:30 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 8/26/26 00:00, Sairaj Kodilkar wrote:
> > Extend the existing user CMPXCHG helpers to support 16-byte operands on
> > x86-64, using LOCK_PREFIX "cmpxchg16b".  This mirrors the existing
> > __try_cmpxchg64_user_asm() / cmpxchg8b path provided for 32-bit kernels,
> > where KVM needs an atomic compare-exchange wider than the generic
> > cmpxchg helper can provide.
>
> Please take a good look at the Sashiko review:
>
> https://sashiko.dev/#/patchset/20260826070004.8100-2-sarunkod%40amd.com
>
> It looks like the "A" constraint isn't one that you can cleanly mirror
> from cmpxchg8b => cmpxchg16b.

Actually, "+A" will work for 64bit targets, as long as the variable is
128-bit. The comment in asm.h applies to 64-bit values, where on
32-bit targets they fit in eax *and* edx, while on 64-bit targets, the
64-bit values fit into rax *or* rdx.

This is documented in GCC documentation:

     ‘A’
          The ‘a’ and ‘d’ registers.  This class is used for
          instructions that return double word results in the ‘ax:dx’
          register pair.  Single word values will be allocated either in
          ‘ax’ or ‘dx’.

And can be confirmed with e.g.:

__int128 val;

void foo (void)
{
  __int128 _v = val;
  asm volatile ("" : "+A" (_v));
  val = _v;
}

which will fail compilation with -ffixed-rax or -ffixed-rdx.

That said, the approach with union of two 64-bit halves can lead to
slightly better code, because the compiler splits the value earlier in
the compilation pipeline.

> Uros, any chance you can give these a good once-over? This seems to be
> just the kind of thing you've been fixing up lately. It would be nice to
> get them right the first time.

Based on the above explanation, these *can* be copied from 32-bit asm
patterns. Even "q" constraint will include all integer registers on
64-bit targets.

Uros.

Re: [PATCH v3 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[Sairaj Kodilkar]

On 8/26/2026 6:49 PM, Uros Bizjak wrote:
> [You don't often get email from ubizjak@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Wed, Aug 26, 2026 at 2:30 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>
>> On 8/26/26 00:00, Sairaj Kodilkar wrote:
>>> Extend the existing user CMPXCHG helpers to support 16-byte operands on
>>> x86-64, using LOCK_PREFIX "cmpxchg16b".  This mirrors the existing
>>> __try_cmpxchg64_user_asm() / cmpxchg8b path provided for 32-bit kernels,
>>> where KVM needs an atomic compare-exchange wider than the generic
>>> cmpxchg helper can provide.
>>
>> Please take a good look at the Sashiko review:
>>
>> https://sashiko.dev/#/patchset/20260826070004.8100-2-sarunkod%40amd.com
>>
>> It looks like the "A" constraint isn't one that you can cleanly mirror
>> from cmpxchg8b => cmpxchg16b.
> 
> Actually, "+A" will work for 64bit targets, as long as the variable is
> 128-bit. The comment in asm.h applies to 64-bit values, where on
> 32-bit targets they fit in eax *and* edx, while on 64-bit targets, the
> 64-bit values fit into rax *or* rdx.
> 
> This is documented in GCC documentation:
> 
>      ‘A’
>           The ‘a’ and ‘d’ registers.  This class is used for
>           instructions that return double word results in the ‘ax:dx’
>           register pair.  Single word values will be allocated either in
>           ‘ax’ or ‘dx’.
> 
> And can be confirmed with e.g.:
> 
> __int128 val;
> 
> void foo (void)
> {
>   __int128 _v = val;
>   asm volatile ("" : "+A" (_v));
>   val = _v;
> }
> 
> which will fail compilation with -ffixed-rax or -ffixed-rdx.
> 
> That said, the approach with union of two 64-bit halves can lead to
> slightly better code, because the compiler splits the value earlier in
> the compilation pipeline.
> 
>> Uros, any chance you can give these a good once-over? This seems to be
>> just the kind of thing you've been fixing up lately. It would be nice to
>> get them right the first time.
> 
> Based on the above explanation, these *can* be copied from 32-bit asm
> patterns. Even "q" constraint will include all integer registers on
> 64-bit targets.

Yep, this is what I was trying to say in my reply to sashiko. Basically
+A can be safely used for 64 bit values on 32 bit machines and 128 bit
values on 64 bit machines.

Also if you prefer split one, I can do that as well.

Sairaj

> 
> Uros.

Re: [PATCH v3 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[David Laight]

On Wed, 26 Aug 2026 15:19:57 +0200
Uros Bizjak <ubizjak@gmail.com> wrote:

> On Wed, Aug 26, 2026 at 2:30 PM Dave Hansen <dave.hansen@intel.com> wrote:
> >
> > On 8/26/26 00:00, Sairaj Kodilkar wrote:  
> > > Extend the existing user CMPXCHG helpers to support 16-byte operands on
> > > x86-64, using LOCK_PREFIX "cmpxchg16b".  This mirrors the existing
> > > __try_cmpxchg64_user_asm() / cmpxchg8b path provided for 32-bit kernels,
> > > where KVM needs an atomic compare-exchange wider than the generic
> > > cmpxchg helper can provide.  
> >
> > Please take a good look at the Sashiko review:
> >
> > https://sashiko.dev/#/patchset/20260826070004.8100-2-sarunkod%40amd.com
> >
> > It looks like the "A" constraint isn't one that you can cleanly mirror
> > from cmpxchg8b => cmpxchg16b.  
> 
> Actually, "+A" will work for 64bit targets, as long as the variable is
> 128-bit. The comment in asm.h applies to 64-bit values, where on
> 32-bit targets they fit in eax *and* edx, while on 64-bit targets, the
> 64-bit values fit into rax *or* rdx.
> 
...
> 
> That said, the approach with union of two 64-bit halves can lead to
> slightly better code, because the compiler splits the value earlier in
> the compilation pipeline.

I think I agree...

From experiments I did with 64bit values on 32bit it is more the case
that the value never gets assigned to a 64bit (on 32bit) 'virtual'
register. If that ever happens all the operations are initially done
with the 'wide' register and then later split (rather than being
generated as a pair of 32bit ops).
This causes excessive register pressure and even spilling of constant
zero values to stack.
You do seem to 'get away' with returning hi << 32 | lo.

I'd guess the same happens for 128bit values on 64bit.
(This is gcc, clang does a lot better.)

David

> 
> > Uros, any chance you can give these a good once-over? This seems to be
> > just the kind of thing you've been fixing up lately. It would be nice to
> > get them right the first time.  
> 
> Based on the above explanation, these *can* be copied from 32-bit asm
> patterns. Even "q" constraint will include all integer registers on
> 64-bit targets.
> 
> Uros.
>