Re: [RFC PATCH v2 02/13] KVM: guest_memfd: Support private/shared conversion of device memory

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Ackerley Tng <ackerleytng@google.com> writes:

> Ackerley Tng <ackerleytng@google.com> writes:
>
>> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> writes:
>>
>>>
>>> [...snip...]
>>>
>>> +static int kvm_gmem_device_convert(struct inode *inode, pgoff_t start,
>>> +		pgoff_t nr_pages, const struct guest_memfd_device_request *req,
>>> +		const struct kvm_memory_slot *slot)
>>> +{
>>> +	struct gmem_inode *gi = GMEM_I(inode);
>>> +	u64 attrs = req ? KVM_MEMORY_ATTRIBUTE_PRIVATE : 0;
>>> +	bool prepared = false;
>>> +	int ret;
>>> +
>>> +	MA_STATE(mas, &gi->attributes, start, start + nr_pages - 1);
>>> +
>>> +	filemap_invalidate_lock(inode->i_mapping);
>>> +	if (__kvm_gmem_range_has_attributes(inode, start, nr_pages, attrs)) {
>>> +		ret = 0;
>>> +		goto out;
>>> +	}
>>> +
>>> +	if (req && !__kvm_gmem_range_has_attributes(inode, start, nr_pages, 0)) {
>>> +		ret = -EEXIST;
>>> +		goto out;
>>> +	}
>>> +
>>> +	ret = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
>>> +	if (ret)
>>> +		goto out;
>>> +
>>> +	kvm_gmem_invalidate_start(inode, start, start + nr_pages, KVM_FILTER_SHARED);
>>> +	if (req && READ_ONCE(gi->device.kvm->vm_dead))
>>> +		ret = -EIO;
>>> +	else
>>> +		ret = gi->provider_ops->prepare_conversion(gi->provider, req);
>>> +	if (!ret) {
>>> +		prepared = true;
>>> +		if (req)
>>> +			ret = kvm_gmem_device_make_private(inode, slot, req, nr_pages);
>>
>> Is there a way to avoid making private from within the conversion flow?
>>
>> kvm_gmem_device_map() below already does it as part of vcpu_run.
>>
>> If we do the above, we'd be aligned with regular conversions, where
>> to-shared conversions eagerly update arch state (make_shared) but
>> to-private notifications to the firmware are done lazily.
>>
>
> I was a bit confused above. IIUC to handle
> RMI_EXIT_VDEV_VALIDATE_MAPPING, the conversion is not done from outside
> the kernel, instead it is done in the vcpu_run completion hook.
>

All of that has changed in the latest series.
kvm_complete_vdev_map_validate() now does the following. For the dependent
changes in other subsystems, you may want to fetch the Git branch
mentioned in the cover letter.

static void kvm_complete_vdev_map_validate(struct kvm_vcpu *vcpu)
{
	struct kvm *kvm = vcpu->kvm;
	struct realm_rec *rec = &vcpu->arch.rec;
	struct realm *realm = &kvm->arch.realm;
	struct kvm_memory_slot *slot;
	unsigned long start = rec->run->exit.dev_mem_base;
	unsigned long top = rec->run->exit.dev_mem_top;
	unsigned long rd = virt_to_phys(realm->rd);
	unsigned long rec_phys = virt_to_phys(rec->rec_page);
	int idx, ret;

	if (vcpu->run->cca_exit.response != KVM_ARM64_TIO_RESPONSE_APPROVE ||
	    top <= start || !PAGE_ALIGNED(start | top | rec->run->exit.dev_mem_pa) ||
	    !kvm_realm_is_private_address(realm, start) ||
	    !kvm_realm_is_private_address(realm, top - 1))
		goto reject;
	idx = srcu_read_lock(&kvm->srcu);
	slot = gfn_to_memslot(kvm, gpa_to_gfn(start));
	if (!slot || (slot->flags & KVM_MEMSLOT_INVALID) ||
	    !(slot->flags & KVM_MEMSLOT_GMEM_DEVICE) ||
	    gpa_to_gfn(top - 1) >= slot->base_gfn + slot->npages ||
	    slot->arch.device.vdev_id != rec->run->exit.vdev_id_1)
		goto out_reject;
	/* Userspace installs the mapping through guest_memfd before approval. */
	if (!kvm_gmem_range_has_attributes(slot, gpa_to_gfn(start),
					   gpa_to_gfn(top), KVM_MEMORY_ATTRIBUTE_PRIVATE))
		goto out_reject;
	while (start < top) {
		unsigned long next, rmi_ret;

		ret = rmi_rtt_dev_validate(rd, rec_phys, start, top, &next, &rmi_ret);
		if (ret || rmi_ret || next <= start || next > top)
			goto out_reject;
		start = next;
	}
	srcu_read_unlock(&kvm->srcu, idx);
	return;
out_reject:
	srcu_read_unlock(&kvm->srcu, idx);
reject:
	rec->run->enter.flags |= REC_ENTER_FLAG_DEV_MEM_RESPONSE;
}


-aneesh