On Tue, Nov 03, 2026, Ritesh Harjani wrote: > Sean Christopherson <[email protected]> writes: > > +Gautam and PPC lists/maintainers > > On Fri, Oct 30, 2026, [email protected] wrote: > >> Thank you for your contribution! Sashiko AI review found 1 potential > >> issue(s) to consider: > >> - [Medium] KVM: PPC: Book3S HV: rmap_nested memory leak in > >> kvmhv_release_all_nested() > >> > @@ -1310,12 +1329,10 @@ static void kvm_destroy_vm(struct kvm *kvm) > >> > kvm->mn_active_invalidate_count = 0; > >> > else > >> > WARN_ON(kvm->mmu_invalidate_in_progress); > >> > + kvm_destroy_memslots(kvm); > >> > + > >> > kvm_arch_destroy_vm(kvm); > >> > kvm_destroy_devices(kvm); > >> > - for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) { > >> > - kvm_free_memslots(kvm, &kvm->__memslots[i][0]); > >> > - kvm_free_memslots(kvm, &kvm->__memslots[i][1]); > >> > - } > >> > >> [Severity: Medium] > >> Does hoisting kvm_destroy_memslots() above kvm_arch_destroy_vm() leak > >> struct rmap_nested allocations in slot->arch.rmap on PowerPC Book3S HV when > >> destroying a VM with nested virtualization enabled? > > > > I don't think so? Patch 3[*], "KVM: PPC: Flush/zap all memslots on > > kvm_arch_flush_shadow_all()", > > should free the rmaps in response to kvm_arch_flush_shadow_all(). From > > what I > > can tell, the page tables have to be in radix mode, and so the rmaps are > > freed via: > > > > kvmppc_radix_flush_memslot() > > | > > -> kvmppc_unmap_pte() > > | > > -> kvmhv_remove_nest_rmap_range() > > | > > -> kvmhv_remove_nest_rmap_list() > > > > That's correct. Maybe, I think Sashiko didn't consider patch-3 of the series > during review of this patch-12. > > > And if that doesn't work, it means memslot deletion on an active VM is > > completely > > broken. > > > > However, it means the memslot code in kvmhv_release_all_nested() is > > dead code, > > Not really. I do see a caller of this, for when we switch from radix > to HPT. So we still need it.
Oh, nice, I somehow missed that. Yay, less work to do! > INCOMING CALLS > - f kvmhv_release_all_nested > - f kvmppc_switch_mmu_to_hpt [arch/powerpc/kvm/book3s_hv.c:5591] > + f kvmppc_alloc_reset_hpt [arch/powerpc/kvm/book3s_64_mmu_hv.c:144] > + f kvmhv_configure_mmu [arch/powerpc/kvm/book3s_hv.c:6330] > + f kvmppc_core_destroy_vm_hv [arch/powerpc/kvm/book3s_hv.c:5930] > > > > So FWIW patch-12 as such looks good to me from PowerPC perspective, > since those rmap entries will get freed up as part of the > mmu_notifier_unregister() (after patch-3). Agreed that Sashiko > comment does not look correct.
