Re: [PATCH v16 27/45] KVM: arm64: CCA: Allow populating initial contents

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

On 06/08/2026 23:43, Ackerley Tng wrote:
> Steven Price <steven.price@arm.com> writes:
> 
>> The VMM needs to populate the realm with some data before starting (e.g.
>> a kernel and initrd). This is measured by the RMM and used as part of
>> the attestation later on.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>

...

>> +static int realm_data_map_init(struct kvm *kvm, unsigned long ipa,
>> +			       kvm_pfn_t dst_pfn, kvm_pfn_t src_pfn,
>> +			       unsigned long flags)
>> +{
>> +	struct realm *realm = &kvm->arch.realm;
>> +	phys_addr_t rd = virt_to_phys(realm->rd);
>> +	phys_addr_t dst_phys, src_phys;
>> +	long ret;
>> +
>> +	lockdep_assert_held(&kvm->slots_lock);
>> +	lockdep_assert_held(&kvm->arch.config_lock);
>> +
>> +	dst_phys = __pfn_to_phys(dst_pfn);
>> +	src_phys = __pfn_to_phys(src_pfn);
>> +
>> +	if (rmi_delegate_page(dst_phys))
>> +		return -ENXIO;
>> +
>> +retry:
>> +	ret = rmi_rtt_data_map_init(rd, dst_phys, ipa, src_phys, flags);
>> +	if (ret >= 0 && RMI_RETURN_STATUS(ret) == RMI_ERROR_RTT) {
>> +		/* Create missing RTTs and retry */
>> +		int level = RMI_RETURN_INDEX(ret);
>> +
>> +		KVM_BUG_ON(level >= KVM_PGTABLE_LAST_LEVEL, kvm);
>> +
>> +		ret = realm_create_rtt_levels(realm, ipa, level,
>> +					      level + 1, NULL);
>> +		if (!ret)
>> +			goto retry;
>> +	}
>> +
>> +	if (ret && WARN_ON(rmi_undelegate_page(dst_phys))) {
>> +		/* Leak the page if the undelegate fails */
>> +		get_page(pfn_to_page(dst_pfn));
> 
> Is there some way to avoid taking a reference on the page? This would
> interfere with conversions. There was a similar discussion for TDX as
> well [1].

Unfortunately, no. The page was transferred to the Realm world (with
rmi_delegate_page() above the retry: ). If we fail to bring it back,
that page is still in the Realm PAS and any access to it by the normal
world would result in a GPF and eventually bring down the system
if it happens from the kernel.

We don't expect that undelegate to fail. The granule_delegate()
should fail if the page was already in use by the RMM for some
purpose (e.g., already mapped at the IPA, because VMM issued
DATA_MAP_INIT twice. Even with the relaxation coming in the
RMM, we will mandate that the "populate" cases will request
strict conditions for granule delegate).

Please note that this is NOT the "unmap" failure, but it is
"Bring the page back to the NS world" failure that causes
the WARN_ON and the leaking.

> 
> TDX originally incremented folio refounts for these:
> 
> + when mapping folios into the Secure EPTs. This one was easier to agree
>    to remove, since TDX can trust guest_memfd to keep pages around on
>    behalf of the guest.
> + To indicate unmapping failure (IIUC this is the same situation as
>    above). This interferes with conversions.
>      + An alternative discussed was to mark these pages as HWPOISON, but
>        that was eventually rejected as adding unnecessary complexity to
>        make TDX special for code paths that only occur on kernel
>        bugs. (In TDX's case the unmap failures would probably only be for
>        kernel bugs.)
>      + I later worked a bit more on memory failure for guest_memfd
>        HugeTLB and found that because we will need to restructure huge
>        pages for conversions, using the HWPOISON flag would be hard to
>        handle. For TDX since the conclusion was not to use a HWPOISON
>        flag to indicate unmap failures anyway, this turned out to be a
>        non-issue. Nobody wanted to use the HWPOISON flag. (I hope you
>        won't need to either)
> 
> So for TDX, on an unmap failure we do a KVM_BUG_ON() and mark the VM as
> dead, and do nothing about the page, it still gets returned to the
> system as if nothing happened.
> 
> Here's my understanding of why this is okay for TDX (Rick and Yan, could
> you please help me here):
> 
> + For unmap failures, the page remains in TDX's Physical Address
>    Metadata Table (PAMT), and the page is still assigned to some TD.
> + If the page was assigned to some other TD, it would be blocked, since
>    the PAMT shows it as already assigned.
> + If the page was used by something completely unrelated to TDX, then in
>    the TDX model the host is free to write and read pages. Nothing goes
>    bad until the TD tries to use that same page, but that TD would never
>    use the page again, that TD is already dead and the HKID for the TD
>    was leaked.

This is not true for CCA. Like I said above, touching the page in Realm
PAS is going to be disastrous for the Host.


> 
> [1] https://lore.kernel.org/all/diqz34bolnta.fsf@ackerleytng-ctop.c.googlers.com/
> 
>> +	}
>> +
>> +	return ret <= 0 ? ret : -ENXIO;
>> +}
>> +
>> +static int populate_region_cb(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>> +			      struct page *src_page, void *opaque)
>> +{
>> +	unsigned long data_flags = *(unsigned long *)opaque;
>> +	phys_addr_t ipa = gfn_to_gpa(gfn);
>> +
>> +	return realm_data_map_init(kvm, ipa, pfn, page_to_pfn(src_page),
>> +				   data_flags);
>> +}
>> +
>> +static long populate_region(struct kvm *kvm,
>> +			    gfn_t base_gfn,
>> +			    unsigned long pages,
>> +			    u64 uaddr,
>> +			    unsigned long data_flags)
>> +{
>> +	long ret = 0;
>> +
>> +	lockdep_assert_held(&kvm->slots_lock);
>> +	lockdep_assert_held(&kvm->arch.config_lock);
>> +
>> +	if (!uaddr)
>> +		return -EINVAL;
>> +
> 
> Why not check for !uaddr together with the other checks in
> kvm_arm_rmi_populate?

Yep, we could move it there.

> 
> Also would it be okay to inline populate_region into
> kvm_arm_rmi_populate below?

Ack.

Suzuki