Re: [PATCH v4 03/23] KVM: PPC: Flush/zap all memslots on kvm_arch_flush_shadow_all()

[Narayana Murty N]

Hi Sean,

On 31/10/26 2:50 AM, Sean Christopherson wrote:
> Flush/zap every memslot in kvm_arch_flush_shadow_all(), i.e. when the
> owning userspace process is exiting, as KVM must ensure all associated
> memory is unmapped from the guest.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>   arch/powerpc/include/asm/kvm_host.h |  1 -
>   arch/powerpc/kvm/powerpc.c          | 11 +++++++++++
>   2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 708a32f89776..50a42f80e5f1 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -910,7 +910,6 @@ struct kvm_vcpu_arch {
>   #define __KVM_HAVE_CREATE_DEVICE
>   
>   static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}
> -static inline void kvm_arch_flush_shadow_all(struct kvm *kvm) {}
>   static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
>   static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
>   
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 9194cf492d1c..9e9d0865c2f6 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -764,6 +764,17 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>   	kvmppc_core_commit_memory_region(kvm, old, new, change);
>   }
>   
> +void kvm_arch_flush_shadow_all(struct kvm *kvm)
> +{
> +	struct kvm_memory_slot *slot;
> +	struct kvm_memslots *slots;
> +	int bkt;
> +
> +	slots = kvm_memslots(kvm);
> +	kvm_for_each_memslot(slot, bkt, slots)
> +		kvmppc_core_flush_memslot(kvm, slot);

For Book3S HV/hash, I am not sure the existing flush_memslot helper can
be used on an active memslot like this.

The normal memslot invalidation path seems to deliberately establish
the opposite ordering.  kvm_invalidate_memslot() first installs an
INVALID slot and makes it active:

	invalid_slot->flags |= KVM_MEMSLOT_INVALID;
	kvm_replace_memslot(kvm, old, invalid_slot);
	kvm_swap_active_memslots(kvm, old->as_id);
and only then does kvm_arch_flush_shadow_memslot(kvm, old);.

The comment there explicitly states:
/*
 * From this point no new shadow pages pointing to a deleted,
 * or moved, memslot will be created.
 */

Similarly, kvmppc_core_flush_memslot_hv() in
arch/powerpc/kvm/book3s_64_mmu_hv.c relies on the memslot already being 
invalid when testing KVMPPC_RMAP_PRESENT without locking, as no new 
HPTEs can be created.

However, kvm_arch_flush_shadow_all() in arch/powerpc/kvm/powerpc.c now 
invokes the same helper while walking active memslots.

Is there any serialization in the mmu_notifier.release() path that 
provides the same guarantee against new HPTE creation?

Otherwise, couldn't an H_ENTER or fault path create a new HPTE while the 
rmap is being scanned, or after it has already been examined?

> +}
> +
>   void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
>   				   struct kvm_memory_slot *slot)
>   {

Thanks,
Narayana Murty.