Query guest_memfd for private/shared status if those guest_memfds
track private/shared status.

With this patch, Coco VMs can use guest_memfd for both shared and
private memory. If Coco VMs choose to use guest_memfd for both
shared and private memory, by creating guest_memfd with the
GUEST_MEMFD_FLAG_SUPPORT_SHARED flag, guest_memfd will be used to
provide the private/shared status of the memory, instead of
kvm->mem_attr_array.

Change-Id: I8f23d7995c12242aa4e09ccf5ec19360e9c9ed83
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 include/linux/kvm_host.h | 19 ++++++++++++-------
 virt/kvm/guest_memfd.c   | 22 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b317392453a5..91279e05e010 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2508,12 +2508,22 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu,
 }
 
 #ifdef CONFIG_KVM_GMEM_SHARED_MEM
+
 bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot);
+bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn);
+
 #else
+
 static inline bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot)
 {
 	return false;
 }
+
+static inline bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn)
+{
+	return false;
+}
+
 #endif /* CONFIG_KVM_GMEM_SHARED_MEM */
 
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
@@ -2544,13 +2554,8 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
 		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).
-		 */
-		return false;
-	}
+	if (kvm_slot_has_gmem(slot) && kvm_gmem_memslot_supports_shared(slot))
+		return kvm_gmem_is_private(slot, gfn);
 
 	return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
 }
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 6f6c4d298f8f..853e989bdcb2 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -865,6 +865,28 @@ bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot)
 }
 EXPORT_SYMBOL_GPL(kvm_gmem_memslot_supports_shared);
 
+bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn)
+{
+	struct inode *inode;
+	struct file *file;
+	pgoff_t index;
+	bool ret;
+
+	file = kvm_gmem_get_file(slot);
+	if (!file)
+		return false;
+
+	index = kvm_gmem_get_index(slot, gfn);
+	inode = file_inode(file);
+
+	filemap_invalidate_lock_shared(inode->i_mapping);
+	ret = kvm_gmem_shareability_get(inode, index) == SHAREABILITY_GUEST;
+	filemap_invalidate_unlock_shared(inode->i_mapping);
+
+	fput(file);
+	return ret;
+}
+
 #else
 #define kvm_gmem_mmap NULL
 #endif /* CONFIG_KVM_GMEM_SHARED_MEM */
-- 
2.49.0.1045.g170613ef41-goog

On Wed, May 14, 2025 at 04:41:45PM -0700, Ackerley Tng wrote:
> Query guest_memfd for private/shared status if those guest_memfds
> track private/shared status.
> 
> With this patch, Coco VMs can use guest_memfd for both shared and
> private memory. If Coco VMs choose to use guest_memfd for both
> shared and private memory, by creating guest_memfd with the
> GUEST_MEMFD_FLAG_SUPPORT_SHARED flag, guest_memfd will be used to
> provide the private/shared status of the memory, instead of
> kvm->mem_attr_array.
> 
> Change-Id: I8f23d7995c12242aa4e09ccf5ec19360e9c9ed83
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>  include/linux/kvm_host.h | 19 ++++++++++++-------
>  virt/kvm/guest_memfd.c   | 22 ++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index b317392453a5..91279e05e010 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2508,12 +2508,22 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu,
>  }
>  
>  #ifdef CONFIG_KVM_GMEM_SHARED_MEM
> +
>  bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot);
> +bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn);
> +
>  #else
> +
>  static inline bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot)
>  {
>  	return false;
>  }
> +
> +static inline bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn)
> +{
> +	return false;
> +}
> +
>  #endif /* CONFIG_KVM_GMEM_SHARED_MEM */
>  
>  #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> @@ -2544,13 +2554,8 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
>  		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).
> -		 */
> -		return false;
> -	}
> +	if (kvm_slot_has_gmem(slot) && kvm_gmem_memslot_supports_shared(slot))
> +		return kvm_gmem_is_private(slot, gfn);
When userspace gets an exit reason KVM_EXIT_MEMORY_FAULT, looks it needs to
update both KVM memory attribute and gmem shareability, via two separate ioctls?


>  	return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;

>  }
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 6f6c4d298f8f..853e989bdcb2 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -865,6 +865,28 @@ bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot)
>  }
>  EXPORT_SYMBOL_GPL(kvm_gmem_memslot_supports_shared);
>  
> +bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn)
> +{
> +	struct inode *inode;
> +	struct file *file;
> +	pgoff_t index;
> +	bool ret;
> +
> +	file = kvm_gmem_get_file(slot);
> +	if (!file)
> +		return false;
> +
> +	index = kvm_gmem_get_index(slot, gfn);
> +	inode = file_inode(file);
> +
> +	filemap_invalidate_lock_shared(inode->i_mapping);
> +	ret = kvm_gmem_shareability_get(inode, index) == SHAREABILITY_GUEST;
> +	filemap_invalidate_unlock_shared(inode->i_mapping);
> +
> +	fput(file);
> +	return ret;
> +}
> +
>  #else
>  #define kvm_gmem_mmap NULL
>  #endif /* CONFIG_KVM_GMEM_SHARED_MEM */
> -- 
> 2.49.0.1045.g170613ef41-goog
>

