[PATCH] KVM: x86: Fix incorrect memory constraint for FXSAVE in emulator

[PATCH] KVM: x86: Fix incorrect memory constraint for FXSAVE in emulator

[Uros Bizjak]

The inline asm used to invoke FXSAVE in em_fxsave() and fxregs_fixup()
incorrectly specifies the memory operand as read-write ("+m"). FXSAVE
does not read from the destination operand; it only writes the current
FPU state to memory.

Using a read-write constraint is incorrect and misleading, as it tells
the compiler that the previous contents of the buffer are consumed by
the instruction. In both cases, the buffer passed to FXSAVE is
uninitialized, and marking it as read-write can therefore create a
false dependency on uninitialized memory.

Fix the constraint to write-only ("=m") to accurately describe the
instruction’s behavior and avoid implying that the buffer is read.

No functional change intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/kvm/emulate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c8e292e9a24d..d60094080e3f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3717,7 +3717,7 @@ static int em_fxsave(struct x86_emulate_ctxt *ctxt)
 
 	kvm_fpu_get();
 
-	rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state));
+	rc = asm_safe("fxsave %[fx]", , [fx] "=m"(fx_state));
 
 	kvm_fpu_put();
 
@@ -3741,7 +3741,7 @@ static noinline int fxregs_fixup(struct fxregs_state *fx_state,
 	struct fxregs_state fx_tmp;
 	int rc;
 
-	rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_tmp));
+	rc = asm_safe("fxsave %[fx]", , [fx] "=m"(fx_tmp));
 	memcpy((void *)fx_state + used_size, (void *)&fx_tmp + used_size,
 	       __fxstate_size(16) - used_size);
 
-- 
2.53.0

Re: [PATCH] KVM: x86: Fix incorrect memory constraint for FXSAVE in emulator

[Paolo Bonzini]

On 2/12/26 11:27, Uros Bizjak wrote:
> The inline asm used to invoke FXSAVE in em_fxsave() and fxregs_fixup()
> incorrectly specifies the memory operand as read-write ("+m"). FXSAVE
> does not read from the destination operand; it only writes the current
> FPU state to memory.
> 
> Using a read-write constraint is incorrect and misleading, as it tells
> the compiler that the previous contents of the buffer are consumed by
> the instruction. In both cases, the buffer passed to FXSAVE is
> uninitialized, and marking it as read-write can therefore create a
> false dependency on uninitialized memory.
> 
> Fix the constraint to write-only ("=m") to accurately describe the
> instruction’s behavior and avoid implying that the buffer is read.

IIRC FXSAVE/FXRSTOR may (at least on some microarchitectures?) leave 
reserved fields untouched.

Intel suggests writing zeros first, and then the "+m" constraint would 
be the right one because "=m" would cause the memset to be dead.

Paolo

> No functional change intended.
> 
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Gleixner <tglx@kernel.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> ---
>   arch/x86/kvm/emulate.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index c8e292e9a24d..d60094080e3f 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3717,7 +3717,7 @@ static int em_fxsave(struct x86_emulate_ctxt *ctxt)
>   
>   	kvm_fpu_get();
>   
> -	rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state));
> +	rc = asm_safe("fxsave %[fx]", , [fx] "=m"(fx_state));
>   
>   	kvm_fpu_put();
>   
> @@ -3741,7 +3741,7 @@ static noinline int fxregs_fixup(struct fxregs_state *fx_state,
>   	struct fxregs_state fx_tmp;
>   	int rc;
>   
> -	rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_tmp));
> +	rc = asm_safe("fxsave %[fx]", , [fx] "=m"(fx_tmp));
>   	memcpy((void *)fx_state + used_size, (void *)&fx_tmp + used_size,
>   	       __fxstate_size(16) - used_size);
>   

Re: [PATCH] KVM: x86: Fix incorrect memory constraint for FXSAVE in emulator

[Uros Bizjak]

