[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen staging-4.22] x86/svm: Work around VMRUN's corruption of VMCB.INTR_SHADOW



commit 1ad773747dd1f48009b79e21e3381664c5c3ce0b
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Thu Oct 29 13:17:47 2026 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Oct 29 13:17:47 2026 +0100

    x86/svm: Work around VMRUN's corruption of VMCB.INTR_SHADOW
    
    I was asked to explain the comment beside STI in the svm_asm_do_resume() 
and,
    in trying to do so, things unravelled.
    
    The original fix inherited from KVM prevented the guests INTR_SHADOW being
    changed from 0 to 1.  This avoid undue delay to interrupts, but it caused 
the
    value to get changed from 1 to 0 if the guest was in an INTR_SHADOW.
    
    Whether this matters for IDT guests is unknown; it depends on what else 
VMRUN
    can do beside event injection and still write back the wrong value.  This 
does
    matter for FRED guests, as INTR_SHADOW is exposed architecturally in the 
FRED
    frame via the STB field.
    
    Fixes: c989ff614f6b ("x86/svm: Separate STI and VMRUN instructions in 
svm_asm_do_resume()")
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
    master commit: df8d101a3854725fd3db3c52f074808e6b3cc539
    master date: 2026-10-26 18:36:52 +0000
---
 xen/arch/x86/hvm/svm/entry.S      | 40 +++++++++++++++++++++++++++++++--------
 xen/arch/x86/x86_64/asm-offsets.c |  9 +++++++++
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index af8db23b03..df24eb03b0 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -57,6 +57,37 @@ __UNLIKELY_END(nsvm_hap)
 
         clgi
 
+        /*
+         * Interrupts need enabling prior to VMRUN.  Anywhere after CLGI is
+         * safe.
+         *
+         * On all Zen CPUs to date, VMRUN has a bug and mixes up the host and
+         * guest INTR_SHADOW state.  If there is a VMExit during VMRUN
+         * (e.g. taking NPT_FAULT during event injection), then the host's
+         * INTR_SHADOW gets written back into the VMCB, corrupting guest
+         * state.
+         *
+         * This is believed to go unnoticed for pre-FRED guests (injecting
+         * events cancels INTR_SHADOW), but FRED changes this behaviour to be
+         * able to observe and restore the shadow via the STB field in a FRED
+         * frame.
+         *
+         * In order to fix, arrange for Xen's INTR_SHADOW at the point of
+         * VMRUN to match the guest's INTR_SHADOW.  This way, if a VMExit
+         * occurs, the VMCB is corrupted with the "correct" value.
+         *
+         * Commonly the guest will not be in an INTR_SHADOW.  In this case,
+         * execute STI early to get Xen's INTR_SHADOW over and done with; the
+         * second STI does nothing as INTR_SHADOW only occurs when IF changes
+         * from 0 to 1.  If the guest is in an INTR_SHADOW, skip the early STI
+         * so the late STI does trigger INTR_SHADOW and cover VMRUN.
+         */
+        mov     VCPU_svm_vmcb(%rbx), %rax
+        testb   $1, VMCB_int_stat(%rax)
+        jnz     1f
+        sti
+1:
+
         /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
         /* SPEC_CTRL_EXIT_TO_SVM       Req: b=curr %rsp=regs/cpuinfo, Clob: 
acd */
         .macro svm_vmentry_spec_ctrl
@@ -74,14 +105,6 @@ __UNLIKELY_END(nsvm_hap)
         ALTERNATIVE "", svm_vmentry_spec_ctrl, X86_FEATURE_SC_MSR_HVM
         ALTERNATIVE "", DO_SPEC_CTRL_DIV, X86_FEATURE_SC_DIV
 
-        /*
-         * Set EFLAGS.IF after CLGI covers us from real interrupts, but not
-         * immediately prior to VMRUN.  The VMRUN instruction leaks it's
-         * INTR_SHADOW into guest state if a VMExit occurs before VMRUN
-         * completes (e.g. taking #NPF during event injecting.)
-         */
-        sti
-
         pop  %r15
         pop  %r14
         pop  %r13
@@ -101,6 +124,7 @@ __UNLIKELY_END(nsvm_hap)
 
         SPEC_CTRL_COND_VERW     /* Req: %rsp=eframe                    Clob: 
efl */
 
+        sti
         vmrun
 
         SAVE_ALL
diff --git a/xen/arch/x86/x86_64/asm-offsets.c 
b/xen/arch/x86/x86_64/asm-offsets.c
index 2872904ae2..40640d606a 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -21,6 +21,10 @@
 # include "../boot/video.h"
 #endif
 
+#ifdef CONFIG_AMD_SVM
+# include "../hvm/svm/vmcb.h"
+#endif
+
 #define DEFINE(_sym, _val)                                                 \
     asm volatile ( "\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\""\
                    :: "i" (_val) )
@@ -133,6 +137,11 @@ void __dummy__(void)
     BLANK();
 #endif
 
+#ifdef CONFIG_AMD_SVM
+    OFFSET(VMCB_int_stat, struct vmcb_struct, int_stat);
+    BLANK();
+#endif
+
 #ifdef CONFIG_PV32
     OFFSET(DOMAIN_is_32bit_pv, struct domain, arch.pv.is_32bit);
     BLANK();
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.22



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.