From 13a50c929df5978cfab076662f386e99b0900d79 Mon Sep 17 00:00:00 2001 From: Yin Fengwei Date: Fri, 27 Jul 2018 15:12:56 +0800 Subject: [PATCH] hv: Explicitly trap VMXE and PCIDE bit for CR4 write Now, we let guest own most CR4 bit. Which means guest handles whether the CR4 writting is invalid or not and GP injection if it's invalid writing. Two bits are exception here: we filter VMX and PCID feature to guest (which means they are supported on native). So we can't depends on guest to inject GP for these bits. Instead, we should explicitly trap these CR4 bits update and inject GP to guest from HV. Signed-off-by: Yin Fengwei Acked-by: Anthony Xu --- hypervisor/arch/x86/vmx.c | 39 +++++++++++++++++++------------ hypervisor/include/arch/x86/vmx.h | 2 +- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c index 0518a7cd7..b11ae63b5 100644 --- a/hypervisor/arch/x86/vmx.c +++ b/hypervisor/arch/x86/vmx.c @@ -505,6 +505,23 @@ int vmx_write_cr3(struct vcpu *vcpu, uint64_t cr3) return 0; } +static bool is_cr4_write_valid(struct vcpu *vcpu, uint64_t cr4) +{ + /* Check if guest try to set fixed to 0 bits or reserved bits */ + if ((cr4 & cr4_always_off_mask) != 0U) + return false; + + /* Do NOT support nested guest */ + if ((cr4 & CR4_VMXE) != 0UL) + return false; + + /* Do NOT support PCID in guest */ + if ((cr4 & CR4_PCIDE) != 0UL) + return false; + + return true; +} + /* * Handling of CR4: * Assume "unrestricted guest" feature is supported by vmx. @@ -529,10 +546,13 @@ int vmx_write_cr3(struct vcpu *vcpu, uint64_t cr3) * - PCE (8) Flexible to guest * - OSFXSR (9) Flexible to guest * - OSXMMEXCPT (10) Flexible to guest - * - VMXE (13) must always be 1 => must lead to a VM exit + * - VMXE (13) Trapped to hide from guest * - SMXE (14) must always be 0 => must lead to a VM exit - * - PCIDE (17) Flexible to guest + * - PCIDE (17) Trapped to hide from guest * - OSXSAVE (18) Flexible to guest + * - XSAVE (19) Flexible to guest + * We always keep align with physical cpu. So it's flexible to + * guest * - SMEP (20) Flexible to guest * - SMAP (21) Flexible to guest * - PKE (22) Flexible to guest @@ -543,19 +563,8 @@ int vmx_write_cr4(struct vcpu *vcpu, uint64_t cr4) &vcpu->arch_vcpu.contexts[vcpu->arch_vcpu.cur_context]; uint64_t cr4_vmx; - /* TODO: Check all invalid guest statuses according to the change of - * CR4, and inject a #GP to guest */ - - /* Check if guest try to set fixed to 0 bits or reserved bits */ - if((cr4 & cr4_always_off_mask) != 0U) { - pr_err("Not allow to set reserved/always off bits for CR4"); - vcpu_inject_gp(vcpu, 0U); - return 0; - } - - /* Do NOT support nested guest */ - if ((cr4 & CR4_VMXE) != 0UL) { - pr_err("Nested guest not supported"); + if (!is_cr4_write_valid(vcpu, cr4)) { + pr_dbg("Invalid cr4 write operation from guest"); vcpu_inject_gp(vcpu, 0U); return 0; } diff --git a/hypervisor/include/arch/x86/vmx.h b/hypervisor/include/arch/x86/vmx.h index a7ee662df..7f7c6e2a4 100644 --- a/hypervisor/include/arch/x86/vmx.h +++ b/hypervisor/include/arch/x86/vmx.h @@ -404,7 +404,7 @@ CR0_NE | CR0_ET | CR0_TS | CR0_EM | CR0_MP | CR0_PE) /* CR4 bits hv want to trap to track status change */ -#define CR4_TRAP_MASK (CR4_PSE | CR4_PAE) +#define CR4_TRAP_MASK (CR4_PSE | CR4_PAE | CR4_VMXE | CR4_PCIDE) #define CR4_RESERVED_MASK ~(CR4_VME | CR4_PVI | CR4_TSD | CR4_DE | CR4_PSE | \ CR4_PAE | CR4_MCE | CR4_PGE | CR4_PCE | \ CR4_OSFXSR | CR4_PCIDE | CR4_OSXSAVE | \