Re: [PATCH v2 07/12] KVM: arm64: CCA: Tear down RTTs

[Suzuki K Poulose]

On 13/11/2026 15:05, Steven Price wrote:
> On 13/11/2026 01:04, Suzuki K Poulose wrote:
> > On 11/11/2026 14:57, Steven Price wrote:
> > > The RMM owns the stage 2 page tables for a realm, and KVM must request
> > > that the RMM creates/destroys entries as necessary. The physical pages
> > > to store the page tables are delegated to the realm as required, and can
> > > be undelegated when no longer used.
> > >
> > > Creating new RTTs is the easy part, tearing down is a little more
> > > tricky. The result of realm_rtt_destroy() can be used to effectively
> > > walk the tree and destroy the entries (undelegating pages that were
> > > given to the realm).
> > >
> > > Tear down a created Realm whether it is still new or has been activated.
> > > Unmap the Realm IPA space once during stage-2 teardown. Defer
> > > destruction of non-root RTTs until final Realm destruction, after
> > > REALM_TERMINATE and before destroying the RD; root RTTs remain until the
> > > RD has gone away.
> > >
> > > Signed-off-by: Steven Price <steven.price@xxxxxxx>

...

> > >      #endif /* __ASM_KVM_RMI_H */
> > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > > index 9df3f8d5b4ef..be014f6c00e7 100644
> > > --- a/arch/arm64/kvm/mmu.c
> > > +++ b/arch/arm64/kvm/mmu.c
> > > @@ -1145,9 +1145,25 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct
> > > kvm_s2_mmu *mmu, unsigned long t
> > >        return err;
> > >    }

...

> > >    void kvm_uninit_stage2_mmu(struct kvm *kvm)
> > >    {
> > > -    kvm_free_stage2_pgd(&kvm->arch.mmu);
> > > +    if (kvm_realm_is_created(kvm))
> > > +        kvm_realm_uninit_stage2(&kvm->arch.mmu);
> > > +    else
> > > +        kvm_free_stage2_pgd(&kvm->arch.mmu);
> >
> > We added kvm_free_stage2_pgd() from kvm_destroy_realm() for Realms in
> > the previous patch. Should we move that here since, this is where we
> >
> > 1. Deal with tearing down stage2
> > 2. And more importantly we are skipping it for Realms.
>
> I'm not quite sure what you're asking for here. We can't free the stage2
> PGD while the realm is existing (because the RMM has the pages) - so we
> defer it.

I understand. My point is, we are deferring the stage2 free for Realm
in this patch, without explicitly showing where it is done. We have
introduced that as part of kvm_destroy_realm() in Patch 6.

My recommendation is to add kvm_free_stage2_pgd() to kvm_destroy_realm()
in this patch, where we are showing when this is deferred to. e.g.,
this patch functionally leaves the reader with no clue when this is
actually freed.

Does that make sense ?


> > >        kvm_mmu_free_memory_cache(&kvm->arch.mmu.split_page_cache);
> > >    }
> > >    diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> > > index 69c628cfb6c7..b904e6541fb0 100644
> > > --- a/arch/arm64/kvm/rmi.c
> > > +++ b/arch/arm64/kvm/rmi.c
> > > @@ -11,16 +11,193 @@
> > >    #include <asm/kvm_pgtable.h>
> > >    #include <asm/virt.h>
> > >    +static inline unsigned long rmi_rtt_level_mapsize(int level)
> > > +{
> > > +    if (WARN_ON(level > KVM_PGTABLE_LAST_LEVEL))
> > > +        return PAGE_SIZE;
> > > +
> > > +    return (1UL << ARM64_HW_PGTABLE_LEVEL_SHIFT(level));
> > > +}
> > > +
> >
> > We agreed to use kvm_granule_size() instead of this ?
> >
> > https://lore.kernel.org/all/cb183290-3fee-4535-aa3e-5cc172c2323a@xxxxxxx
>
> That was a screw-up when I was squashing fixes back in - the next patch
> removes the code. I'll fix this up in the next version so it never
> appears. Thanks for pointing it out.
>
> > >    u32 kvm_rmm_ipa_limit(void)
> > >    {
> > >        return u64_get_bits(rmi_feat_reg(0), RMI_FEATURE_REGISTER_0_S2SZ);
> > >    }
> > >    +static int get_start_level(struct realm *realm)
> > > +{
> > > +    return 4 - max(2, stage2_pgtable_levels(realm->ia_bits));
> >
> > I don't think we need this "max()". ia_bits is guaranteed to be
> > > = 32bit (ARM64_MIN_PARANGE_BITS, see kvm_init_ipa_range()), implying
> > that we have at least 2 levels on all page sizes.
>
> Ah, very true - Sashiko complained, but I hadn't noticed
> ARM64_MIN_PARANGE_BITS provided an effective minimum. I'll remove the
> max() and add a comment.
>
> > > +}
> > > +
> > > +static void free_rtt(phys_addr_t phys)
> > > +{
> > > +    kvm_account_pgtable_pages(phys_to_virt(phys), -1);
> > > +    if (rmi_free_delegated_page(phys))
> > > +        kvm_account_pgtable_pages(phys_to_virt(phys), 1);
> >
> > Is there a reason why remove the account before freeing ? Could we do
> > this after we have reclaimed it ?
>
> Sashiko again:
>
> > Does calling kvm_account_pgtable_pages() after rmi_free_delegated_page()
> > access the page after it has already been freed and uncharged from its
> > memcg?
>
> Basically the problem is that rmi_free_delegated_page() frees the page
> back to the allocator. So doing the account after freeing means that
> technically the page could have been reused.

Thanks, for the explanation, that makes sense.

Cheers
Suzuki