linux-kernel - Re: [PATCH v2 23/32] KVM: nVMX: Add helper to handle TLB flushes on nested VM-Enter/VM-Exit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Mar 2020 11:18:32 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Ben Gardon <bgardon@...gle.com>,
        Junaid Shahid <junaids@...gle.com>,
        Liran Alon <liran.alon@...cle.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        John Haxby <john.haxby@...cle.com>,
        Miaohe Lin <linmiaohe@...wei.com>,
        Tom Lendacky <thomas.lendacky@....com>
Subject: Re: [PATCH v2 23/32] KVM: nVMX: Add helper to handle TLB flushes on
 nested VM-Enter/VM-Exit

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.