Re: [PATCH v9 09/17] KVM: x86/mmu: Handle guest page faults for guest_memfd with shared memory

[David Hildenbrand <david@redhat.com>]

On 13.05.25 18:34, Fuad Tabba wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> For memslots backed by guest_memfd with shared mem support, the KVM MMU
> always faults-in pages from guest_memfd, and not from the userspace_addr.
> Towards this end, this patch also introduces a new guest_memfd flag,
> GUEST_MEMFD_FLAG_SUPPORT_SHARED, which indicates that the guest_memfd
> instance supports in-place shared memory.
> 
> This flag is only supported if the VM creating the guest_memfd instance
> belongs to certain types determined by architecture. Only non-CoCo VMs
> are permitted to use guest_memfd with shared mem, for now.
> 
> Function names have also been updated for accuracy -
> kvm_mem_is_private() returns true only when the current private/shared
> state (in the CoCo sense) of the memory is private, and returns false if
> the current state is shared explicitly or impicitly, e.g., belongs to a
> non-CoCo VM.
> 
> kvm_mmu_faultin_pfn_gmem() is updated to indicate that it can be used
> to fault in not just private memory, but more generally, from
> guest_memfd.
> 
> Co-developed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> Co-developed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---


[...]

> +
>   #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
>   static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
>   {
> @@ -2515,10 +2524,30 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
>   bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
>   					 struct kvm_gfn_range *range);
>   
> +/*
> + * Returns true if the given gfn's private/shared status (in the CoCo sense) is
> + * private.
> + *
> + * A return value of false indicates that the gfn is explicitly or implicity

s/implicity/implicitly/

> + * shared (i.e., non-CoCo VMs).
> + */
>   static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
>   {
> -	return IS_ENABLED(CONFIG_KVM_GMEM) &&
> -	       kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
> +	struct kvm_memory_slot *slot;
> +
> +	if (!IS_ENABLED(CONFIG_KVM_GMEM))
> +		return false;
> +
> +	slot = gfn_to_memslot(kvm, gfn);
> +	if (kvm_slot_has_gmem(slot) && kvm_gmem_memslot_supports_shared(slot)) {
> +		/*
> +		 * For now, memslots only support in-place shared memory if the
> +		 * host is allowed to mmap memory (i.e., non-Coco VMs).
> +		 */

Not accurate: there is no in-place conversion support in this series, 
because there is no such itnerface. So the reason is that all memory is 
shared for there VM types?

> +		return false;
> +	}
> +
> +	return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
>   }
>   #else
>   static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 2f499021df66..fe0245335c96 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -388,6 +388,23 @@ static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)
>   
>   	return 0;
>   }
> +
> +bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot)
> +{
> +	struct file *file;
> +	bool ret;
> +
> +	file = kvm_gmem_get_file((struct kvm_memory_slot *)slot);
> +	if (!file)
> +		return false;
> +
> +	ret = kvm_gmem_supports_shared(file_inode(file));
> +
> +	fput(file);
> +	return ret;

Would it make sense to cache that information in the memslot, to avoid 
the get/put?

We could simply cache when creating the memslot I guess.

As an alternative ... could we simple get/put when managing the memslot?

-- 
Cheers,

David / dhildenb