Re: [PATCH 3/3] KVM: arm64: Move stage-2 teardown into kvm_vm_s2_ops

[Suzuki K Poulose]

On 03/11/2026 09:37, Suzuki K Poulose wrote:
> Hi Fuad
>
> On 03/11/2026 08:47, Fuad Tabba wrote:
> > Replace kvm_stage2_destroy()'s is_protected_kvm_enabled() and
> > KVM_PGT_FN() dispatch with a vm_stage2_destroy hook in kvm_vm_s2_ops,
> > like the other stage-2 operations. Both pKVM flavors share
> > pkvm_stage2_destroy(). Like kvm_vm_stage2_destroy(), it takes the range
> > from kvm_phys_size().
> >
> > No functional change intended.
>
> Someone beat you on this (well almost) ;-)
>
> https://lore.kernel.org/all/4304d0c7-2220-4add-9c71-e6dc6b4fbc11@xxxxxxx
>
>
> > Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> > ---
> >   arch/arm64/include/asm/kvm_host.h |  2 ++
> >   arch/arm64/kvm/mmu.c              | 24 +++++++++++++-----------
> >   2 files changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/ 
> > asm/kvm_host.h
> > index 97332ffcb1300..b36779bafae9b 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -233,6 +233,7 @@ struct kvm_vcpu_ops {
> >   struct kvm_gfn_range;
> >   struct kvm_s2_fault_desc;
> > +struct kvm_pgtable;
> >   struct kvm_vm_s2_ops {
> >       bool (*vm_age_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
> > @@ -243,6 +244,7 @@ struct kvm_vm_s2_ops {
> >       void (*vm_stage2_unmap_range)(struct kvm_s2_mmu *mmu,
> >                         phys_addr_t start, u64 size,
> >                         bool may_block);
> > +    void (*vm_stage2_destroy)(struct kvm_pgtable *pgt);
> >       int (*vm_mem_abort)(const struct kvm_s2_fault_desc *s2fd);
> >   };
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 10b425a0141be..9df3c80a5acd1 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1029,17 +1029,16 @@ static void stage2_destroy_range(struct 
> > kvm_pgtable *pgt, phys_addr_t addr,
> >       } while (addr = next, addr != end);
> >   }
> > -static void kvm_stage2_destroy(struct kvm_pgtable *pgt)
> > +static void kvm_vm_stage2_destroy(struct kvm_pgtable *pgt)
> >   {
> > -    unsigned int ia_bits = VTCR_EL2_IPA(pgt->mmu->vtcr);
> > +    stage2_destroy_range(pgt, 0, kvm_phys_size(pgt->mmu));

Also, now that stage2_destroy_range is only called for normal KVM,
could we fold that in here ?

Cheers
Suzuki


> > +    kvm_pgtable_stage2_destroy_pgd(pgt);
> > +}
> > -    /* pKVM walks its mapping tree, not the IPA space, and 
> > reschedules per mapping */
> > -    if (is_protected_kvm_enabled())
> > -        pkvm_pgtable_stage2_destroy_range(pgt, 0, BIT(ia_bits));
> > -    else
> > -        stage2_destroy_range(pgt, 0, BIT(ia_bits));
> > -
> > -    KVM_PGT_FN(kvm_pgtable_stage2_destroy_pgd)(pgt);
> > +static void pkvm_stage2_destroy(struct kvm_pgtable *pgt)
> > +{
> > +    pkvm_pgtable_stage2_destroy_range(pgt, 0, kvm_phys_size(pgt->mmu));
> > +    pkvm_pgtable_stage2_destroy_pgd(pgt);
> >   }
> >   /**
> > @@ -1126,7 +1125,7 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct 
> > kvm_s2_mmu *mmu, unsigned long t
> >       return 0;
> >   out_destroy_pgtable:
> > -    kvm_stage2_destroy(pgt);
> > +    kvm->arch.vm_s2_ops->vm_stage2_destroy(pgt);
>
> minor nit: should we retain the kvm_stage2_destroy() to dispatch the 
> callback ?
>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
>
> Cheers
> Suzuki
>
>
> >       mmu->pgt = NULL;
> >   out_free_pgtable:
> >       kfree(pgt);
> > @@ -1246,7 +1245,7 @@ void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)
> >       write_unlock(&kvm->mmu_lock);
> >       if (pgt) {
> > -        kvm_stage2_destroy(pgt);
> > +        kvm->arch.vm_s2_ops->vm_stage2_destroy(pgt);
> >           kfree(pgt);
> >       }
> > @@ -3152,6 +3151,7 @@ static const struct kvm_vm_s2_ops 
> > protected_pkvm_vm_s2_ops = {
> >       .vm_age_gfn            = no_age_gfn,
> >       .vm_test_age_gfn        = no_age_gfn,
> >       .vm_stage2_unmap_range        = no_stage2_unmap_range,
> > +    .vm_stage2_destroy        = pkvm_stage2_destroy,
> >       .vm_mem_abort            = protected_pkvm_mem_abort,
> >   };
> > @@ -3161,6 +3161,7 @@ static const struct kvm_vm_s2_ops pkvm_vm_s2_ops 
> > = {
> >       .vm_age_gfn            = pkvm_age_gfn,
> >       .vm_test_age_gfn        = pkvm_test_age_gfn,
> >       .vm_stage2_unmap_range        = pkvm_stage2_unmap_range,
> > +    .vm_stage2_destroy        = pkvm_stage2_destroy,
> >       .vm_mem_abort            = kvm_vm_mem_abort,
> >   };
> > @@ -3170,6 +3171,7 @@ static const struct kvm_vm_s2_ops 
> > kvm_default_vm_s2_ops = {
> >       .vm_age_gfn            = kvm_vm_age_gfn,
> >       .vm_test_age_gfn        = kvm_vm_test_age_gfn,
> >       .vm_stage2_unmap_range        = kvm_vm_stage2_unmap_range,
> > +    .vm_stage2_destroy        = kvm_vm_stage2_destroy,
> >       .vm_mem_abort            = kvm_vm_mem_abort,
> >   };