[Xen-changelog] [xen staging] x86/VT-x: Don't activate VMCS Shadowing outside of nested vmx mode

[patchbot@xxxxxxx]

commit 75ce36eb72cb93e8a3c9f60fd5e697067921d712
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Fri Dec 7 17:00:47 2018 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Mon Dec 10 16:24:08 2018 +0000

    x86/VT-x: Don't activate VMCS Shadowing outside of nested vmx mode
    
    By default on capable hardware, SECONDARY_EXEC_ENABLE_VMCS_SHADOWING is
    activated unilaterally.  The VMCS Link pointer is initialised to ~0, but the
    VMREAD/VMWRITE bitmap pointers are not.
    
    This causes the 16bit IVT and Bios Data Area get interpreted as the 
read/write
    permission bitmap for guests which blindly execute VMREAD/VMWRITE
    instructions.
    
    This is not a security issue because the VMCS Link pointer being ~0 causes
    VMREAD/VMWRITE to complete with VMFailInvalid (rather than modifying a
    potential shadow VMCS), and the contents of MFN 0 has already been 
determined
    not to contain any interesting data because of L1TF's ability to read that 
4k
    frame.
    
    Leave VMCS Shadowing disabled by default, and toggle it in
    nvmx_{set,clear}_vmcs_pointer().  This isn't the most efficient course of
    action, but it is the most simple way of leaving nested-virt working as it 
did
    before.
    
    While editing construct_vmcs(), collect all default secondary_exec_control
    modifications together.  The disabling of PML is latently buggy because it
    happens after secondary_exec_control are written into the VMCS, although 
there
    is an unconditional update later which writes the correct value into 
hardware.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/hvm/vmx/vmcs.c | 32 ++++++++++++++------------------
 xen/arch/x86/hvm/vmx/vvmx.c |  8 ++++++++
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index dec21d1fa4..d6366c202a 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1012,14 +1012,22 @@ static int construct_vmcs(struct vcpu *v)
     v->arch.hvm.vmx.secondary_exec_control = vmx_secondary_exec_control;
 
     /*
-     * Disable descriptor table exiting: It's controlled by the VM event
-     * monitor requesting it.
+     * Disable features which we don't want active by default:
+     *  - Descriptor table exiting only if wanted by introspection
+     *  - x2APIC - default is xAPIC mode
+     *  - VPID settings chosen at VMEntry time
+     *  - VMCS Shadowing only when in nested VMX mode
+     *  - PML only when logdirty is active
+     *  - VMFUNC/#VE only if wanted by altp2m
      */
     v->arch.hvm.vmx.secondary_exec_control &=
-        ~SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING;
-
-    /* Disable VPID for now: we decide when to enable it on VMENTER. */
-    v->arch.hvm.vmx.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
+        ~(SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING |
+          SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
+          SECONDARY_EXEC_ENABLE_VPID |
+          SECONDARY_EXEC_ENABLE_VMCS_SHADOWING |
+          SECONDARY_EXEC_ENABLE_PML |
+          SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
+          SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
 
     if ( paging_mode_hap(d) )
     {
@@ -1038,18 +1046,9 @@ static int construct_vmcs(struct vcpu *v)
         vmentry_ctl &= ~VM_ENTRY_LOAD_GUEST_PAT;
     }
 
-    /* Disable Virtualize x2APIC mode by default. */
-    v->arch.hvm.vmx.secondary_exec_control &=
-        ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
-
     /* Do not enable Monitor Trap Flag unless start single step debug */
     v->arch.hvm.vmx.exec_control &= ~CPU_BASED_MONITOR_TRAP_FLAG;
 
-    /* Disable VMFUNC and #VE for now: they may be enabled later by altp2m. */
-    v->arch.hvm.vmx.secondary_exec_control &=
-        ~(SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
-          SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
-
     if ( !has_vlapic(d) )
     {
         /* Disable virtual apics, TPR */
@@ -1133,9 +1132,6 @@ static int construct_vmcs(struct vcpu *v)
         __vmwrite(POSTED_INTR_NOTIFICATION_VECTOR, posted_intr_vector);
     }
 
-    /* Disable PML anyway here as it will only be enabled in log dirty mode */
-    v->arch.hvm.vmx.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
-
     /* Host data selectors. */
     __vmwrite(HOST_SS_SELECTOR, __HYPERVISOR_DS);
     __vmwrite(HOST_DS_SELECTOR, __HYPERVISOR_DS);
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index a72b519ce0..9f6ea5c1f7 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1098,6 +1098,10 @@ static void nvmx_set_vmcs_pointer(struct vcpu *v, struct 
vmcs_struct *vvmcs)
 
     __vmpclear(vvmcs_maddr);
     vvmcs->vmcs_revision_id |= VMCS_RID_TYPE_MASK;
+    v->arch.hvm.vmx.secondary_exec_control |=
+        SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
+    __vmwrite(SECONDARY_VM_EXEC_CONTROL,
+              v->arch.hvm.vmx.secondary_exec_control);
     __vmwrite(VMCS_LINK_POINTER, vvmcs_maddr);
     __vmwrite(VMREAD_BITMAP, page_to_maddr(v->arch.hvm.vmx.vmread_bitmap));
     __vmwrite(VMWRITE_BITMAP, page_to_maddr(v->arch.hvm.vmx.vmwrite_bitmap));
@@ -1109,6 +1113,10 @@ static void nvmx_clear_vmcs_pointer(struct vcpu *v, 
struct vmcs_struct *vvmcs)
 
     __vmpclear(vvmcs_maddr);
     vvmcs->vmcs_revision_id &= ~VMCS_RID_TYPE_MASK;
+    v->arch.hvm.vmx.secondary_exec_control &=
+        ~SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
+    __vmwrite(SECONDARY_VM_EXEC_CONTROL,
+              v->arch.hvm.vmx.secondary_exec_control);
     __vmwrite(VMCS_LINK_POINTER, ~0ul);
     __vmwrite(VMREAD_BITMAP, 0);
     __vmwrite(VMWRITE_BITMAP, 0);
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog