Re: [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12

[Sean Christopherson <seanjc@google.com>]

On Thu, Nov 25, 2021, Lai Jiangshan wrote:
> On Thu, Nov 25, 2021 at 9:49 AM Sean Christopherson <seanjc@google.com> wrote:
> > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> > index 2ef1d5562a54..dafe5881ae51 100644
> > --- a/arch/x86/kvm/vmx/nested.c
> > +++ b/arch/x86/kvm/vmx/nested.c
> > @@ -1162,29 +1162,26 @@ static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu,
> >         WARN_ON(!enable_vpid);
> >
> >         /*
> > -        * If VPID is enabled and used by vmc12, but L2 does not have a unique
> > -        * TLB tag (ASID), i.e. EPT is disabled and KVM was unable to allocate
> > -        * a VPID for L2, flush the current context as the effective ASID is
> > -        * common to both L1 and L2.
> > -        *
> > -        * Defer the flush so that it runs after vmcs02.EPTP has been set by
> > -        * KVM_REQ_LOAD_MMU_PGD (if nested EPT is enabled) and to avoid
> > -        * redundant flushes further down the nested pipeline.
> > -        *
> > -        * If a TLB flush isn't required due to any of the above, and vpid12 is
> > -        * changing then the new "virtual" VPID (vpid12) will reuse the same
> > -        * "real" VPID (vpid02), and so needs to be flushed.  There's no direct
> > -        * mapping between vpid02 and vpid12, vpid02 is per-vCPU and reused for
> > -        * all nested vCPUs.  Remember, a flush on VM-Enter does not invalidate
> > -        * guest-physical mappings, so there is no need to sync the nEPT MMU.
> > +        * VPID is enabled and in use by vmcs12.  If vpid12 is changing, then
> > +        * emulate a guest TLB flush as KVM does not track vpid12 history nor
> > +        * is the VPID incorporated into the MMU context.  I.e. KVM must assume
> > +        * that the new vpid12 has never been used and thus represents a new
> > +        * guest ASID that cannot have entries in the TLB.
> >          */
> > -       if (!nested_has_guest_tlb_tag(vcpu)) {
> > -               kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> > -       } else if (is_vmenter &&
> > -                  vmcs12->virtual_processor_id != vmx->nested.last_vpid) {
> > +       if (is_vmenter && vmcs12->virtual_processor_id != vmx->nested.last_vpid) {
> >                 vmx->nested.last_vpid = vmcs12->virtual_processor_id;
> 
> How about when vmx->nested.last_vpid == vmcs12->virtual_processor_id == 0?
> 
> I think KVM_REQ_TLB_FLUSH_GUEST is needed in this case too.

That's handled by code that's just out of sight in this diff.  The first check in
nested_vmx_transition_tlb_flush() is to see if vmcs12 has VPID enabled.  If the
code in this patch is reached, vmcs12->virtual_processor_id is guaranteed to be
non-zero as VM-Enter fails if VPID is enabled but VPID==0.

static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu,
					    struct vmcs12 *vmcs12,
					    bool is_vmenter)
{
	struct vcpu_vmx *vmx = to_vmx(vcpu);

	/*
	 * If vmcs12 doesn't use VPID, L1 expects linear and combined mappings
	 * for *all* contexts to be flushed on VM-Enter/VM-Exit, i.e. it's a
	 * full TLB flush from the guest's perspective.  This is required even
	 * if VPID is disabled in the host as KVM may need to synchronize the
	 * MMU in response to the guest TLB flush.
	 *
	 * Note, using TLB_FLUSH_GUEST is correct even if nested EPT is in use.
	 * EPT is a special snowflake, as guest-physical mappings aren't
	 * flushed on VPID invalidations, including VM-Enter or VM-Exit with
	 * VPID disabled.  As a result, KVM _never_ needs to sync nEPT
	 * entries on VM-Enter because L1 can't rely on VM-Enter to flush
	 * those mappings.
	 */
	if (!nested_cpu_has_vpid(vmcs12)) {
		kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
		return;
	}

	...
}