On 5/27/2025 11:55 AM, Yan Zhao wrote:
> On Wed, May 14, 2025 at 04:41:45PM -0700, Ackerley Tng wrote:
>> Query guest_memfd for private/shared status if those guest_memfds
>> track private/shared status.
>>
>> With this patch, Coco VMs can use guest_memfd for both shared and
>> private memory. If Coco VMs choose to use guest_memfd for both
>> shared and private memory, by creating guest_memfd with the
>> GUEST_MEMFD_FLAG_SUPPORT_SHARED flag, guest_memfd will be used to
>> provide the private/shared status of the memory, instead of
>> kvm->mem_attr_array.
>>
>> Change-Id: I8f23d7995c12242aa4e09ccf5ec19360e9c9ed83
>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>> ---
>>   include/linux/kvm_host.h | 19 ++++++++++++-------
>>   virt/kvm/guest_memfd.c   | 22 ++++++++++++++++++++++
>>   2 files changed, 34 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index b317392453a5..91279e05e010 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -2508,12 +2508,22 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu,
>>   }
>>   
>>   #ifdef CONFIG_KVM_GMEM_SHARED_MEM
>> +
>>   bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot);
>> +bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn);
>> +
>>   #else
>> +
>>   static inline bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot)
>>   {
>>   	return false;
>>   }
>> +
>> +static inline bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn)
>> +{
>> +	return false;
>> +}
>> +
>>   #endif /* CONFIG_KVM_GMEM_SHARED_MEM */
>>   
>>   #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
>> @@ -2544,13 +2554,8 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
>>   		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).
>> -		 */
>> -		return false;
>> -	}
>> +	if (kvm_slot_has_gmem(slot) && kvm_gmem_memslot_supports_shared(slot))
>> +		return kvm_gmem_is_private(slot, gfn);
> When userspace gets an exit reason KVM_EXIT_MEMORY_FAULT, looks it needs to
> update both KVM memory attribute and gmem shareability, via two separate ioctls?
IIUC, when userspace sets flag GUEST_MEMFD_FLAG_SUPPORT_SHARED to create the
guest_memfd, the check for memory attribute will go through the guest_memfd way,
the information in kvm->mem_attr_array will not be used.

So if userspace sets GUEST_MEMFD_FLAG_SUPPORT_SHARED, it uses
KVM_GMEM_CONVERT_SHARED/PRIVATE to update gmem shareability.
If userspace doesn't set GUEST_MEMFD_FLAG_SUPPORT_SHARED, it still uses
KVM_SET_MEMORY_ATTRIBUTES to update KVM memory attribute tracking.


>
>
>>   	return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
>>   }

