x86: Bring host CR4 into well-defined state during setup

Analogously to CR0: Avoid any uncertainty about the state of CR4 left
behind by Linux: check for unexpectedly set reserved bits or required-1
bits, and otherwise set our own state.

A side effect of this change is the VMX's vcpu_exit will no longer clear
VMXE in CR4 but only in the cached Linux state that arch_cpu_restore
will write back.

CC: Valentine Sinitsyn <valentine.sinitsyn@gmail.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

diff --git a/hypervisor/arch/x86/include/asm/percpu.h b/hypervisor/arch/x86/include/asm/percpu.h
index 183bff5605d6f81667c72e53f10e3e1700b5be5b..dff68ccf490e394d4bb48d3da722977d91387309 100644 (file)
--- a/hypervisor/arch/x86/include/asm/percpu.h
+++ b/hypervisor/arch/x86/include/asm/percpu.h
@@ -65,6 +65,7 @@ struct per_cpu {
        unsigned long linux_ip;
        unsigned long linux_cr0;
        unsigned long linux_cr3;
+       unsigned long linux_cr4;
        struct segment linux_cs;
        struct segment linux_ds;
        struct segment linux_es;

diff --git a/hypervisor/arch/x86/include/asm/vcpu.h b/hypervisor/arch/x86/include/asm/vcpu.h
index 6d95e1749bedd50b93f66070b51801d0a823cc44..8135248cecf5607935915bdf433d8d104c7e0a3d 100644 (file)
--- a/hypervisor/arch/x86/include/asm/vcpu.h
+++ b/hypervisor/arch/x86/include/asm/vcpu.h
@@ -24,6 +24,7 @@
 #define X86_CR0_HOST_STATE \
        (X86_CR0_PG | X86_CR0_WP | X86_CR0_NE | X86_CR0_ET | X86_CR0_TS | \
         X86_CR0_MP | X86_CR0_PE)
+#define X86_CR4_HOST_STATE     X86_CR4_PAE
 
 struct vcpu_io_bitmap {
        u8 *data;

diff --git a/hypervisor/arch/x86/setup.c b/hypervisor/arch/x86/setup.c
index 89e2ebf58a5153cefcdd90489b9e8bd86a72bb5c..d1b59856b7a90f5c08b10adafc77aa2dcabc5580 100644 (file)
--- a/hypervisor/arch/x86/setup.c
+++ b/hypervisor/arch/x86/setup.c
@@ -187,6 +187,7 @@ int arch_cpu_init(struct per_cpu *cpu_data)
        asm volatile("ltr %%ax" : : "a" (GDT_DESC_TSS * 8));
 
        cpu_data->linux_cr0 = read_cr0();
+       cpu_data->linux_cr4 = read_cr4();
 
        /* swap CR3 */
        cpu_data->linux_cr3 = read_cr3();
@@ -255,6 +256,7 @@ void arch_cpu_restore(struct per_cpu *cpu_data, int return_code)
        write_msr(MSR_EFER, cpu_data->linux_efer);
        write_cr0(cpu_data->linux_cr0);
        write_cr3(cpu_data->linux_cr3);
+       write_cr4(cpu_data->linux_cr4);
 
        asm volatile("lgdtq %0" : : "m" (cpu_data->linux_gdtr));
        asm volatile("lidtq %0" : : "m" (cpu_data->linux_idtr));

diff --git a/hypervisor/arch/x86/svm.c b/hypervisor/arch/x86/svm.c
index 299295c607ee5eef2b8bcaf2d672dd3631e02036..671bf56b5883adc5a6458c097f95292ff8f24008 100644 (file)
--- a/hypervisor/arch/x86/svm.c
+++ b/hypervisor/arch/x86/svm.c
@@ -177,7 +177,7 @@ static int vmcb_setup(struct per_cpu *cpu_data)
 
        vmcb->cr0 = cpu_data->linux_cr0 & SVM_CR0_ALLOWED_BITS;
        vmcb->cr3 = cpu_data->linux_cr3;
-       vmcb->cr4 = read_cr4();
+       vmcb->cr4 = cpu_data->linux_cr4;
 
        set_svm_segment_from_segment(&vmcb->cs, &cpu_data->linux_cs);
        set_svm_segment_from_segment(&vmcb->ds, &cpu_data->linux_ds);
@@ -406,12 +406,14 @@ int vcpu_init(struct per_cpu *cpu_data)
         * But we want to avoid surprises with new features unknown to us but
         * set by Linux. So check if any assumed revered bit was set and bail
         * out if so.
+        * Note that the APM defines all reserved CR4 bits as must-be-zero.
         */
        if (cpu_data->linux_cr0 & X86_CR0_RESERVED)
                return -EIO;
 
-       /* bring CR0 into well-defined state */
+       /* bring CR0 and CR4 into well-defined states */
        write_cr0(X86_CR0_HOST_STATE);
+       write_cr4(X86_CR4_HOST_STATE);
 
        write_msr(MSR_VM_HSAVE_PA, paging_hvirt2phys(cpu_data->host_state));
 

diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c
index e2594fb9ae02cc085df2886eae14151ee53a6527..971163ea7ecc22354bc4bccc2f8bf4546104a690 100644 (file)
--- a/hypervisor/arch/x86/vmx.c
+++ b/hypervisor/arch/x86/vmx.c
@@ -472,7 +472,7 @@ static bool vmcs_setup(struct per_cpu *cpu_data)
        ok &= vmcs_write64(HOST_RIP, (unsigned long)vmx_vmexit);
 
        ok &= vmx_set_guest_cr(CR0_IDX, cpu_data->linux_cr0);
-       ok &= vmx_set_guest_cr(CR4_IDX, read_cr4());
+       ok &= vmx_set_guest_cr(CR4_IDX, cpu_data->linux_cr4);
 
        ok &= vmcs_write64(GUEST_CR3, cpu_data->linux_cr3);
 
@@ -565,12 +565,11 @@ static bool vmcs_setup(struct per_cpu *cpu_data)
 
 int vcpu_init(struct per_cpu *cpu_data)
 {
-       unsigned long cr4, feature_ctrl, mask;
+       unsigned long feature_ctrl, mask;
        u32 revision_id;
        int err;
 
-       cr4 = read_cr4();
-       if (cr4 & X86_CR4_VMXE)
+       if (cpu_data->linux_cr4 & X86_CR4_VMXE)
                return -EBUSY;
 
        err = vmx_check_features();
@@ -603,19 +602,21 @@ int vcpu_init(struct per_cpu *cpu_data)
         * set by Linux. So check if any assumed revered bit was set or should
         * be set for VMX operation and bail out if so.
         */
-       if ((cpu_data->linux_cr0 | cr_required1[CR0_IDX]) & X86_CR0_RESERVED)
+       if ((cpu_data->linux_cr0 | cr_required1[CR0_IDX]) & X86_CR0_RESERVED ||
+           (cpu_data->linux_cr4 | cr_required1[CR4_IDX]) & X86_CR4_RESERVED)
                return -EIO;
        /*
-        * Bring CR0 into well-defined state. If it doesn't match with VMX
-        * requirements, vmxon will fail.
+        * Bring CR0 and CR4 into well-defined states. If they do not match
+        * with VMX requirements, vmxon will fail.
+        * X86_CR4_OSXSAVE is enabled if available so that xsetbv can be
+        * executed on behalf of a cell.
         */
        write_cr0(X86_CR0_HOST_STATE);
-
-       write_cr4(cr4 | X86_CR4_VMXE);
-       // TODO: validate CR4
+       write_cr4(X86_CR4_HOST_STATE | X86_CR4_VMXE |
+                 ((cpuid_ecx(1) & X86_FEATURE_XSAVE) ? X86_CR4_OSXSAVE : 0));
 
        if (!vmxon(cpu_data))  {
-               write_cr4(cr4);
+               write_cr4(cpu_data->linux_cr4);
                return -EIO;
        }
 
@@ -643,7 +644,7 @@ void vcpu_exit(struct per_cpu *cpu_data)
 
        vmcs_clear(cpu_data);
        asm volatile("vmxoff" : : : "cc");
-       write_cr4(read_cr4() & ~X86_CR4_VMXE);
+       cpu_data->linux_cr4 &= ~X86_CR4_VMXE;
 }
 
 void __attribute__((noreturn)) vcpu_activate_vmm(struct per_cpu *cpu_data)
@@ -677,6 +678,7 @@ vcpu_deactivate_vmm(struct registers *guest_regs)
 
        cpu_data->linux_cr0 = vmcs_read64(GUEST_CR0);
        cpu_data->linux_cr3 = vmcs_read64(GUEST_CR3);
+       cpu_data->linux_cr4 = vmcs_read64(GUEST_CR4);
 
        cpu_data->linux_gdtr.base = vmcs_read64(GUEST_GDTR_BASE);
        cpu_data->linux_gdtr.limit = vmcs_read64(GUEST_GDTR_LIMIT);