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

[Sean Christopherson <seanjc@google.com>]

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));