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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 02/11/2026 14:12, 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).
> 
> 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@arm.com>
> ---
>   arch/arm64/include/asm/kvm_rmi.h |   9 ++
>   arch/arm64/kvm/mmu.c             |  17 ++-
>   arch/arm64/kvm/rmi.c             | 189 +++++++++++++++++++++++++++++++
>   3 files changed, 214 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
> index f1a1ed6b91f5..58d825e40bdf 100644
> --- a/arch/arm64/include/asm/kvm_rmi.h
> +++ b/arch/arm64/include/asm/kvm_rmi.h
> @@ -57,6 +57,7 @@ enum realm_state {
>    * @sro: Preallocated SRO state context for Realm MMU operations
>    * @state: The lifetime state machine for the realm
>    * @ia_bits: Number of valid Input Address bits in the IPA
> + * @stage2_unmapped: The Realm stage-2 mappings have been removed
>    */
>   struct realm {
>   	void *rd;
> @@ -70,6 +71,7 @@ struct realm {
>   	struct rmi_sro_state *sro;
>   	enum realm_state state;
>   	unsigned int ia_bits;
> +	bool stage2_unmapped;
>   };
>   
>   void kvm_init_rmi(void);
> @@ -77,5 +79,12 @@ u32 kvm_rmm_ipa_limit(void);
>   
>   int kvm_init_realm(struct kvm *kvm);
>   void kvm_destroy_realm(struct kvm *kvm);
> +int kvm_realm_teardown_stage2(struct kvm *kvm);
> +
> +static inline bool kvm_realm_is_private_address(struct realm *realm,
> +						unsigned long addr)
> +{
> +	return !(addr & BIT(realm->ia_bits - 1));
> +}
>   
>   #endif /* __ASM_KVM_RMI_H */
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index b049ea67d9af..51fda7b2ff7f 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1145,9 +1145,24 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu, unsigned long t
>   	return err;
>   }
>   
> +static void kvm_realm_uninit_stage2(struct kvm_s2_mmu *mmu)
> +{
> +	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> +
> +	mutex_lock(&kvm->arch.config_lock);
> +	if (kvm_realm_state(kvm) == REALM_STATE_ACTIVE) {
> +		kvm_set_realm_state(kvm, REALM_STATE_DYING);
> +		WARN_ON(kvm_realm_teardown_stage2(kvm));
> +	}
> +	mutex_unlock(&kvm->arch.config_lock);
> +}
> +
>   void kvm_uninit_stage2_mmu(struct kvm *kvm)
>   {
> -	kvm_free_stage2_pgd(&kvm->arch.mmu);
> +	if (kvm_vm_is_realm(kvm))
> +		kvm_realm_uninit_stage2(&kvm->arch.mmu);
> +	else
> +		kvm_free_stage2_pgd(&kvm->arch.mmu);
>   	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 45ecbaa467d1..f190143b33ba 100644
> --- a/arch/arm64/kvm/rmi.c
> +++ b/arch/arm64/kvm/rmi.c
> @@ -11,6 +11,14 @@
>   #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));
> +}


Now that we are using the same PAGE_SIZE as GRANULE size, we could
replace the above helper with kvm_granule_size() ?


>   static bool rmi_has_feature(int reg, unsigned long feature)
>   {
>   	return !!u64_get_bits(rmi_feat_reg(reg), feature);
> @@ -21,11 +29,180 @@ 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 - stage2_pgtable_levels(realm->ia_bits);
> +}
> +
> +static void free_rtt(phys_addr_t phys)
> +{
> +	if (rmi_free_delegated_page(phys))
> +		return;
> +
> +	kvm_account_pgtable_pages(phys_to_virt(phys), -1);
> +}
> +

...

> +
> +static int realm_tear_down_rtt_level(struct realm *realm, int level,
> +				     unsigned long start, unsigned long end)
> +{
> +	ssize_t map_size;
> +	unsigned long addr, next_addr;
> +
> +	if (WARN_ON(level > KVM_PGTABLE_LAST_LEVEL))
> +		return -EINVAL;
> +
> +	map_size = rmi_rtt_level_mapsize(level - 1);
> +
> +	for (addr = start; addr < end; addr = next_addr) {
> +		phys_addr_t rtt_granule;
> +		long ret;
> +		unsigned long align_addr = ALIGN(addr, map_size);

We assume that the start is aligned to the kvm_granule_size(level - 1) 
and thus this works ? Should we enforce this before the loop ?

> +
> +		next_addr = ALIGN(addr + 1, map_size);

So that we can be sure we have (align_addr != next_addr).

> +
> +		if (next_addr > end || align_addr != addr) {
> +			/*
> +			 * The target range is smaller than what this level
> +			 * covers, recurse deeper.
> +			 */
> +			ret = realm_tear_down_rtt_level(realm,
> +							level + 1,
> +							addr,
> +							min(next_addr, end));
> +			if (ret)
> +				return ret;
> +			continue;
> +		}
> +
> +		ret = realm_rtt_destroy(realm, addr, level,
> +					&rtt_granule, &next_addr);
> +		if (ret < 0)
> +			return ret;
> +
> +		switch (RMI_RESULT_STATUS(ret)) {
> +		case RMI_SUCCESS:
> +			free_rtt(rtt_granule);
> +			break;
> +		case RMI_ERROR_RTT:
> +			if (next_addr > addr) {
> +				/* Missing RTT, skip */
> +				break;
> +			}
> +			/*
> +			 * We tear down the RTT range for the full IPA
> +			 * space, after everything is unmapped. Also we
> +			 * descend down only if we cannot tear down a
> +			 * top level RTT. Thus RMM must be able to walk
> +			 * to the requested level. e.g., a block mapping
> +			 * exists at L1 or L2.
> +			 */
> +			if (WARN_ON(RMI_RESULT_DATA_LEVEL(ret) != level))
> +				return -EBUSY;
> +			if (WARN_ON(level == KVM_PGTABLE_LAST_LEVEL))
> +				return -EBUSY;
> +
> +			/*
> +			 * The table has active entries in it, recurse deeper
> +			 * and tear down the RTTs.
> +			 */
> +			next_addr = ALIGN(addr + 1, map_size);
> +			ret = realm_tear_down_rtt_level(realm,
> +							level + 1,
> +							addr,
> +							next_addr);
> +			if (ret)
> +				return ret;
> +			/*
> +			 * Now that the child RTTs are destroyed,
> +			 * retry at this level.
> +			 */
> +			next_addr = addr;
> +			break;
> +		default:
> +			WARN_ON(1);
> +			return -ENXIO;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int realm_tear_down_rtt_range(struct realm *realm,
> +				     unsigned long start, unsigned long end)
> +{
> +	/*
> +	 * Root level RTTs can only be destroyed after the RD is destroyed. So
> +	 * tear down everything below the root level
> +	 */
> +	return realm_tear_down_rtt_level(realm, get_start_level(realm) + 1,
> +					 start, end);
> +}
> +
> +static int realm_destroy_rtts(struct kvm *kvm)
> +{
> +	struct realm *realm = &kvm->arch.realm;
> +	unsigned int ia_bits = realm->ia_bits;
> +
> +	lockdep_assert_held(&kvm->arch.config_lock);

Should we assert that realm->stage2_unmapped == true here, since we
expect to not have any valid mappings ?

> +
> +	return realm_tear_down_rtt_range(realm, 0, (1UL << ia_bits));
> +}
> +
> +static void realm_unmap_stage2(struct kvm *kvm)
> +{
> +	struct realm *realm = &kvm->arch.realm;
> +
> +	lockdep_assert_held(&kvm->arch.config_lock);
> +
> +	if (realm->stage2_unmapped)
> +		return;
> +
> +	write_lock(&kvm->mmu_lock);
> +	kvm_stage2_unmap_range(&kvm->arch.mmu, 0,
> +			       BIT(realm->ia_bits - 1), true);


> +	write_unlock(&kvm->mmu_lock);

Do we need this write_lock() ? IIUC, the Realm is dying and this is
called from kvm_destroy_realm()/ kvm_free_stage2_pgd(). KVM assumes that
stage2_destroy_range() is called without kvm->mmu_lock held (along with
other relaxations).  The comment above stage2_destroy_range() says:


/*
  * Assume that @pgt is valid and unlinked from the KVM MMU to free the
  * page-table without taking the kvm_mmu_lock and without performing any
  * TLB invalidations.
  *
  * Also, the range of addresses can be large enough to cause need_resched
  * warnings, for instance on CONFIG_PREEMPT_NONE kernels. Hence, invoke
  * cond_resched() periodically to prevent hogging the CPU for a long time
  * and schedule something else, if required.
  */

So, I believe we should be safe here without the lock ?

Also, could we not call the realm unmap routine directly here, rather
than going through the generic helper to get back to what we want ?


Suzuki


> +
> +	realm->stage2_unmapped = true;
> +}
> +
> +int kvm_realm_teardown_stage2(struct kvm *kvm)
> +{
> +	lockdep_assert_held(&kvm->arch.config_lock);
> +
> +	realm_unmap_stage2(kvm);
> +	return 0;
> +}
> +
>   void kvm_destroy_realm(struct kvm *kvm)
>   {
>   	struct realm *realm = &kvm->arch.realm;
>   	size_t pgd_size = kvm_pgtable_stage2_pgd_size(kvm->arch.mmu.vtcr);
>   
> +	guard(mutex)(&kvm->arch.config_lock);
> +
>   	if (!kvm_realm_is_created(kvm)) {
>   		kfree(realm->sro);
>   		realm->sro = NULL;
> @@ -34,12 +211,24 @@ void kvm_destroy_realm(struct kvm *kvm)
>   
>   	kvm_set_realm_state(kvm, REALM_STATE_DYING);
>   
> +	/*
> +	 * REALM_DESTROY requires the realm to be non-live: all RECs must have
> +	 * been destroyed and the root RTTs must be empty. Unmap the IPA space
> +	 * and destroy any non-root RTTs before tearing down the RD. The root
> +	 * RTT pages are still owned by the RMM at this point, so keep the KVM
> +	 * pgtable alive until after REALM_DESTROY and undelegation.
> +	 */
> +	realm_unmap_stage2(kvm);
> +
>   	if (realm->rd) {
>   		phys_addr_t rd_phys = virt_to_phys(realm->rd);
>   
>   		if (WARN_ON(rmi_realm_terminate(rd_phys, realm->sro)))
>   			return;
>   
> +		if (WARN_ON(realm_destroy_rtts(kvm)))
> +			return;
> +
>   		if (WARN_ON(rmi_realm_destroy(rd_phys, realm->sro)))
>   			return;
>   		rmi_free_delegated_page(rd_phys);