[PATCH] KVM: nVMX: Only update last_vpid on a successful nested VM-Enter

[Yosry Ahmed]

Delay calling nested_vmx_transition_tlb_flush() in the nested VM-Enter
path until all the checks are completed, and performing any requested
TLB flushes for L2 is guaranteed (i.e.
kvm_service_local_tlb_flush_requests() is called in L2's context),
either before L2 is run in vcpu_enter_guest() or as part of a "full"
nested VM-Exit (i.e.  through __nested_vmx_vmexit()).

nested_vmx_transition_tlb_flush() checks if L1 changed L2's VPID in
vmcs12 (among other things), updates last_vpid accordingly, and requests
a TLB flush (through KVM_REQ_TLB_FLUSH_GUEST). With the current code
path, it is possible for the nested VM-Enter to fail after
nested_vmx_transition_tlb_flush() already updates last_vpid, but in this
case KVM will *not* call kvm_service_local_tlb_flush_requests() in L2's
context, and flushing L2's VPID will missed. If L1 later runs L2 with
the same VPID, nested_vmx_transition_tlb_flush() won't detect a change
in VPID, and L2's VPID will not be flushed.

Fixes: 5c614b3583e7 ("KVM: nVMX: nested VPID emulation")
Cc: stable@xxxxxxxxxxxxxxx
Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx> # Internal review
Signed-off-by: Yosry Ahmed <yosry@xxxxxxxxxx>
---

Note: I was able to reproduce the bug by hacking a nested TLB flushes
selftest I am working on as part of nSVM TLB optimizations. I haven't
sent out a new version that includes the selftest yet, but basically the
test includes one test case where L1 updates a mapping and changes L2's
VPID to effectively flush the TLB. Inserting a failed VM-Enter before
the correct VM-Enter makes that test case fail. This patch fixes it.

This is probably too vague, so I will reply to this patch when I send
out the selftest with exact diff needed to repro. This is me just
pointing out that a repro exists, and that I will add a pointer to it
later.


---
 arch/x86/kvm/vmx/nested.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b5460de4b1a72..0a4ea410b0483 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2818,8 +2818,6 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 	if (kvm_caps.has_tsc_control)
 		vmcs_write64(TSC_MULTIPLIER, vcpu->arch.tsc_scaling_ratio);
 
-	nested_vmx_transition_tlb_flush(vcpu, vmcs12, true);
-
 	if (nested_cpu_has_ept(vmcs12))
 		nested_ept_init_mmu_context(vcpu);
 
@@ -3739,6 +3737,8 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 		vmx_start_preemption_timer(vcpu, timer_value);
 	}
 
+	nested_vmx_transition_tlb_flush(vcpu, vmcs12, true);
+
 	/*
 	 * Note no nested_vmx_succeed or nested_vmx_fail here. At this point
 	 * we are no longer running L1, and VMLAUNCH/VMRESUME has not yet

base-commit: 6bc96b971766fbbbbdd9fb2642cedacaf02da957
-- 
2.55.0.229.g6434b31f56-goog