On Wed, May 28, 2025 at 04:08:34PM +0800, Binbin Wu wrote:
> 
> 
> On 5/27/2025 11:55 AM, Yan Zhao wrote:
> > On Wed, May 14, 2025 at 04:41:45PM -0700, Ackerley Tng wrote:
> > > Query guest_memfd for private/shared status if those guest_memfds
> > > track private/shared status.
> > > 
> > > With this patch, Coco VMs can use guest_memfd for both shared and
> > > private memory. If Coco VMs choose to use guest_memfd for both
> > > shared and private memory, by creating guest_memfd with the
> > > GUEST_MEMFD_FLAG_SUPPORT_SHARED flag, guest_memfd will be used to
> > > provide the private/shared status of the memory, instead of
> > > kvm->mem_attr_array.
> > > 
> > > Change-Id: I8f23d7995c12242aa4e09ccf5ec19360e9c9ed83
> > > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> > > ---
> > >   include/linux/kvm_host.h | 19 ++++++++++++-------
> > >   virt/kvm/guest_memfd.c   | 22 ++++++++++++++++++++++
> > >   2 files changed, 34 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index b317392453a5..91279e05e010 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -2508,12 +2508,22 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu,
> > >   }
> > >   #ifdef CONFIG_KVM_GMEM_SHARED_MEM
> > > +
> > >   bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot);
> > > +bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn);
> > > +
> > >   #else
> > > +
> > >   static inline bool kvm_gmem_memslot_supports_shared(const struct kvm_memory_slot *slot)
> > >   {
> > >   	return false;
> > >   }
> > > +
> > > +static inline bool kvm_gmem_is_private(struct kvm_memory_slot *slot, gfn_t gfn)
> > > +{
> > > +	return false;
> > > +}
> > > +
> > >   #endif /* CONFIG_KVM_GMEM_SHARED_MEM */
> > >   #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> > > @@ -2544,13 +2554,8 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
> > >   		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).
> > > -		 */
> > > -		return false;
> > > -	}
> > > +	if (kvm_slot_has_gmem(slot) && kvm_gmem_memslot_supports_shared(slot))
> > > +		return kvm_gmem_is_private(slot, gfn);
> > When userspace gets an exit reason KVM_EXIT_MEMORY_FAULT, looks it needs to
> > update both KVM memory attribute and gmem shareability, via two separate ioctls?
> IIUC, when userspace sets flag GUEST_MEMFD_FLAG_SUPPORT_SHARED to create the
> guest_memfd, the check for memory attribute will go through the guest_memfd way,
> the information in kvm->mem_attr_array will not be used.
> 
> So if userspace sets GUEST_MEMFD_FLAG_SUPPORT_SHARED, it uses
> KVM_GMEM_CONVERT_SHARED/PRIVATE to update gmem shareability.
> If userspace doesn't set GUEST_MEMFD_FLAG_SUPPORT_SHARED, it still uses
> KVM_SET_MEMORY_ATTRIBUTES to update KVM memory attribute tracking.
Ok, so the user needs to search the memory region and guest_memfd to choose the
right ioctl.

For slots with guest_memfd of flag GUEST_MEMFD_FLAG_SUPPORT_SHARED, the
KVM_LPAGE_MIXED_FLAG bit in the lpage_info cannot reflect the truth and a false
value there may also prevent KVM from installing a huge page.

> > 
> > 
> > >   	return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
> > >   }
> 
>

On Wed, May 28, 2025, Yan Zhao wrote:
> On Wed, May 28, 2025 at 04:08:34PM +0800, Binbin Wu wrote:
> > On 5/27/2025 11:55 AM, Yan Zhao wrote:
> > > On Wed, May 14, 2025 at 04:41:45PM -0700, Ackerley Tng wrote:
> > > > @@ -2544,13 +2554,8 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
> > > >   		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).
> > > > -		 */
> > > > -		return false;
> > > > -	}
> > > > +	if (kvm_slot_has_gmem(slot) && kvm_gmem_memslot_supports_shared(slot))
> > > > +		return kvm_gmem_is_private(slot, gfn);
> > > When userspace gets an exit reason KVM_EXIT_MEMORY_FAULT, looks it needs to
> > > update both KVM memory attribute and gmem shareability, via two separate ioctls?
> > IIUC, when userspace sets flag GUEST_MEMFD_FLAG_SUPPORT_SHARED to create the
> > guest_memfd, the check for memory attribute will go through the guest_memfd way,
> > the information in kvm->mem_attr_array will not be used.
> > 
> > So if userspace sets GUEST_MEMFD_FLAG_SUPPORT_SHARED, it uses
> > KVM_GMEM_CONVERT_SHARED/PRIVATE to update gmem shareability.
> > If userspace doesn't set GUEST_MEMFD_FLAG_SUPPORT_SHARED, it still uses
> > KVM_SET_MEMORY_ATTRIBUTES to update KVM memory attribute tracking.
> Ok, so the user needs to search the memory region and guest_memfd to choose the
> right ioctl.

I don't see any reason to support "split" models like this.  Tracking PRIVATE in
two separate locations would be all kinds of crazy.  E.g. if a slot is temporarily
deleted, memory could unexpected toggle between private and shared.  As evidenced
by Yan's questions, the cognitive load on developers would also be very high.

Just make userspace choose between per-VM and per-gmem, and obviously allow
in-place conversions if and only if attributes are per-gmem.

I (or someone else?) suggested adding a capability to disable per-VM tracking, but
I don't see any reason to allow userspace to opt-out on a per-VM basis either.
The big selling point of in-place conversions is that it avoids having to double
provision some amount of guest memory.  Those types of changes go far beyond the
VMM.  So I have a very hard time imagining a use case where VMM A will want to
use per-VM attributes while VMM B will want per-gmem attributes.

Using a read-only module param will also simplify the internal code, as KVM will
be able to route memory attributes queries without need a pointer to the "struct
kvm".

In the future, we might have to swizzle things, e.g. if we want with per-VM RWX
attributes, but that's largely a future problem, and a module param also gives us
more flexibility anyways since they tend not to be considered rigid ABI in KVM.