Skip to content

Commit 2657b82

Browse files
committed
KVM: nVMX: Treat vpid01 as current if L2 is active, but with VPID disabled
When getting the current VPID, e.g. to emulate a guest TLB flush, return vpid01 if L2 is running but with VPID disabled, i.e. if VPID is disabled in vmcs12. Architecturally, if VPID is disabled, then the guest and host effectively share VPID=0. KVM emulates this behavior by using vpid01 when running an L2 with VPID disabled (see prepare_vmcs02_early_rare()), and so KVM must also treat vpid01 as the current VPID while L2 is active. Unconditionally treating vpid02 as the current VPID when L2 is active causes KVM to flush TLB entries for vpid02 instead of vpid01, which results in TLB entries from L1 being incorrectly preserved across nested VM-Enter to L2 (L2=>L1 isn't problematic, because the TLB flush after nested VM-Exit flushes vpid01). The bug manifests as failures in the vmx_apicv_test KVM-Unit-Test, as KVM incorrectly retains TLB entries for the APIC-access page across a nested VM-Enter. Opportunisticaly add comments at various touchpoints to explain the architectural requirements, and also why KVM uses vpid01 instead of vpid02. All credit goes to Chao, who root caused the issue and identified the fix. Link: https://lore.kernel.org/all/ZwzczkIlYGX+QXJz@intel.com Fixes: 2b4a5a5 ("KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST") Cc: stable@vger.kernel.org Cc: Like Xu <like.xu.linux@gmail.com> Debugged-by: Chao Gao <chao.gao@intel.com> Reviewed-by: Chao Gao <chao.gao@intel.com> Tested-by: Chao Gao <chao.gao@intel.com> Link: https://lore.kernel.org/r/20241031202011.1580522-1-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 979956b commit 2657b82

File tree

2 files changed

+26
-6
lines changed

2 files changed

+26
-6
lines changed

arch/x86/kvm/vmx/nested.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,11 +1197,14 @@ static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu,
11971197
kvm_hv_nested_transtion_tlb_flush(vcpu, enable_ept);
11981198

11991199
/*
1200-
* If vmcs12 doesn't use VPID, L1 expects linear and combined mappings
1201-
* for *all* contexts to be flushed on VM-Enter/VM-Exit, i.e. it's a
1202-
* full TLB flush from the guest's perspective. This is required even
1203-
* if VPID is disabled in the host as KVM may need to synchronize the
1204-
* MMU in response to the guest TLB flush.
1200+
* If VPID is disabled, then guest TLB accesses use VPID=0, i.e. the
1201+
* same VPID as the host, and so architecturally, linear and combined
1202+
* mappings for VPID=0 must be flushed at VM-Enter and VM-Exit. KVM
1203+
* emulates L2 sharing L1's VPID=0 by using vpid01 while running L2,
1204+
* and so KVM must also emulate TLB flush of VPID=0, i.e. vpid01. This
1205+
* is required if VPID is disabled in KVM, as a TLB flush (there are no
1206+
* VPIDs) still occurs from L1's perspective, and KVM may need to
1207+
* synchronize the MMU in response to the guest TLB flush.
12051208
*
12061209
* Note, using TLB_FLUSH_GUEST is correct even if nested EPT is in use.
12071210
* EPT is a special snowflake, as guest-physical mappings aren't
@@ -2315,6 +2318,17 @@ static void prepare_vmcs02_early_rare(struct vcpu_vmx *vmx,
23152318

23162319
vmcs_write64(VMCS_LINK_POINTER, INVALID_GPA);
23172320

2321+
/*
2322+
* If VPID is disabled, then guest TLB accesses use VPID=0, i.e. the
2323+
* same VPID as the host. Emulate this behavior by using vpid01 for L2
2324+
* if VPID is disabled in vmcs12. Note, if VPID is disabled, VM-Enter
2325+
* and VM-Exit are architecturally required to flush VPID=0, but *only*
2326+
* VPID=0. I.e. using vpid02 would be ok (so long as KVM emulates the
2327+
* required flushes), but doing so would cause KVM to over-flush. E.g.
2328+
* if L1 runs L2 X with VPID12=1, then runs L2 Y with VPID12 disabled,
2329+
* and then runs L2 X again, then KVM can and should retain TLB entries
2330+
* for VPID12=1.
2331+
*/
23182332
if (enable_vpid) {
23192333
if (nested_cpu_has_vpid(vmcs12) && vmx->nested.vpid02)
23202334
vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->nested.vpid02);
@@ -5950,6 +5964,12 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
59505964
return nested_vmx_fail(vcpu,
59515965
VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
59525966

5967+
/*
5968+
* Always flush the effective vpid02, i.e. never flush the current VPID
5969+
* and never explicitly flush vpid01. INVVPID targets a VPID, not a
5970+
* VMCS, and so whether or not the current vmcs12 has VPID enabled is
5971+
* irrelevant (and there may not be a loaded vmcs12).
5972+
*/
59535973
vpid02 = nested_get_vpid02(vcpu);
59545974
switch (type) {
59555975
case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:

arch/x86/kvm/vmx/vmx.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3216,7 +3216,7 @@ void vmx_flush_tlb_all(struct kvm_vcpu *vcpu)
32163216

32173217
static inline int vmx_get_current_vpid(struct kvm_vcpu *vcpu)
32183218
{
3219-
if (is_guest_mode(vcpu))
3219+
if (is_guest_mode(vcpu) && nested_cpu_has_vpid(get_vmcs12(vcpu)))
32203220
return nested_get_vpid02(vcpu);
32213221
return to_vmx(vcpu)->vpid;
32223222
}

0 commit comments

Comments
 (0)