linux-kernel - Re: [PATCH] KVM: nVMX: Always use TLB_FLUSH

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]

Message-ID: <Z4mlsr-xJnKxnDKc@google.com>
Date: Thu, 16 Jan 2025 16:34:58 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: Jim Mattson <jmattson@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: nVMX: Always use TLB_FLUSH_GUEST for nested VM-Enter/VM-Exit

On Thu, Jan 16, 2025, Yosry Ahmed wrote:
> On Thu, Jan 16, 2025 at 2:35 PM Sean Christopherson <seanjc@...gle.com> wrote:
> > How about:
> >
> >          * Note, only the hardware TLB entries need to be flushed, as VPID is
> >          * fully enabled from L1's perspective, i.e. there's no architectural
> >          * TLB flush from L1's perspective.
> 
> I hate to bikeshed, but I want to explicitly call out that we do not
> need to synchronize the MMU.

Why?  Honest question, I want to understand what's unclear.  My hesitation to
talk about synchronizing MMUs is that it brings things into play that aren't
super relevant to this specific code, and might even add confusion.  Specifically,
kvm_vcpu_flush_tlb_guest() does NOT synchronize MMUs when EPT/TDP is enabled, but
the fact that this path is reachable if and only if EPT is enabled is completely
coincidental.

E.g. very hypothetically, if KVM used the same EPT root (I already forgot Intel's
new acronym) for L1 and L2, then this would no longer be true:

 * If L0 uses EPT, L1 and L2 run with different EPTP because
 * guest_mode is part of kvm_mmu_page_role. Thus, TLB entries
 * are tagged with different EPTP.

L1 vs. L2 EPT usage would no longer use separate ASID tags, and so KVM would
need to FLUSH_CURRENT on transitions in most cases, e.g. to purge APICv mappings.

The comment above !nested_cpu_has_vpid() talks at length about synchronizing MMUs
because the EPT behavior in particular is subtle and rather unintuitive.  I.e.
the comment is much more about NOT using KVM_REQ_MMU_SYNC than it is about using
KVM_REQ_TLB_FLUSH_GUEST.

> Maybe this?
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 2ed454186e59c..a9171909de63d 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -1239,6 +1239,11 @@ static void
> nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu,
>          * 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.
> +        *
> +        * Note, only the hardware TLB entries need to be flushed, as VPID is
> +        * fully enabled from L1's perspective, i.e. there's no
> +        * architectural TLB flush from L1's perspective. Hence, synchronizing
> +        * the MMU is not required as the mappings are still valid.
>          */
>         if (!nested_has_guest_tlb_tag(vcpu))
>                 kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);