On Tue, Mar 17, 2020 at 06:17:59PM +0100, Paolo Bonzini wrote:
> On 17/03/20 05:52, Sean Christopherson wrote:
> > + nested_vmx_transition_tlb_flush(vcpu, vmcs12);
> > +
> > + /*
> > + * There is no direct mapping between vpid02 and vpid12, vpid02 is
> > + * per-vCPU and reused for all nested vCPUs. If vpid12 is changing
> > + * then the new "virtual" VPID will reuse the same "real" VPID,
> > + * vpid02, and so needs to be sync'd. Skip the sync if a TLB flush
> > + * has already been requested, but always update the last used VPID.
> > + */
> > + if (nested_cpu_has_vpid(vmcs12) && nested_has_guest_tlb_tag(vcpu) &&
> > + vmcs12->virtual_processor_id != vmx->nested.last_vpid) {
> > + vmx->nested.last_vpid = vmcs12->virtual_processor_id;
> > + if (!kvm_test_request(KVM_REQ_TLB_FLUSH, vcpu))
> > + vpid_sync_context(nested_get_vpid02(vcpu));
> > }
>
> Would it make sense to move nested_vmx_transition_tlb_flush into an
> "else" branch?
Maybe? I tried that at one point, but didn't like making the call to
nested_vmx_transition_tlb_flush() conditional. My intent is to have
the ...tlb_flush() call be standalone, i.e. logic that is common to all
nested transitions, so that someone can look at the code can easily
(relatively speaking) understand the basic rules for TLB flushing on
nested transitions.
I also tried the oppositie, i.e. putting the above code in an else-branch,
with nested_vmx_transition_tlb_flush() returning true if it requested a
flush. But that required updating vmx->nested.last_vpid in a separate
flow, which was quite awkward.
> And should this also test that KVM_REQ_TLB_FLUSH_CURRENT is not set?
Doh, yes.
