Re: [PATCH v2 2/4] KVM: PPC: Book3S HV: track the state GFNs associated with secure VMs

On Thu, Jun 18, 2020 at 03:31:06PM +0200, Laurent Dufour wrote:

Le 18/06/2020 à 11:19, Ram Pai a écrit :

quoted

.snip..

quoted

************************************************************************
 1. States of a GFN
    ---------------
 The GFN can be in one of the following states.

diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c

...snip...

quoted

index 803940d..3448459 100644

--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c

@@ -1100,7 +1100,7 @@ void kvmppc_radix_flush_memslot(struct kvm *kvm,
 	unsigned int shift;
 	if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START)
-		kvmppc_uvmem_drop_pages(memslot, kvm, true);
+		kvmppc_uvmem_drop_pages(memslot, kvm, true, false);

When reviewing the v1 of this series, I asked you the question about
the fact that the call here is made with purge_gfn = false. Your
answer was:

quoted

This function does not know, under what context it is called. Since
its job is to just flush the memslot, it cannot assume anything
about purging the pages in the memslot.

Indeed in the case of the memory hotplug operation, this function is
called to wipe the page from the secure device in the case the pages
are secured. In that case the purge is required. Indeed, I checked
the other call to kvmppc_radix_flush_memslot() in
kvmppc_core_flush_memslot_hv() and I cannot see why in that case too
purge_gfn should be false, especially when the memslot is reused as
detailed in __kvm_set_memory_region() around the call to
kvm_arch_flush_shadow_memslot().

I'm sorry to not have ask this earlier, but could you please elaborate on this?

You are right. kvmppc_radix_flush_memslot() is getting called everytime with
the intention of disassociating the memslot from that VM. Which implies,
the memslot is intended to be deleted and possibly reused.

I should be calling kvmppc_uvmem_drop_pages() with purge_gfn=true, here
aswell.

I expect some form of problem showing up in memhot-plug/unplug path.

RP