KVM: nVMX: eVMCS: Filter out VM_EXIT_SAVE_VMX_PREEMPTION_TIMER
Subsystems (2)
Sasha Levin
maintainerPATCH 1/2 KVM: nVMX: eVMCS: Filter out VM_EXIT_SAVE_VMX_PREEMPTION_TIMER
From: Vitaly Kuznetsov <[email protected]>
[ Upstream commit 7a601e2cf61558dfd534a9ecaad09f5853ad8204 ]
Enlightened VMCS v1 doesn't have VMX_PREEMPTION_TIMER_VALUE field, PIN_BASED_VMX_PREEMPTION_TIMER is also filtered out already so it makes sense to filter out VM_EXIT_SAVE_VMX_PREEMPTION_TIMER too.
Note, none of the currently existing Windows/Hyper-V versions are known to enable 'save VMX-preemption timer value' when eVMCS is in use, the change is aimed at making the filtering future proof.
Message-Id: <[email protected]>
--- arch/x86/kvm/vmx/evmcs.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
arch/x86/kvm/vmx/evmcs.h +3 -1
@@ -58,7 +58,9 @@ DECLARE_STATIC_KEY_FALSE(enable_evmcs); SECONDARY_EXEC_SHADOW_VMCS | \ SECONDARY_EXEC_TSC_SCALING | \ SECONDARY_EXEC_PAUSE_LOOP_EXITING) -#define EVMCS1_UNSUPPORTED_VMEXIT_CTRL (VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL) +#define EVMCS1_UNSUPPORTED_VMEXIT_CTRL \ + (VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL | \ + VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)#define EVMCS1_UNSUPPORTED_VMENTRY_CTRL (VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL)#define EVMCS1_UNSUPPORTED_VMFUNC (VMX_VMFUNC_EPTP_SWITCHING)
Signature · 1 line
2.34.1
Sasha Levin
maintainerPATCH 2/2 KVM: nVMX: WARN on any attempt to allocate shadow VMCS for vmcs02
From: Sean Christopherson <[email protected]>
[ Upstream commit d6e656cd266cdcc95abd372c7faef05bee271d1a ]
WARN if KVM attempts to allocate a shadow VMCS for vmcs02. KVM emulates VMCS shadowing but doesn't virtualize it, i.e. KVM should never allocate a "real" shadow VMCS for L2.
The previous code WARNed but continued anyway with the allocation, presumably in an attempt to avoid NULL pointer dereference. However, alloc_vmcs (and hence alloc_shadow_vmcs) can fail, and indeed the sole caller does:
if (enable_shadow_vmcs && !alloc_shadow_vmcs(vcpu)) goto out_shadow_vmcs;
which makes it not a useful attempt.
Message-Id: <[email protected]>
--- arch/x86/kvm/vmx/nested.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
arch/x86/kvm/vmx/nested.c +12 -10
@@ -4360,18 +4360,20 @@ static struct vmcs *alloc_shadow_vmcs(struct kvm_vcpu *vcpu)struct loaded_vmcs *loaded_vmcs = vmx->loaded_vmcs;/* -* We should allocate a shadow vmcs for vmcs01 only when L1 -* executes VMXON and free it when L1 executes VMXOFF. -* As it is invalid to execute VMXON twice, we shouldn't reach -* here when vmcs01 already have an allocated shadow vmcs. +* KVM allocates a shadow VMCS only when L1 executes VMXON and frees it +* when L1 executes VMXOFF or the vCPU is forced out of nested +* operation. VMXON faults if the CPU is already post-VMXON, so it +* should be impossible to already have an allocated shadow VMCS. KVM +* doesn't support virtualization of VMCS shadowing, so vmcs01 should +* always be the loaded VMCS. */ -WARN_ON (loaded_vmcs == &vmx->vmcs01 && loaded_vmcs->shadow_vmcs); +if (WARN_ON (loaded_vmcs != &vmx->vmcs01 || loaded_vmcs->shadow_vmcs)) +return loaded_vmcs->shadow_vmcs; + + loaded_vmcs->shadow_vmcs =alloc_vmcs (true); +if (loaded_vmcs->shadow_vmcs) +vmcs_clear (loaded_vmcs->shadow_vmcs); -if (!loaded_vmcs->shadow_vmcs) { - loaded_vmcs->shadow_vmcs =alloc_vmcs (true); -if (loaded_vmcs->shadow_vmcs) -vmcs_clear (loaded_vmcs->shadow_vmcs); - }return loaded_vmcs->shadow_vmcs; }
Signature · 1 line
2.34.1
Re: [PATCH MANUALSEL 5.4 2/2] KVM: nVMX: WARN on any attempt to allocate shadow VMCS for vmcs02
Quoted 51 lines · Sasha Levin
From: Sean Christopherson <[email protected]>
[ Upstream commit
d6e656cd266cdcc95abd372c7faef05bee271d1a]WARN if KVM attempts to allocate a shadow VMCS for vmcs02. KVM emulates VMCS shadowing but doesn't virtualize it, i.e. KVM should never allocate a "real" shadow VMCS for L2.
The previous code WARNed but continued anyway with the allocation, presumably in an attempt to avoid NULL pointer dereference. However,
alloc_vmcs(and hencealloc_shadow_vmcs) can fail, and indeed the sole caller does:if (enable_shadow_vmcs && !alloc_shadow_vmcs(vcpu)) goto out_shadow_vmcs;which makes it not a useful attempt.
Signed-off-by Sean Christopherson <[email protected]>Message-Id: <[email protected]>
Signed-off-by Paolo Bonzini <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- arch/x86/kvm/vmx/nested.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)arch/x86/kvm/vmx/nested.c +11
-5@@ -4360,18 +4360,20 @@ static struct vmcs *alloc_shadow_vmcs(struct kvm_vcpu *vcpu)struct loaded_vmcs *loaded_vmcs = vmx->loaded_vmcs;/* -* We should allocate a shadow vmcs for vmcs01 only when L1 -* executes VMXON and free it when L1 executes VMXOFF. -* As it is invalid to execute VMXON twice, we shouldn't reach -* here when vmcs01 already have an allocated shadow vmcs. +* KVM allocates a shadow VMCS only when L1 executes VMXON and frees it +* when L1 executes VMXOFF or the vCPU is forced out of nested +* operation. VMXON faults if the CPU is already post-VMXON, so it +* should be impossible to already have an allocated shadow VMCS. KVM +* doesn't support virtualization of VMCS shadowing, so vmcs01 should +* always be the loaded VMCS. */ -WARN_ON (loaded_vmcs == &vmx->vmcs01 && loaded_vmcs->shadow_vmcs); +if (WARN_ON (loaded_vmcs != &vmx->vmcs01 || loaded_vmcs->shadow_vmcs)) +return loaded_vmcs->shadow_vmcs; + + loaded_vmcs->shadow_vmcs =alloc_vmcs (true); +if (loaded_vmcs->shadow_vmcs)
arch/x86/kvm/vmx/nested.c static struct vmcs *alloc_shadow_vmcs(struct kvm_vcpu *vcpu) · line 4379
Quoted 10 lines
arch/x86/kvm/vmx/nested.c +1
-5+vmcs_clear (loaded_vmcs->shadow_vmcs); -if (!loaded_vmcs->shadow_vmcs) { - loaded_vmcs->shadow_vmcs =alloc_vmcs (true); -if (loaded_vmcs->shadow_vmcs) -vmcs_clear (loaded_vmcs->shadow_vmcs); - }return loaded_vmcs->shadow_vmcs; }
NACK
Quoted 3 lines · Sasha Levin
From: Vitaly Kuznetsov <[email protected]>
[ Upstream commit
7a601e2cf61558dfd534a9ecaad09f5853ad8204]
Paolo
Quoted 31 lines
Enlightened VMCS v1 doesn't have
VMX_PREEMPTION_TIMER_VALUEfield,PIN_BASED_VMX_PREEMPTION_TIMERis also filtered out already so it makes sense to filter outVM_EXIT_SAVE_VMX_PREEMPTION_TIMERtoo.Note, none of the currently existing Windows/Hyper-V versions are known to enable 'save VMX-preemption timer value' when eVMCS is in use, the change is aimed at making the filtering future proof.
Signed-off-by Vitaly Kuznetsov <[email protected]>Message-Id: <[email protected]>
Signed-off-by Paolo Bonzini <[email protected]>Signed-off-by Sasha Levin <[email protected]>--- arch/x86/kvm/vmx/evmcs.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)arch/x86/kvm/vmx/evmcs.h +3
-1@@ -58,7 +58,9 @@ DECLARE_STATIC_KEY_FALSE(enable_evmcs); SECONDARY_EXEC_SHADOW_VMCS | \ SECONDARY_EXEC_TSC_SCALING | \ SECONDARY_EXEC_PAUSE_LOOP_EXITING) -#define EVMCS1_UNSUPPORTED_VMEXIT_CTRL (VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL) +#define EVMCS1_UNSUPPORTED_VMEXIT_CTRL \ + (VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL | \ + VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)#define EVMCS1_UNSUPPORTED_VMENTRY_CTRL (VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL)#define EVMCS1_UNSUPPORTED_VMFUNC (VMX_VMFUNC_EPTP_SWITCHING)