Re: [PATCH v16 17/45] KVM: arm64: CCA: Tear down RTTs

On 11/08/2026 15:51, Suzuki K Poulose wrote:
> 
> 
> On 04/08/2026 13:16, Suzuki K Poulose wrote:
>> Hi there
>>
>> On 03/08/2026 23:29, Alper Gun wrote:
>>> On Mon, Aug 3, 2026 at 6:45 AM Steven Price <[email protected]>
>>> wrote:
>>>> [...]
>>>> +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);
>>>> +
>>>> +       realm->stage2_unmapped = true;
>>>> +}
>>>> +
>>>> +int kvm_realm_teardown_stage2(struct kvm *kvm)
>>>> +{
>>>> +       lockdep_assert_held(&kvm->arch.config_lock);
>>>> +
>>>> +       realm_unmap_stage2(kvm);
>>>> +       return realm_destroy_rtts(kvm);
>>>> +}
>>>
>>> Hi Steven,
>>>
>>> I noticed a warning in dmesg when destroying a realm:
>>>
>>>    [  158.533495] Call trace:
>>>    [  158.535927]  kvm_destroy_realm+0x208/0x218
>>>    [  158.540356]  kvm_arch_destroy_vm+0xc8/0xd8
>>>    [  158.544439]  kvm_put_kvm+0x324/0x4b0
>>>    [  158.548002]  kvm_vm_stats_release+0x1c/0x38
>>
>> Thanks for the report and the detailed analysis.
>>
>>>
>>> I traced this and found the following:
>>>
>>> At this point, calling realm_destroy_rtts() may fail destroying the RTTs
>>> because RECs are still active and the Realm is not terminated, but the
> 
> May be we should bail out early in the realm_map_ipa() based on the
> state of the Realm (i.e. state != ACTIVE). But looking at this
> I see that we serialize the state of the realm using the config_lock,
> which is a bit odd.

Bailing out early in realm_map_ipa() if the VM isn't ACTIVE makes sense,
I'm not sure if it's truly necessary but it's a good defensive check.

>>> walk finishes and still sets realm->rtts_destroyed = true.
>>>
>>> Later, kvm_destroy_realm() calls rmi_realm_terminate() and then calls
>>> realm_destroy_rtts(kvm) again. But because realm->rtts_destroyed is
>>> already
>>> set to true, it exits early without tearing down the RTTs.
>>>
>>> Consequently, rmi_realm_destroy() is invoked while the RTT tables still
>>> exist in RMM, which fails and triggers WARN_ON(rmi_realm_destroy).
>>>
>>> The fix I used was to remove realm_destroy_rtts() from
>>> kvm_realm_teardown_stage2():
>>> --- a/arch/arm64/kvm/rmi.c
>>> +++ b/arch/arm64/kvm/rmi.c
>>> @@ -434,7 +434,7 @@ int kvm_realm_teardown_stage2(struct kvm *kvm)
>>>    lockdep_assert_held(&kvm->arch.config_lock);
>>>
>>>    realm_unmap_stage2(kvm);
>>> - return realm_destroy_rtts(kvm);
>>> + return 0;
>>>   }
>>>
>>> I think we don't need RTT destruction here as it will be handled
>>> cleanly in
>>> kvm_destroy_realm() later.

Ack - yes this is now unnecessary. Thanks for tracking this down.

>> That sounds sensible to me, I will take a look and fix it locally for
>> the next version.
> 
> On a second look, I think the rtts_destroyed flag may not be saving us
> much. If the RTTs are destroyed, we should complete the second round
> fairly easily.

Agreed, I think dropping rtts_destroyed makes sense - it's not actually
helping and it's making the code confusing.

Thanks,
Steve

> Suzuki
> 
>>
>>
>> Thanks
>> Suzuki
>>
>>>
>>> Thanks,
>>> Alper
>>
>