On Thu, Feb 12, 2026 at 2:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 2/12/26 11:27, Uros Bizjak wrote:
> > The inline asm used to invoke FXSAVE in em_fxsave() and fxregs_fixup()
> > incorrectly specifies the memory operand as read-write ("+m"). FXSAVE
> > does not read from the destination operand; it only writes the current
> > FPU state to memory.
> >
> > Using a read-write constraint is incorrect and misleading, as it tells
> > the compiler that the previous contents of the buffer are consumed by
> > the instruction. In both cases, the buffer passed to FXSAVE is
> > uninitialized, and marking it as read-write can therefore create a
> > false dependency on uninitialized memory.
> >
> > Fix the constraint to write-only ("=m") to accurately describe the
> > instruction’s behavior and avoid implying that the buffer is read.
>
> IIRC FXSAVE/FXRSTOR may (at least on some microarchitectures?) leave
> reserved fields untouched.
>
> Intel suggests writing zeros first, and then the "+m" constraint would
> be the right one because "=m" would cause the memset to be dead.

Please note that the struct is not initialized before fxsave, so if
"+m" is required, the struct should be initialized.

Uros.

Re: [PATCH] KVM: x86: Fix incorrect memory constraint for FXSAVE in emulator

[Sean Christopherson]

On Thu, Feb 12, 2026, Uros Bizjak wrote:
> On Thu, Feb 12, 2026 at 2:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 2/12/26 11:27, Uros Bizjak wrote:
> > > The inline asm used to invoke FXSAVE in em_fxsave() and fxregs_fixup()
> > > incorrectly specifies the memory operand as read-write ("+m"). FXSAVE
> > > does not read from the destination operand; it only writes the current
> > > FPU state to memory.
> > >
> > > Using a read-write constraint is incorrect and misleading, as it tells
> > > the compiler that the previous contents of the buffer are consumed by
> > > the instruction. In both cases, the buffer passed to FXSAVE is
> > > uninitialized, and marking it as read-write can therefore create a
> > > false dependency on uninitialized memory.
> > >
> > > Fix the constraint to write-only ("=m") to accurately describe the
> > > instruction’s behavior and avoid implying that the buffer is read.
> >
> > IIRC FXSAVE/FXRSTOR may (at least on some microarchitectures?) leave
> > reserved fields untouched.
> >
> > Intel suggests writing zeros first, and then the "+m" constraint would
> > be the right one because "=m" would cause the memset to be dead.
> 
> Please note that the struct is not initialized before fxsave, so if
> "+m" is required, the struct should be initialized.

Regardless of CPU behavior with respect to reserved fields, I believe "+m" is
correct and "=m" is wrong, strictly speaking.  The SDM very explicitly says:

  Bytes 464:511 are available to software use. The processor does not write to
  bytes 464:511 of an FXSAVE area.

I.e. the entirety of the struct isn't written by FXSAVE, and so using "=m" is
technically wrong because those bytes are "read".  In practice, it shouldn't
matter because fxstate_size() (correctly) truncates the size to a max of 464
bytes, so that KVM-as-the-virutal-CPU honors the architecture and doesn't write
to the software-available fields.  I.e. those bytes should never truly be read
by software.

Given that emulating FXSAVE/FXRSTOR can't possibly be hot paths, explicitly
initializing the on-stack structs seems prudent, e.g.

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c8e292e9a24d..20ed588015f1 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3708,7 +3708,7 @@ static inline size_t fxstate_size(struct x86_emulate_ctxt *ctxt)
  */
 static int em_fxsave(struct x86_emulate_ctxt *ctxt)
 {
-       struct fxregs_state fx_state;
+       struct fxregs_state fx_state = {};
        int rc;
 
        rc = check_fxsr(ctxt);
@@ -3738,7 +3738,7 @@ static int em_fxsave(struct x86_emulate_ctxt *ctxt)
 static noinline int fxregs_fixup(struct fxregs_state *fx_state,
                                 const size_t used_size)
 {
-       struct fxregs_state fx_tmp;
+       struct fxregs_state fx_tmp = {};
        int rc;
 
        rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_tmp));