Received-SPF: pass (google.com: domain of linux-kernel+bounces-205665-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240530210714.364118-1-rick.p.edgecombe@intel.com> <20240530210714.364118-15-rick.p.edgecombe@intel.com>
In-Reply-To: <20240530210714.364118-15-rick.p.edgecombe@intel.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Fri, 7 Jun 2024 11:03:25 +0200
Message-ID: <CABgObfYL9uujoLTmSBW0LqoQbOGKpfZsB50BZqMo5_WOChrZ-A@mail.gmail.com>
Subject: Re: [PATCH v2 14/15] KVM: x86/tdp_mmu: Invalidate correct roots
To: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: seanjc@google.com, kvm@vger.kernel.org, kai.huang@intel.com, 
	dmatlack@google.com, erdemaktas@google.com, isaku.yamahata@gmail.com, 
	linux-kernel@vger.kernel.org, sagis@google.com, yan.y.zhao@intel.com, 
	Sean Christopherson <sean.j.christopherson@intel.com>, Isaku Yamahata <isaku.yamahata@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 30, 2024 at 11:07=E2=80=AFPM Rick Edgecombe
<rick.p.edgecombe@intel.com> wrote:
>
> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> When invalidating roots, respect the root type passed.
>
> kvm_tdp_mmu_invalidate_roots() is called with different root types. For
> kvm_mmu_zap_all_fast() it only operates on shared roots. But when tearing
> down a VM it needs to invalidate all roots. Check the root type in root
> iterator.

This patch and patch 12 are small enough that they can be merged.

> @@ -1135,6 +1135,7 @@ void kvm_tdp_mmu_zap_invalidated_roots(struct kvm *=
kvm)
>  void kvm_tdp_mmu_invalidate_roots(struct kvm *kvm,
>                                   enum kvm_process process_types)
>  {
> +       enum kvm_tdp_mmu_root_types root_types =3D kvm_process_to_root_ty=
pes(kvm, process_types);

Maybe pass directly enum kvm_tdp_mmu_root_types?

Looking at patch 12:

+ /*
+ * The private page tables doesn't support fast zapping.  The
+ * caller should handle it by other way.
+ */
+ kvm_tdp_mmu_invalidate_roots(kvm, KVM_PROCESS_SHARED);

now that we have separated private-ness and external-ness, it sounds
much better to write:

/*
 * External page tables don't support fast zapping, therefore
 * their mirrors must be invalidated separately by the caller.
 */
kvm_tdp_mmu_invalidate_roots(kvm, KVM_DIRECT_ROOTS);

while kvm_mmu_uninit_tdp_mmu() can pass KVM_ANY_ROOTS. It may also be
worth adding an

if (WARN_ON_ONCE(root_types & KVM_INVALID_ROOTS))
  root_types &=3D ~KVM_INVALID_ROOTS;

to document the invariants.

Paolo