During VM forking and resetting a failed vmentry has been observed due
to the guest non-register state going out-of-sync with the guest register
state. For example, a VM fork reset right after a STI instruction can trigger
the failed entry. This is due to the guest non-register state not being saved
from the parent VM, thus the reset operation only copies the register state.

Fix this by including the guest non-register state in hvm_hw_cpu so that when
its copied from the parent VM the vCPU state remains in sync.

SVM is not currently wired-in as VM forking is VMX only and saving non-register
state during normal save/restore/migration operation hasn't been needed. If
deemed necessary in the future it can be wired in by adding a svm-substructure
to hvm_hw_cpu.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
---
v3: Add XEN_X86_VMX flag and vmx-substructure in hvm_hw_cpu
v2: Include all CPU non-register state and fold the ops into vmx_vmcs_save &
    vmx_vmcs_restore.
Note: no sanity checking is performed on the fields to reduce the cycles during
      fuzzing.
---
 xen/arch/x86/hvm/vmx/vmx.c             | 17 ++++++++++++++++-
 xen/include/public/arch-x86/hvm/save.h | 13 +++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c075370f64..6da3842d6e 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -713,7 +713,7 @@ static void vmx_restore_dr(struct vcpu *v)
 
 static void vmx_vmcs_save(struct vcpu *v, struct hvm_hw_cpu *c)
 {
-    unsigned long ev;
+    unsigned long ev, activity_state, intr_info;
 
     vmx_vmcs_enter(v);
 
@@ -721,6 +721,10 @@ static void vmx_vmcs_save(struct vcpu *v, struct hvm_hw_cpu *c)
     __vmread(GUEST_SYSENTER_ESP, &c->sysenter_esp);
     __vmread(GUEST_SYSENTER_EIP, &c->sysenter_eip);
 
+    __vmread(GUEST_ACTIVITY_STATE, &activity_state);
+    __vmread(GUEST_INTERRUPTIBILITY_INFO, &intr_info);
+    __vmread(GUEST_PENDING_DBG_EXCEPTIONS, &c->vmx.pending_dbg);
+
     __vmread(VM_ENTRY_INTR_INFO, &ev);
     if ( (ev & INTR_INFO_VALID_MASK) &&
          hvm_event_needs_reinjection(MASK_EXTR(ev, INTR_INFO_INTR_TYPE_MASK),
@@ -732,6 +736,10 @@ static void vmx_vmcs_save(struct vcpu *v, struct hvm_hw_cpu *c)
     }
 
     vmx_vmcs_exit(v);
+
+    c->vmx.activity_state = activity_state;
+    c->vmx.interruptibility_info = intr_info;
+    c->flags |= XEN_X86_VMX;
 }
 
 static int vmx_restore_cr0_cr3(
@@ -807,6 +815,13 @@ static int vmx_vmcs_restore(struct vcpu *v, struct hvm_hw_cpu *c)
 
     __vmwrite(GUEST_DR7, c->dr7);
 
+    if ( c->flags & XEN_X86_VMX )
+    {
+        __vmwrite(GUEST_ACTIVITY_STATE, c->vmx.activity_state);
+        __vmwrite(GUEST_INTERRUPTIBILITY_INFO, c->vmx.interruptibility_info);
+        __vmwrite(GUEST_PENDING_DBG_EXCEPTIONS, c->vmx.pending_dbg);
+    }
+
     if ( c->pending_valid &&
          hvm_event_needs_reinjection(c->pending_type, c->pending_vector) )
     {
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 773a380bc2..0f728aa5d9 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -52,6 +52,7 @@ DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
  * Compat:
  *     - Pre-3.4 didn't have msr_tsc_aux
  *     - Pre-4.7 didn't have fpu_initialised
+ *     - Pre-4.17 didn't have non-register state
  */
 
 struct hvm_hw_cpu {
@@ -163,9 +164,21 @@ struct hvm_hw_cpu {
     uint32_t error_code;
 
 #define _XEN_X86_FPU_INITIALISED        0
+#define _XEN_X86_VMX                    1
 #define XEN_X86_FPU_INITIALISED         (1U<<_XEN_X86_FPU_INITIALISED)
+#define XEN_X86_VMX                     (1U<<_XEN_X86_VMX)
     uint32_t flags;
     uint32_t pad0;
+
+    /* non-register state */
+    union {
+        /* if flags & XEN_X86_VMX */
+        struct {
+            uint32_t activity_state;
+            uint32_t interruptibility_info;
+            uint64_t pending_dbg;
+        } vmx;
+    };
 };
 
 struct hvm_hw_cpu_compat {
-- 
2.25.1

On 21.03.2022 17:26, Tamas K Lengyel wrote:
> During VM forking and resetting a failed vmentry has been observed due
> to the guest non-register state going out-of-sync with the guest register
> state. For example, a VM fork reset right after a STI instruction can trigger
> the failed entry. This is due to the guest non-register state not being saved
> from the parent VM, thus the reset operation only copies the register state.
> 
> Fix this by including the guest non-register state in hvm_hw_cpu so that when
> its copied from the parent VM the vCPU state remains in sync.
> 
> SVM is not currently wired-in as VM forking is VMX only and saving non-register
> state during normal save/restore/migration operation hasn't been needed. If
> deemed necessary in the future it can be wired in by adding a svm-substructure
> to hvm_hw_cpu.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

On Mon, Mar 21, 2022, 12:41 PM Jan Beulich <jbeulich@suse.com> wrote:

> On 21.03.2022 17:26, Tamas K Lengyel wrote:
> > During VM forking and resetting a failed vmentry has been observed due
> > to the guest non-register state going out-of-sync with the guest register
> > state. For example, a VM fork reset right after a STI instruction can
> trigger
> > the failed entry. This is due to the guest non-register state not being
> saved
> > from the parent VM, thus the reset operation only copies the register
> state.
> >
> > Fix this by including the guest non-register state in hvm_hw_cpu so that
> when
> > its copied from the parent VM the vCPU state remains in sync.
> >
> > SVM is not currently wired-in as VM forking is VMX only and saving
> non-register
> > state during normal save/restore/migration operation hasn't been needed.
> If
> > deemed necessary in the future it can be wired in by adding a
> svm-substructure
> > to hvm_hw_cpu.
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>

Thanks, will send v4 shortly, will need a couple fixes still.

Tamas

>