Re: [PATCH 01/21] KVM: x86/mmu: Implement memslot deletion for TDX

[Paolo Bonzini]

On 9/4/24 05:07, Rick Edgecombe wrote:
> Force TDX VMs to use the KVM_X86_QUIRK_SLOT_ZAP_ALL behavior.
>
> TDs cannot use the fast zapping operation to implement memslot deletion for
> a couple reasons:
> 1. KVM cannot fully zap and re-build TDX private PTEs without coordinating
>     with the guest. This is due to the TDs needing to "accept" memory. So
>     an operation to delete a memslot needs to limit the private zapping to
>     the range of the memslot.
> 2. For reason (1), kvm_mmu_zap_all_fast() is limited to direct (shared)
>     roots. This means it will not zap the mirror (private) PTEs. If a
>     memslot is deleted with private memory mapped, the private memory would
>     remain mapped in the TD. Then if later the gmem fd was whole punched,
>     the pages could be freed on the host while still mapped in the TD. This
>     is because that operation would no longer have the memslot to map the
>     pgoff to the gfn.
>
> To handle the first case, userspace could simply set the
> KVM_X86_QUIRK_SLOT_ZAP_ALL quirk for TDs. This would prevent the issue in
> (1), but it is not sufficient to resolve (2) because the problems there
> extend beyond the userspace's TD, to affecting the rest of the host. So the
> zap-leafs-only behavior is required for both
>
> A couple options were considered, including forcing
> KVM_X86_QUIRK_SLOT_ZAP_ALL to always be on for TDs, however due to the
> currently limited quirks interface (no way to query quirks, or force them
> to be disabled), this would require developing additional interfaces. So
> instead just do the simple thing and make TDs always do the zap-leafs
> behavior like when KVM_X86_QUIRK_SLOT_ZAP_ALL is disabled.
>
> While at it, have the new behavior apply to all non-KVM_X86_DEFAULT_VM VMs,
> as the previous behavior was not ideal (see [0]). It is assumed until
> proven otherwise that the other VM types will not be exposed to the bug[1]
> that derailed that effort.
>
> Memslot deletion needs to zap both the private and shared mappings of a
> GFN, so update the attr_filter field in kvm_mmu_zap_memslot_leafs() to
> include both.
>
> Co-developed-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> Link: https://lore.kernel.org/kvm/20190205205443.1059-1-sean.j.christopherson@xxxxxxxxx/ [0]
> Link: https://patchwork.kernel.org/project/kvm/patch/20190205210137.1377-11-sean.j.christopherson@xxxxxxxxx [1]
> ---
> TDX MMU part 2 v1:
>   - Clarify TDX limits on zapping private memory (Sean)
>
> Memslot quirk series:
>   - New patch
> ---
>   arch/x86/kvm/mmu/mmu.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index a8d91cf11761..7e66d7c426c1 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -7104,6 +7104,7 @@ static void kvm_mmu_zap_memslot_leafs(struct kvm *kvm, struct kvm_memory_slot *s
>   		.start = slot->base_gfn,
>   		.end = slot->base_gfn + slot->npages,
>   		.may_block = true,
> +		.attr_filter = KVM_FILTER_PRIVATE | KVM_FILTER_SHARED,
>   	};
>   	bool flush = false;
>   

Stale commit message, I guess.

Paolo