[RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[Fuad Tabba]

The purpose of this series is to serve as a potential base for
restricted mmap() support for guest_memfd [1]. It would allow
experimentation with what that support would be like, in the safe
environment of a new VM type used for testing.

This series adds a new VM type for arm64,
KVM_VM_TYPE_ARM_SW_PROTECTED, analogous to the x86
KVM_X86_SW_PROTECTED_VM. This type is to serve as a development
and testing vehicle for Confidential (CoCo) VMs.

Similar to the x86 type, this is currently only for development
and testing. It's not meant to be used for "real" VMs, and
especially not in production. The behavior and effective ABI for
software-protected VMs is unstable.

This series enables mmap() support for guest_memfd specifically
for the new software-protected VM type, only when explicitly
enabled in the config.

The series is based on Linux 6.13-rc7 and much of the code within
is a subset of the latest series I sent [1], with the addition of
the new software protected vm type.

To test this series, there's a kvmtool branch with support for
guest_memfd for arm64 and the new runtime options of
--guest_memfd and --sw_protected, which marks the VM as software
protected [2]. I plan on upstreaming this kvmtool branch after
more testing and tidying up.

Please let me know if this series is useful as a stand-alone
series, or if I should merge it with the other guest_memfd for
future respins [1].

Cheers,
/fuad

[1] https://lore.kernel.org/all/20250117163001.2326672-1-tabba@google.com/
[2] https://android-kvm.googlesource.com/kvmtool/+/refs/heads/tabba/guestmem-6.13

Fuad Tabba (9):
  KVM: guest_memfd: Allow host to mmap guest_memfd() pages
  KVM: guest_memfd: Add guest_memfd support to
    kvm_(read|/write)_guest_page()
  KVM: guest_memfd: Add KVM capability to check if guest_memfd is host
    mappable
  KVM: arm64: Skip VMA checks for slots without userspace address
  KVM: arm64: Refactor user_mem_abort() calculation of force_pte
  KVM: arm64: Handle guest_memfd()-backed guest page faults
  KVM: arm64: Introduce KVM_VM_TYPE_ARM_SW_PROTECTED machine type
  KVM: guest_memfd: selftests: guest_memfd mmap() test when mapping is
    allowed
  KVM: arm64: Enable mapping guest_memfd in arm64

 Documentation/virt/kvm/api.rst                |   5 +
 arch/arm64/include/asm/kvm_host.h             |  10 ++
 arch/arm64/kvm/Kconfig                        |   1 +
 arch/arm64/kvm/arm.c                          |   5 +
 arch/arm64/kvm/mmu.c                          | 101 ++++++++++----
 arch/x86/include/asm/kvm_host.h               |   2 +
 include/linux/kvm_host.h                      |  16 +++
 include/uapi/linux/kvm.h                      |   7 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../testing/selftests/kvm/guest_memfd_test.c  |  60 ++++++++-
 tools/testing/selftests/kvm/lib/kvm_util.c    |   3 +-
 virt/kvm/Kconfig                              |   4 +
 virt/kvm/guest_memfd.c                        |  71 ++++++++++
 virt/kvm/kvm_main.c                           | 127 ++++++++++++++----
 14 files changed, 353 insertions(+), 60 deletions(-)


base-commit: 5bc55a333a2f7316b58edc7573e8e893f7acb532
-- 
2.48.0.rc2.279.g1de40edade-goog

[RFC PATCH v1 1/9] KVM: guest_memfd: Allow host to mmap guest_memfd() pages

[Fuad Tabba]

Add support for mmap() and fault() for guest_memfd in the host
for VMs that support in place conversion between shared and
private. To that end, this patch adds the ability to check
whether the architecture has that support, and only allows mmap()
if that's the case.

Additionally, this is gated with a new configuration option,
CONFIG_KVM_GMEM_MAPPABLE.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/x86/include/asm/kvm_host.h |  2 +
 include/linux/kvm_host.h        | 11 +++++
 virt/kvm/Kconfig                |  4 ++
 virt/kvm/guest_memfd.c          | 71 +++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e159e44a6a1b..c0e149bc1d79 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2206,6 +2206,8 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
 #define kvm_arch_has_private_mem(kvm) false
 #endif
 
+#define kvm_arch_private_mem_inplace(kvm) false
+
 #define kvm_arch_has_readonly_mem(kvm) (!(kvm)->arch.has_protected_state)
 
 static inline u16 kvm_read_ldt(void)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 401439bb21e3..ebca0ab4c5e2 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -717,6 +717,17 @@ static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
 }
 #endif
 
+/*
+ * Arch code must define kvm_arch_private_mem_inplace if support for private
+ * memory is enabled it supports in-place conversion between shared and private.
+ */
+#if !defined(kvm_arch_private_mem_inplace) && !IS_ENABLED(CONFIG_KVM_PRIVATE_MEM)
+static inline bool kvm_arch_private_mem_inplace(struct kvm *kvm)
+{
+	return false;
+}
+#endif
+
 #ifndef kvm_arch_has_readonly_mem
 static inline bool kvm_arch_has_readonly_mem(struct kvm *kvm)
 {
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 54e959e7d68f..59400fd8f539 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -124,3 +124,7 @@ config HAVE_KVM_ARCH_GMEM_PREPARE
 config HAVE_KVM_ARCH_GMEM_INVALIDATE
        bool
        depends on KVM_PRIVATE_MEM
+
+config KVM_GMEM_MAPPABLE
+       select KVM_PRIVATE_MEM
+       bool
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 47a9f68f7b24..9ee162bf6bde 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -307,7 +307,78 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn)
 	return gfn - slot->base_gfn + slot->gmem.pgoff;
 }
 
+#ifdef CONFIG_KVM_GMEM_MAPPABLE
+static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
+{
+	struct inode *inode = file_inode(vmf->vma->vm_file);
+	struct folio *folio;
+	vm_fault_t ret = VM_FAULT_LOCKED;
+
+	filemap_invalidate_lock_shared(inode->i_mapping);
+
+	folio = kvm_gmem_get_folio(inode, vmf->pgoff);
+	if (IS_ERR(folio)) {
+		ret = VM_FAULT_SIGBUS;
+		goto out_filemap;
+	}
+
+	if (folio_test_hwpoison(folio)) {
+		ret = VM_FAULT_HWPOISON;
+		goto out_folio;
+	}
+
+	if (!folio_test_uptodate(folio)) {
+		unsigned long nr_pages = folio_nr_pages(folio);
+		unsigned long i;
+
+		for (i = 0; i < nr_pages; i++)
+			clear_highpage(folio_page(folio, i));
+
+		folio_mark_uptodate(folio);
+	}
+
+	vmf->page = folio_file_page(folio, vmf->pgoff);
+
+out_folio:
+	if (ret != VM_FAULT_LOCKED) {
+		folio_unlock(folio);
+		folio_put(folio);
+	}
+
+out_filemap:
+	filemap_invalidate_unlock_shared(inode->i_mapping);
+
+	return ret;
+}
+
+static const struct vm_operations_struct kvm_gmem_vm_ops = {
+	.fault = kvm_gmem_fault,
+};
+
+static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct kvm_gmem *gmem = file->private_data;
+
+	if (!kvm_arch_private_mem_inplace(gmem->kvm))
+		return -ENODEV;
+
+	if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) !=
+	    (VM_SHARED | VM_MAYSHARE)) {
+		return -EINVAL;
+	}
+
+	file_accessed(file);
+	vm_flags_set(vma, VM_DONTDUMP);
+	vma->vm_ops = &kvm_gmem_vm_ops;
+
+	return 0;
+}
+#else
+#define kvm_gmem_mmap NULL
+#endif /* CONFIG_KVM_GMEM_MAPPABLE */
+
 static struct file_operations kvm_gmem_fops = {
+	.mmap		= kvm_gmem_mmap,
 	.open		= generic_file_open,
 	.release	= kvm_gmem_release,
 	.fallocate	= kvm_gmem_fallocate,
-- 
2.48.0.rc2.279.g1de40edade-goog

Re: [RFC PATCH v1 1/9] KVM: guest_memfd: Allow host to mmap guest_memfd() pages

[David Hildenbrand]

On 22.01.25 16:27, Fuad Tabba wrote:
> Add support for mmap() and fault() for guest_memfd in the host
> for VMs that support in place conversion between shared and
> private.
 > To that end, this patch adds the ability to check> whether the 
architecture has that support, and only allows mmap()
> if that's the case.
 > > Additionally, this is gated with a new configuration option,
> CONFIG_KVM_GMEM_MAPPABLE.
 > > Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>   arch/x86/include/asm/kvm_host.h |  2 +
>   include/linux/kvm_host.h        | 11 +++++
>   virt/kvm/Kconfig                |  4 ++
>   virt/kvm/guest_memfd.c          | 71 +++++++++++++++++++++++++++++++++
>   4 files changed, 88 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e159e44a6a1b..c0e149bc1d79 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -2206,6 +2206,8 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
>   #define kvm_arch_has_private_mem(kvm) false
>   #endif
>   
> +#define kvm_arch_private_mem_inplace(kvm) false
> +
>   #define kvm_arch_has_readonly_mem(kvm) (!(kvm)->arch.has_protected_state)
>   
>   static inline u16 kvm_read_ldt(void)
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 401439bb21e3..ebca0ab4c5e2 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -717,6 +717,17 @@ static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
>   }
>   #endif
>   
> +/*
> + * Arch code must define kvm_arch_private_mem_inplace if support for private
> + * memory is enabled it supports in-place conversion between shared and private.
> + */
> +#if !defined(kvm_arch_private_mem_inplace) && !IS_ENABLED(CONFIG_KVM_PRIVATE_MEM)
> +static inline bool kvm_arch_private_mem_inplace(struct kvm *kvm)
I assume right now this would rather indicate "support for shared 
(mappable) memory in guest_memfd".

Maybe there is a better way to express that :)

kvm_arch_gmem_supports_shared_mem ?

The in-place conversion is (at least to me) implied with support for 
shared memory.

> +
>   #ifndef kvm_arch_has_readonly_mem
>   static inline bool kvm_arch_has_readonly_mem(struct kvm *kvm)
>   {
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index 54e959e7d68f..59400fd8f539 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -124,3 +124,7 @@ config HAVE_KVM_ARCH_GMEM_PREPARE
>   config HAVE_KVM_ARCH_GMEM_INVALIDATE
>          bool
>          depends on KVM_PRIVATE_MEM
> +
> +config KVM_GMEM_MAPPABLE
> +       select KVM_PRIVATE_MEM

Easier to grasp might be:

KVM_GMEM_MAPPABLE -> KVM_GMEM_SHARED_MEM

Support for "shared" memory kind of imply mmap support (otherwise, how 
to make use of it :) ),


(KVM_PRIVATE_MEM -> KVM_GMEM might also make sense, but it's a different 
discussion)

> +       bool
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 47a9f68f7b24..9ee162bf6bde 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -307,7 +307,78 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn)
>   	return gfn - slot->base_gfn + slot->gmem.pgoff;
>   }
>   
> +#ifdef CONFIG_KVM_GMEM_MAPPABLE
> +static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
> +{
> +	struct inode *inode = file_inode(vmf->vma->vm_file);
> +	struct folio *folio;
> +	vm_fault_t ret = VM_FAULT_LOCKED;
> +
> +	filemap_invalidate_lock_shared(inode->i_mapping);
> +
> +	folio = kvm_gmem_get_folio(inode, vmf->pgoff);


Would the idea be later that kvm_gmem_get_folio() would fail on private 
memory, or do you envision other checks in this code here in the future?

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 1/9] KVM: guest_memfd: Allow host to mmap guest_memfd() pages

[Fuad Tabba]

Hi David,

On Wed, 22 Jan 2025 at 22:07, David Hildenbrand <david@redhat.com> wrote:
>
> On 22.01.25 16:27, Fuad Tabba wrote:
> > Add support for mmap() and fault() for guest_memfd in the host
> > for VMs that support in place conversion between shared and
> > private.
>  > To that end, this patch adds the ability to check> whether the
> architecture has that support, and only allows mmap()
> > if that's the case.
>  > > Additionally, this is gated with a new configuration option,
> > CONFIG_KVM_GMEM_MAPPABLE.
>  > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >   arch/x86/include/asm/kvm_host.h |  2 +
> >   include/linux/kvm_host.h        | 11 +++++
> >   virt/kvm/Kconfig                |  4 ++
> >   virt/kvm/guest_memfd.c          | 71 +++++++++++++++++++++++++++++++++
> >   4 files changed, 88 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index e159e44a6a1b..c0e149bc1d79 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -2206,6 +2206,8 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
> >   #define kvm_arch_has_private_mem(kvm) false
> >   #endif
> >
> > +#define kvm_arch_private_mem_inplace(kvm) false
> > +
> >   #define kvm_arch_has_readonly_mem(kvm) (!(kvm)->arch.has_protected_state)
> >
> >   static inline u16 kvm_read_ldt(void)
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 401439bb21e3..ebca0ab4c5e2 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -717,6 +717,17 @@ static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
> >   }
> >   #endif
> >
> > +/*
> > + * Arch code must define kvm_arch_private_mem_inplace if support for private
> > + * memory is enabled it supports in-place conversion between shared and private.
> > + */
> > +#if !defined(kvm_arch_private_mem_inplace) && !IS_ENABLED(CONFIG_KVM_PRIVATE_MEM)
> > +static inline bool kvm_arch_private_mem_inplace(struct kvm *kvm)
> I assume right now this would rather indicate "support for shared
> (mappable) memory in guest_memfd".
>
> Maybe there is a better way to express that :)
>
> kvm_arch_gmem_supports_shared_mem ?
>
> The in-place conversion is (at least to me) implied with support for
> shared memory.

Sounds better than what I had.

> > +
> >   #ifndef kvm_arch_has_readonly_mem
> >   static inline bool kvm_arch_has_readonly_mem(struct kvm *kvm)
> >   {
> > diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> > index 54e959e7d68f..59400fd8f539 100644
> > --- a/virt/kvm/Kconfig
> > +++ b/virt/kvm/Kconfig
> > @@ -124,3 +124,7 @@ config HAVE_KVM_ARCH_GMEM_PREPARE
> >   config HAVE_KVM_ARCH_GMEM_INVALIDATE
> >          bool
> >          depends on KVM_PRIVATE_MEM
> > +
> > +config KVM_GMEM_MAPPABLE
> > +       select KVM_PRIVATE_MEM
>
> Easier to grasp might be:
>
> KVM_GMEM_MAPPABLE -> KVM_GMEM_SHARED_MEM
>
> Support for "shared" memory kind of imply mmap support (otherwise, how
> to make use of it :) ),

Like the one above, this makes more sense.


>
> (KVM_PRIVATE_MEM -> KVM_GMEM might also make sense, but it's a different
> discussion)
>
> > +       bool
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 47a9f68f7b24..9ee162bf6bde 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -307,7 +307,78 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn)
> >       return gfn - slot->base_gfn + slot->gmem.pgoff;
> >   }
> >
> > +#ifdef CONFIG_KVM_GMEM_MAPPABLE
> > +static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
> > +{
> > +     struct inode *inode = file_inode(vmf->vma->vm_file);
> > +     struct folio *folio;
> > +     vm_fault_t ret = VM_FAULT_LOCKED;
> > +
> > +     filemap_invalidate_lock_shared(inode->i_mapping);
> > +
> > +     folio = kvm_gmem_get_folio(inode, vmf->pgoff);
>
>
> Would the idea be later that kvm_gmem_get_folio() would fail on private
> memory, or do you envision other checks in this code here in the future?

There would be other checks in the future, the idea is that they would
be the ones in:
https://lore.kernel.org/all/20250117163001.2326672-8-tabba@google.com/

What this patch series has, but the other one doesn't, is checks on
mmap to ensure whether the VM type supports that operation in
principle or not.

Thanks,
/fuad

> --
> Cheers,
>
> David / dhildenb
>

Re: [RFC PATCH v1 1/9] KVM: guest_memfd: Allow host to mmap guest_memfd() pages

[David Hildenbrand]

>>> +       bool
>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>> index 47a9f68f7b24..9ee162bf6bde 100644
>>> --- a/virt/kvm/guest_memfd.c
>>> +++ b/virt/kvm/guest_memfd.c
>>> @@ -307,7 +307,78 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn)
>>>        return gfn - slot->base_gfn + slot->gmem.pgoff;
>>>    }
>>>
>>> +#ifdef CONFIG_KVM_GMEM_MAPPABLE
>>> +static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
>>> +{
>>> +     struct inode *inode = file_inode(vmf->vma->vm_file);
>>> +     struct folio *folio;
>>> +     vm_fault_t ret = VM_FAULT_LOCKED;
>>> +
>>> +     filemap_invalidate_lock_shared(inode->i_mapping);
>>> +
>>> +     folio = kvm_gmem_get_folio(inode, vmf->pgoff);
>>
>>
>> Would the idea be later that kvm_gmem_get_folio() would fail on private
>> memory, or do you envision other checks in this code here in the future?
> 
> There would be other checks in the future, the idea is that they would
> be the ones in:
> https://lore.kernel.org/all/20250117163001.2326672-8-tabba@google.com/
> 

Thanks, so I wonder if this patch should just add necessary callback(s) 
as well, to make this patch look like it adds most of the infrastructure 
on the mmap level.

kvm_gmem_is_shared() or sth like that, documenting that it must be 
called after kvm_gmem_get_folio() -- with a raised folio reference / 
folio lock.

Alternatively, provide a

	kvm_gmem_get_shared_folio()

that abstracts that operation.

We could also for now ensure that we really only get small folios back, 
and even get rid of the clearing loop.


The "WARN_ON_ONCE(folio_test_guestmem(folio)" would be added separately.

In the context of this series, the callback would be a nop and always 
say "yes".

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 1/9] KVM: guest_memfd: Allow host to mmap guest_memfd() pages

[Fuad Tabba]

On Thu, 23 Jan 2025 at 10:28, David Hildenbrand <david@redhat.com> wrote:
>
> >>> +       bool
> >>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> >>> index 47a9f68f7b24..9ee162bf6bde 100644
> >>> --- a/virt/kvm/guest_memfd.c
> >>> +++ b/virt/kvm/guest_memfd.c
> >>> @@ -307,7 +307,78 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn)
> >>>        return gfn - slot->base_gfn + slot->gmem.pgoff;
> >>>    }
> >>>
> >>> +#ifdef CONFIG_KVM_GMEM_MAPPABLE
> >>> +static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
> >>> +{
> >>> +     struct inode *inode = file_inode(vmf->vma->vm_file);
> >>> +     struct folio *folio;
> >>> +     vm_fault_t ret = VM_FAULT_LOCKED;
> >>> +
> >>> +     filemap_invalidate_lock_shared(inode->i_mapping);
> >>> +
> >>> +     folio = kvm_gmem_get_folio(inode, vmf->pgoff);
> >>
> >>
> >> Would the idea be later that kvm_gmem_get_folio() would fail on private
> >> memory, or do you envision other checks in this code here in the future?
> >
> > There would be other checks in the future, the idea is that they would
> > be the ones in:
> > https://lore.kernel.org/all/20250117163001.2326672-8-tabba@google.com/
> >
>
> Thanks, so I wonder if this patch should just add necessary callback(s)
> as well, to make this patch look like it adds most of the infrastructure
> on the mmap level.
>
> kvm_gmem_is_shared() or sth like that, documenting that it must be
> called after kvm_gmem_get_folio() -- with a raised folio reference /
> folio lock.
>
> Alternatively, provide a
>
>         kvm_gmem_get_shared_folio()
>
> that abstracts that operation.
>
> We could also for now ensure that we really only get small folios back,
> and even get rid of the clearing loop.
>
>
> The "WARN_ON_ONCE(folio_test_guestmem(folio)" would be added separately.
>
> In the context of this series, the callback would be a nop and always
> say "yes".

I agree, especially if this patch series were to serve as a prelude to
the other one that adds restricted mmap() support.

Cheers,
/fuad

> --
> Cheers,
>
> David / dhildenb
>

[RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Fuad Tabba]

Make kvm_(read|/write)_guest_page() capable of accessing guest
memory for slots that don't have a userspace address, but only if
the memory is mappable, which also indicates that it is
accessible by the host.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 virt/kvm/kvm_main.c | 118 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 99 insertions(+), 19 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index de2c11dae231..ad9802012a3f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3094,21 +3094,93 @@ static int next_segment(unsigned long len, int offset)
 		return len;
 }
 
-/* Copy @len bytes from guest memory at '(@gfn * PAGE_SIZE) + @offset' to @data */
-static int __kvm_read_guest_page(struct kvm_memory_slot *slot, gfn_t gfn,
-				 void *data, int offset, int len)
+#ifdef CONFIG_KVM_GMEM_MAPPABLE
+static int __kvm_read_guest_memfd_page(struct kvm *kvm,
+				       struct kvm_memory_slot *slot,
+				       gfn_t gfn, void *data, int offset,
+				       int len)
+{
+	struct page *page;
+	u64 pfn;
+	int r;
+
+	/*
+	 * Holds the folio lock until after checking whether it can be faulted
+	 * in, to avoid races with paths that change a folio's mappability.
+	 */
+	r = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, NULL);
+	if (r)
+		return r;
+
+	memcpy(data, page_address(page) + offset, len);
+	kvm_release_page_clean(page);
+
+	return r;
+}
+
+static int __kvm_write_guest_memfd_page(struct kvm *kvm,
+					struct kvm_memory_slot *slot,
+					gfn_t gfn, const void *data,
+					int offset, int len)
 {
+	struct page *page;
+	u64 pfn;
 	int r;
+
+	/*
+	 * Holds the folio lock until after checking whether it can be faulted
+	 * in, to avoid races with paths that change a folio's mappability.
+	 */
+	r = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, NULL);
+	if (r)
+		return r;
+
+	memcpy(page_address(page) + offset, data, len);
+	kvm_release_page_dirty(page);
+
+	return r;
+}
+#else
+static int __kvm_read_guest_memfd_page(struct kvm *kvm,
+				       struct kvm_memory_slot *slot,
+				       gfn_t gfn, void *data, int offset,
+				       int len)
+{
+	WARN_ON_ONCE(1);
+	return -EIO;
+}
+
+static int __kvm_write_guest_memfd_page(struct kvm *kvm,
+					struct kvm_memory_slot *slot,
+					gfn_t gfn, const void *data,
+					int offset, int len)
+{
+	WARN_ON_ONCE(1);
+	return -EIO;
+}
+#endif /* CONFIG_KVM_GMEM_MAPPABLE */
+
+/* Copy @len bytes from guest memory at '(@gfn * PAGE_SIZE) + @offset' to @data */
+
+static int __kvm_read_guest_page(struct kvm *kvm, struct kvm_memory_slot *slot,
+				 gfn_t gfn, void *data, int offset, int len)
+{
 	unsigned long addr;
 
 	if (WARN_ON_ONCE(offset + len > PAGE_SIZE))
 		return -EFAULT;
 
+	if (kvm_arch_private_mem_inplace(kvm) &&
+	    kvm_slot_can_be_private(slot) &&
+	    !slot->userspace_addr) {
+		return __kvm_read_guest_memfd_page(kvm, slot, gfn, data,
+						   offset, len);
+	}
+
 	addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
 	if (kvm_is_error_hva(addr))
 		return -EFAULT;
-	r = __copy_from_user(data, (void __user *)addr + offset, len);
-	if (r)
+	if (__copy_from_user(data, (void __user *)addr + offset, len))
 		return -EFAULT;
 	return 0;
 }
@@ -3118,7 +3190,7 @@ int kvm_read_guest_page(struct kvm *kvm, gfn_t gfn, void *data, int offset,
 {
 	struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
 
-	return __kvm_read_guest_page(slot, gfn, data, offset, len);
+	return __kvm_read_guest_page(kvm, slot, gfn, data, offset, len);
 }
 EXPORT_SYMBOL_GPL(kvm_read_guest_page);
 
@@ -3127,7 +3199,7 @@ int kvm_vcpu_read_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn, void *data,
 {
 	struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
 
-	return __kvm_read_guest_page(slot, gfn, data, offset, len);
+	return __kvm_read_guest_page(vcpu->kvm, slot, gfn, data, offset, len);
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_read_guest_page);
 
@@ -3204,22 +3276,30 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_read_guest_atomic);
 
 /* Copy @len bytes from @data into guest memory at '(@gfn * PAGE_SIZE) + @offset' */
 static int __kvm_write_guest_page(struct kvm *kvm,
-				  struct kvm_memory_slot *memslot, gfn_t gfn,
-			          const void *data, int offset, int len)
+				  struct kvm_memory_slot *slot, gfn_t gfn,
+				  const void *data, int offset, int len)
 {
-	int r;
-	unsigned long addr;
-
 	if (WARN_ON_ONCE(offset + len > PAGE_SIZE))
 		return -EFAULT;
 
-	addr = gfn_to_hva_memslot(memslot, gfn);
-	if (kvm_is_error_hva(addr))
-		return -EFAULT;
-	r = __copy_to_user((void __user *)addr + offset, data, len);
-	if (r)
-		return -EFAULT;
-	mark_page_dirty_in_slot(kvm, memslot, gfn);
+	if (kvm_arch_private_mem_inplace(kvm) &&
+	    kvm_slot_can_be_private(slot) &&
+	    !slot->userspace_addr) {
+		int r = __kvm_write_guest_memfd_page(kvm, slot, gfn, data,
+						     offset, len);
+
+		if (r)
+			return r;
+	} else {
+		unsigned long addr = gfn_to_hva_memslot(slot, gfn);
+
+		if (kvm_is_error_hva(addr))
+			return -EFAULT;
+		if (__copy_to_user((void __user *)addr + offset, data, len))
+			return -EFAULT;
+	}
+
+	mark_page_dirty_in_slot(kvm, slot, gfn);
 	return 0;
 }
 
-- 
2.48.0.rc2.279.g1de40edade-goog

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[David Hildenbrand]

On 22.01.25 16:27, Fuad Tabba wrote:
> Make kvm_(read|/write)_guest_page() capable of accessing guest
> memory for slots that don't have a userspace address, but only if
> the memory is mappable, which also indicates that it is
> accessible by the host.

Interesting. So far my assumption was that, for shared memory, user 
space would simply mmap() guest_memdd and pass it as userspace address 
to the same memslot that has this guest_memfd for private memory.

Wouldn't that be easier in the first shot? (IOW, not require this patch 
with the cost of faulting the shared page into the page table on access)

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Fuad Tabba]

On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
>
> On 22.01.25 16:27, Fuad Tabba wrote:
> > Make kvm_(read|/write)_guest_page() capable of accessing guest
> > memory for slots that don't have a userspace address, but only if
> > the memory is mappable, which also indicates that it is
> > accessible by the host.
>
> Interesting. So far my assumption was that, for shared memory, user
> space would simply mmap() guest_memdd and pass it as userspace address
> to the same memslot that has this guest_memfd for private memory.
>
> Wouldn't that be easier in the first shot? (IOW, not require this patch
> with the cost of faulting the shared page into the page table on access)

This has to do more with the ABI I had for pkvm and shared memory
implementations, in which you don't need to specify the userspace
address for memory in a guestmem memslot. The issue is there is no
obvious address to map it to. This would be the case in kvm:arm64 for
tracking paravirtualized time, which the userspace doesn't necessarily
need to interact with, but kvm does.

That said, we could always have a userspace address dedicated to
mapping shared locations, and use that address when the necessity
arises. Or we could always require that memslots have a userspace
address, even if not used. I don't really have a strong preference.

Thanks,
/fuad

> --
> Cheers,
>
> David / dhildenb
>

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[David Hildenbrand]

On 23.01.25 10:48, Fuad Tabba wrote:
> On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
>>
>> On 22.01.25 16:27, Fuad Tabba wrote:
>>> Make kvm_(read|/write)_guest_page() capable of accessing guest
>>> memory for slots that don't have a userspace address, but only if
>>> the memory is mappable, which also indicates that it is
>>> accessible by the host.
>>
>> Interesting. So far my assumption was that, for shared memory, user
>> space would simply mmap() guest_memdd and pass it as userspace address
>> to the same memslot that has this guest_memfd for private memory.
>>
>> Wouldn't that be easier in the first shot? (IOW, not require this patch
>> with the cost of faulting the shared page into the page table on access)
> 

In light of:

https://lkml.kernel.org/r/20250117190938.93793-4-imbrenda@linux.ibm.com

there can, in theory, be memslots that start at address 0 and have a 
"valid" mapping. This case is done from the kernel (and on special s390x 
hardware), though, so it does not apply here at all so far.

In practice, getting address 0 as a valid address is unlikely, because 
the default:

$ sysctl  vm.mmap_min_addr
vm.mmap_min_addr = 65536

usually prohibits it for good reason.

> This has to do more with the ABI I had for pkvm and shared memory
> implementations, in which you don't need to specify the userspace
> address for memory in a guestmem memslot. The issue is there is no
> obvious address to map it to. This would be the case in kvm:arm64 for
> tracking paravirtualized time, which the userspace doesn't necessarily
> need to interact with, but kvm does.

So I understand correctly: userspace wouldn't have to mmap it because it 
is not interested in accessing it, but there is nothing speaking against 
mmaping it, at least in the first shot.

I assume it would not be a private memslot (so far, my understanding is 
that internal memslots never have a guest_memfd attached). 
kvm_gmem_create() is only called via KVM_CREATE_GUEST_MEMFD, to be set 
on user-created memslots.

> 
> That said, we could always have a userspace address dedicated to
> mapping shared locations, and use that address when the necessity
> arises. Or we could always require that memslots have a userspace
> address, even if not used. I don't really have a strong preference.

So, the simpler version where user space would simply mmap guest_memfd 
to provide the address via userspace_addr would at least work for the 
use case of paravirtualized time?

It would get rid of the immediate need for this patch and patch #4 to 
get it flying.


One interesting question is: when would you want shared memory in 
guest_memfd and *not* provide it as part of the same memslot.


One nice thing about the mmap might be that access go via user-space 
page tables: E.g., __kvm_read_guest_page can just access the memory 
without requiring the folio lock and an additional temporary folio 
reference on every access -- it's handled implicitly via the mapcount.

(of course, to map the page we still need that once on the fault path)

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Patrick Roy]

On Thu, 2025-01-23 at 11:39 +0000, David Hildenbrand wrote:
> On 23.01.25 10:48, Fuad Tabba wrote:
>> On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
>>>
>>> On 22.01.25 16:27, Fuad Tabba wrote:
>>>> Make kvm_(read|/write)_guest_page() capable of accessing guest
>>>> memory for slots that don't have a userspace address, but only if
>>>> the memory is mappable, which also indicates that it is
>>>> accessible by the host.
>>>
>>> Interesting. So far my assumption was that, for shared memory, user
>>> space would simply mmap() guest_memdd and pass it as userspace address
>>> to the same memslot that has this guest_memfd for private memory.
>>>
>>> Wouldn't that be easier in the first shot? (IOW, not require this patch
>>> with the cost of faulting the shared page into the page table on access)
>>
> 
> In light of:
> 
> https://lkml.kernel.org/r/20250117190938.93793-4-imbrenda@linux.ibm.com
> 
> there can, in theory, be memslots that start at address 0 and have a
> "valid" mapping. This case is done from the kernel (and on special s390x
> hardware), though, so it does not apply here at all so far.
> 
> In practice, getting address 0 as a valid address is unlikely, because
> the default:
> 
> $ sysctl  vm.mmap_min_addr
> vm.mmap_min_addr = 65536
> 
> usually prohibits it for good reason.
> 
>> This has to do more with the ABI I had for pkvm and shared memory
>> implementations, in which you don't need to specify the userspace
>> address for memory in a guestmem memslot. The issue is there is no
>> obvious address to map it to. This would be the case in kvm:arm64 for
>> tracking paravirtualized time, which the userspace doesn't necessarily
>> need to interact with, but kvm does.
> 
> So I understand correctly: userspace wouldn't have to mmap it because it
> is not interested in accessing it, but there is nothing speaking against
> mmaping it, at least in the first shot.
> 
> I assume it would not be a private memslot (so far, my understanding is
> that internal memslots never have a guest_memfd attached).
> kvm_gmem_create() is only called via KVM_CREATE_GUEST_MEMFD, to be set
> on user-created memslots.
> 
>>
>> That said, we could always have a userspace address dedicated to
>> mapping shared locations, and use that address when the necessity
>> arises. Or we could always require that memslots have a userspace
>> address, even if not used. I don't really have a strong preference.
> 
> So, the simpler version where user space would simply mmap guest_memfd
> to provide the address via userspace_addr would at least work for the
> use case of paravirtualized time?

fwiw, I'm currently prototyping something like this for x86 (although
not by putting the gmem address into userspace_addr, but by adding a new
field to memslots, so that memory attributes continue working), based on
what we talked about at the last guest_memfd sync meeting (the whole
"how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
story). So I guess if we're going down this route for x86, maybe it
makes sense to do the same on ARM, for consistency?

> It would get rid of the immediate need for this patch and patch #4 to
> get it flying.
> 
> 
> One interesting question is: when would you want shared memory in
> guest_memfd and *not* provide it as part of the same memslot. 

In my testing of non-CoCo gmem VMs on ARM, I've been able to get quite
far without giving KVM a way to internally access shared parts of gmem - 
it's why I was probing Fuad for this simplified series, because
KVM_SW_PROTECTED_VM + mmap (for loading guest kernel) is enough to get a
working non-CoCo VM on ARM (although I admittedly never looked at clocks
inside the guest - maybe that's one thing that breaks if KVM can't
access gmem. How to guest and host agree on the guest memory range
used to exchange paravirtual timekeeping information? Could that exchange
be intercepted in userspace, and set to shared via memory attributes (e.g.
placed outside gmem)? That's the route I'm going down the paravirtual
time on x86).

> One nice thing about the mmap might be that access go via user-space
> page tables: E.g., __kvm_read_guest_page can just access the memory
> without requiring the folio lock and an additional temporary folio
> reference on every access -- it's handled implicitly via the mapcount.
> 
> (of course, to map the page we still need that once on the fault path)

Doing a direct map access in kvm_{read,write}_guest() and friends will
also get tricky if guest_memfd folios ever don't have direct map
entries. On-demand restoration is painful, both complexity and
performance wise [1], while going through a userspace mapping of
guest_memfd would "just work".

> -- 
> Cheers,
> 
> David / dhildenb
> 

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Fuad Tabba]

Hi Patrick,

On Thu, 23 Jan 2025 at 11:57, Patrick Roy <roypat@amazon.co.uk> wrote:
>
>
>
> On Thu, 2025-01-23 at 11:39 +0000, David Hildenbrand wrote:
> > On 23.01.25 10:48, Fuad Tabba wrote:
> >> On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
> >>>
> >>> On 22.01.25 16:27, Fuad Tabba wrote:
> >>>> Make kvm_(read|/write)_guest_page() capable of accessing guest
> >>>> memory for slots that don't have a userspace address, but only if
> >>>> the memory is mappable, which also indicates that it is
> >>>> accessible by the host.
> >>>
> >>> Interesting. So far my assumption was that, for shared memory, user
> >>> space would simply mmap() guest_memdd and pass it as userspace address
> >>> to the same memslot that has this guest_memfd for private memory.
> >>>
> >>> Wouldn't that be easier in the first shot? (IOW, not require this patch
> >>> with the cost of faulting the shared page into the page table on access)
> >>
> >
> > In light of:
> >
> > https://lkml.kernel.org/r/20250117190938.93793-4-imbrenda@linux.ibm.com
> >
> > there can, in theory, be memslots that start at address 0 and have a
> > "valid" mapping. This case is done from the kernel (and on special s390x
> > hardware), though, so it does not apply here at all so far.
> >
> > In practice, getting address 0 as a valid address is unlikely, because
> > the default:
> >
> > $ sysctl  vm.mmap_min_addr
> > vm.mmap_min_addr = 65536
> >
> > usually prohibits it for good reason.
> >
> >> This has to do more with the ABI I had for pkvm and shared memory
> >> implementations, in which you don't need to specify the userspace
> >> address for memory in a guestmem memslot. The issue is there is no
> >> obvious address to map it to. This would be the case in kvm:arm64 for
> >> tracking paravirtualized time, which the userspace doesn't necessarily
> >> need to interact with, but kvm does.
> >
> > So I understand correctly: userspace wouldn't have to mmap it because it
> > is not interested in accessing it, but there is nothing speaking against
> > mmaping it, at least in the first shot.
> >
> > I assume it would not be a private memslot (so far, my understanding is
> > that internal memslots never have a guest_memfd attached).
> > kvm_gmem_create() is only called via KVM_CREATE_GUEST_MEMFD, to be set
> > on user-created memslots.
> >
> >>
> >> That said, we could always have a userspace address dedicated to
> >> mapping shared locations, and use that address when the necessity
> >> arises. Or we could always require that memslots have a userspace
> >> address, even if not used. I don't really have a strong preference.
> >
> > So, the simpler version where user space would simply mmap guest_memfd
> > to provide the address via userspace_addr would at least work for the
> > use case of paravirtualized time?
>
> fwiw, I'm currently prototyping something like this for x86 (although
> not by putting the gmem address into userspace_addr, but by adding a new
> field to memslots, so that memory attributes continue working), based on
> what we talked about at the last guest_memfd sync meeting (the whole
> "how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
> story). So I guess if we're going down this route for x86, maybe it
> makes sense to do the same on ARM, for consistency?
>
> > It would get rid of the immediate need for this patch and patch #4 to
> > get it flying.
> >
> >
> > One interesting question is: when would you want shared memory in
> > guest_memfd and *not* provide it as part of the same memslot.
>
> In my testing of non-CoCo gmem VMs on ARM, I've been able to get quite
> far without giving KVM a way to internally access shared parts of gmem -
> it's why I was probing Fuad for this simplified series, because
> KVM_SW_PROTECTED_VM + mmap (for loading guest kernel) is enough to get a
> working non-CoCo VM on ARM (although I admittedly never looked at clocks
> inside the guest - maybe that's one thing that breaks if KVM can't
> access gmem. How to guest and host agree on the guest memory range
> used to exchange paravirtual timekeeping information? Could that exchange
> be intercepted in userspace, and set to shared via memory attributes (e.g.
> placed outside gmem)? That's the route I'm going down the paravirtual
> time on x86).

For an idea of what it looks like on arm64, here's how kvmtool handles it:
https://github.com/kvmtool/kvmtool/blob/master/arm/aarch64/pvtime.c

Cheers,
/fuad





> > One nice thing about the mmap might be that access go via user-space
> > page tables: E.g., __kvm_read_guest_page can just access the memory
> > without requiring the folio lock and an additional temporary folio
> > reference on every access -- it's handled implicitly via the mapcount.
> >
> > (of course, to map the page we still need that once on the fault path)
>
> Doing a direct map access in kvm_{read,write}_guest() and friends will
> also get tricky if guest_memfd folios ever don't have direct map
> entries. On-demand restoration is painful, both complexity and
> performance wise [1], while going through a userspace mapping of
> guest_memfd would "just work".
>
> > --
> > Cheers,
> >
> > David / dhildenb
> >

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Patrick Roy]

On Thu, 2025-01-23 at 12:28 +0000, Fuad Tabba wrote:
> Hi Patrick,
> 
> On Thu, 23 Jan 2025 at 11:57, Patrick Roy <roypat@amazon.co.uk> wrote:
>>
>>
>>
>> On Thu, 2025-01-23 at 11:39 +0000, David Hildenbrand wrote:
>>> On 23.01.25 10:48, Fuad Tabba wrote:
>>>> On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
>>>>>
>>>>> On 22.01.25 16:27, Fuad Tabba wrote:
>>>>>> Make kvm_(read|/write)_guest_page() capable of accessing guest
>>>>>> memory for slots that don't have a userspace address, but only if
>>>>>> the memory is mappable, which also indicates that it is
>>>>>> accessible by the host.
>>>>>
>>>>> Interesting. So far my assumption was that, for shared memory, user
>>>>> space would simply mmap() guest_memdd and pass it as userspace address
>>>>> to the same memslot that has this guest_memfd for private memory.
>>>>>
>>>>> Wouldn't that be easier in the first shot? (IOW, not require this patch
>>>>> with the cost of faulting the shared page into the page table on access)
>>>>
>>>
>>> In light of:
>>>
>>> https://lkml.kernel.org/r/20250117190938.93793-4-imbrenda@linux.ibm.com
>>>
>>> there can, in theory, be memslots that start at address 0 and have a
>>> "valid" mapping. This case is done from the kernel (and on special s390x
>>> hardware), though, so it does not apply here at all so far.
>>>
>>> In practice, getting address 0 as a valid address is unlikely, because
>>> the default:
>>>
>>> $ sysctl  vm.mmap_min_addr
>>> vm.mmap_min_addr = 65536
>>>
>>> usually prohibits it for good reason.
>>>
>>>> This has to do more with the ABI I had for pkvm and shared memory
>>>> implementations, in which you don't need to specify the userspace
>>>> address for memory in a guestmem memslot. The issue is there is no
>>>> obvious address to map it to. This would be the case in kvm:arm64 for
>>>> tracking paravirtualized time, which the userspace doesn't necessarily
>>>> need to interact with, but kvm does.
>>>
>>> So I understand correctly: userspace wouldn't have to mmap it because it
>>> is not interested in accessing it, but there is nothing speaking against
>>> mmaping it, at least in the first shot.
>>>
>>> I assume it would not be a private memslot (so far, my understanding is
>>> that internal memslots never have a guest_memfd attached).
>>> kvm_gmem_create() is only called via KVM_CREATE_GUEST_MEMFD, to be set
>>> on user-created memslots.
>>>
>>>>
>>>> That said, we could always have a userspace address dedicated to
>>>> mapping shared locations, and use that address when the necessity
>>>> arises. Or we could always require that memslots have a userspace
>>>> address, even if not used. I don't really have a strong preference.
>>>
>>> So, the simpler version where user space would simply mmap guest_memfd
>>> to provide the address via userspace_addr would at least work for the
>>> use case of paravirtualized time?
>>
>> fwiw, I'm currently prototyping something like this for x86 (although
>> not by putting the gmem address into userspace_addr, but by adding a new
>> field to memslots, so that memory attributes continue working), based on
>> what we talked about at the last guest_memfd sync meeting (the whole
>> "how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
>> story). So I guess if we're going down this route for x86, maybe it
>> makes sense to do the same on ARM, for consistency?
>>
>>> It would get rid of the immediate need for this patch and patch #4 to
>>> get it flying.
>>>
>>>
>>> One interesting question is: when would you want shared memory in
>>> guest_memfd and *not* provide it as part of the same memslot.
>>
>> In my testing of non-CoCo gmem VMs on ARM, I've been able to get quite
>> far without giving KVM a way to internally access shared parts of gmem -
>> it's why I was probing Fuad for this simplified series, because
>> KVM_SW_PROTECTED_VM + mmap (for loading guest kernel) is enough to get a
>> working non-CoCo VM on ARM (although I admittedly never looked at clocks
>> inside the guest - maybe that's one thing that breaks if KVM can't
>> access gmem. How to guest and host agree on the guest memory range
>> used to exchange paravirtual timekeeping information? Could that exchange
>> be intercepted in userspace, and set to shared via memory attributes (e.g.
>> placed outside gmem)? That's the route I'm going down the paravirtual
>> time on x86).
> 
> For an idea of what it looks like on arm64, here's how kvmtool handles it:
> https://github.com/kvmtool/kvmtool/blob/master/arm/aarch64/pvtime.c
> 
> Cheers,
> /fuad
 
Thanks! In that example, kvmtool actually allocates a separate memslot for
the pvclock stuff, so I guess it's always possible to simply put it into
a non-gmem memslot, which indeed sidesteps this issue as you mention in
your reply to David :D
  
>>> One nice thing about the mmap might be that access go via user-space
>>> page tables: E.g., __kvm_read_guest_page can just access the memory
>>> without requiring the folio lock and an additional temporary folio
>>> reference on every access -- it's handled implicitly via the mapcount.
>>>
>>> (of course, to map the page we still need that once on the fault path)
>>
>> Doing a direct map access in kvm_{read,write}_guest() and friends will
>> also get tricky if guest_memfd folios ever don't have direct map
>> entries. On-demand restoration is painful, both complexity and
>> performance wise [1], while going through a userspace mapping of
>> guest_memfd would "just work".
>>
>>> --
>>> Cheers,
>>>
>>> David / dhildenb
>>>

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[David Hildenbrand]

On 23.01.25 14:57, Patrick Roy wrote:
> 
> 
> On Thu, 2025-01-23 at 12:28 +0000, Fuad Tabba wrote:
>> Hi Patrick,
>>
>> On Thu, 23 Jan 2025 at 11:57, Patrick Roy <roypat@amazon.co.uk> wrote:
>>>
>>>
>>>
>>> On Thu, 2025-01-23 at 11:39 +0000, David Hildenbrand wrote:
>>>> On 23.01.25 10:48, Fuad Tabba wrote:
>>>>> On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
>>>>>>
>>>>>> On 22.01.25 16:27, Fuad Tabba wrote:
>>>>>>> Make kvm_(read|/write)_guest_page() capable of accessing guest
>>>>>>> memory for slots that don't have a userspace address, but only if
>>>>>>> the memory is mappable, which also indicates that it is
>>>>>>> accessible by the host.
>>>>>>
>>>>>> Interesting. So far my assumption was that, for shared memory, user
>>>>>> space would simply mmap() guest_memdd and pass it as userspace address
>>>>>> to the same memslot that has this guest_memfd for private memory.
>>>>>>
>>>>>> Wouldn't that be easier in the first shot? (IOW, not require this patch
>>>>>> with the cost of faulting the shared page into the page table on access)
>>>>>
>>>>
>>>> In light of:
>>>>
>>>> https://lkml.kernel.org/r/20250117190938.93793-4-imbrenda@linux.ibm.com
>>>>
>>>> there can, in theory, be memslots that start at address 0 and have a
>>>> "valid" mapping. This case is done from the kernel (and on special s390x
>>>> hardware), though, so it does not apply here at all so far.
>>>>
>>>> In practice, getting address 0 as a valid address is unlikely, because
>>>> the default:
>>>>
>>>> $ sysctl  vm.mmap_min_addr
>>>> vm.mmap_min_addr = 65536
>>>>
>>>> usually prohibits it for good reason.
>>>>
>>>>> This has to do more with the ABI I had for pkvm and shared memory
>>>>> implementations, in which you don't need to specify the userspace
>>>>> address for memory in a guestmem memslot. The issue is there is no
>>>>> obvious address to map it to. This would be the case in kvm:arm64 for
>>>>> tracking paravirtualized time, which the userspace doesn't necessarily
>>>>> need to interact with, but kvm does.
>>>>
>>>> So I understand correctly: userspace wouldn't have to mmap it because it
>>>> is not interested in accessing it, but there is nothing speaking against
>>>> mmaping it, at least in the first shot.
>>>>
>>>> I assume it would not be a private memslot (so far, my understanding is
>>>> that internal memslots never have a guest_memfd attached).
>>>> kvm_gmem_create() is only called via KVM_CREATE_GUEST_MEMFD, to be set
>>>> on user-created memslots.
>>>>
>>>>>
>>>>> That said, we could always have a userspace address dedicated to
>>>>> mapping shared locations, and use that address when the necessity
>>>>> arises. Or we could always require that memslots have a userspace
>>>>> address, even if not used. I don't really have a strong preference.
>>>>
>>>> So, the simpler version where user space would simply mmap guest_memfd
>>>> to provide the address via userspace_addr would at least work for the
>>>> use case of paravirtualized time?
>>>
>>> fwiw, I'm currently prototyping something like this for x86 (although
>>> not by putting the gmem address into userspace_addr, but by adding a new
>>> field to memslots, so that memory attributes continue working), based on
>>> what we talked about at the last guest_memfd sync meeting (the whole
>>> "how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
>>> story). So I guess if we're going down this route for x86, maybe it
>>> makes sense to do the same on ARM, for consistency?
>>>
>>>> It would get rid of the immediate need for this patch and patch #4 to
>>>> get it flying.
>>>>
>>>>
>>>> One interesting question is: when would you want shared memory in
>>>> guest_memfd and *not* provide it as part of the same memslot.
>>>
>>> In my testing of non-CoCo gmem VMs on ARM, I've been able to get quite
>>> far without giving KVM a way to internally access shared parts of gmem -
>>> it's why I was probing Fuad for this simplified series, because
>>> KVM_SW_PROTECTED_VM + mmap (for loading guest kernel) is enough to get a
>>> working non-CoCo VM on ARM (although I admittedly never looked at clocks
>>> inside the guest - maybe that's one thing that breaks if KVM can't
>>> access gmem. How to guest and host agree on the guest memory range
>>> used to exchange paravirtual timekeeping information? Could that exchange
>>> be intercepted in userspace, and set to shared via memory attributes (e.g.
>>> placed outside gmem)? That's the route I'm going down the paravirtual
>>> time on x86).
>>
>> For an idea of what it looks like on arm64, here's how kvmtool handles it:
>> https://github.com/kvmtool/kvmtool/blob/master/arm/aarch64/pvtime.c
>>
>> Cheers,
>> /fuad
>   
> Thanks! In that example, kvmtool actually allocates a separate memslot for
> the pvclock stuff, so I guess it's always possible to simply put it into
> a non-gmem memslot, which indeed sidesteps this issue as you mention in
> your reply to David :D

Does that work on CC where all memory defaults to private first, and the 
VM explicitly has to opt into marking it shared first, or how exactly 
would the flow of operations be in the cases of the non-gmem ("good 
old") memslot?

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Fuad Tabba]

On Thu, 23 Jan 2025 at 14:21, David Hildenbrand <david@redhat.com> wrote:
>
> On 23.01.25 14:57, Patrick Roy wrote:
> >
> >
> > On Thu, 2025-01-23 at 12:28 +0000, Fuad Tabba wrote:
> >> Hi Patrick,
> >>
> >> On Thu, 23 Jan 2025 at 11:57, Patrick Roy <roypat@amazon.co.uk> wrote:
> >>>
> >>>
> >>>
> >>> On Thu, 2025-01-23 at 11:39 +0000, David Hildenbrand wrote:
> >>>> On 23.01.25 10:48, Fuad Tabba wrote:
> >>>>> On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
> >>>>>>
> >>>>>> On 22.01.25 16:27, Fuad Tabba wrote:
> >>>>>>> Make kvm_(read|/write)_guest_page() capable of accessing guest
> >>>>>>> memory for slots that don't have a userspace address, but only if
> >>>>>>> the memory is mappable, which also indicates that it is
> >>>>>>> accessible by the host.
> >>>>>>
> >>>>>> Interesting. So far my assumption was that, for shared memory, user
> >>>>>> space would simply mmap() guest_memdd and pass it as userspace address
> >>>>>> to the same memslot that has this guest_memfd for private memory.
> >>>>>>
> >>>>>> Wouldn't that be easier in the first shot? (IOW, not require this patch
> >>>>>> with the cost of faulting the shared page into the page table on access)
> >>>>>
> >>>>
> >>>> In light of:
> >>>>
> >>>> https://lkml.kernel.org/r/20250117190938.93793-4-imbrenda@linux.ibm.com
> >>>>
> >>>> there can, in theory, be memslots that start at address 0 and have a
> >>>> "valid" mapping. This case is done from the kernel (and on special s390x
> >>>> hardware), though, so it does not apply here at all so far.
> >>>>
> >>>> In practice, getting address 0 as a valid address is unlikely, because
> >>>> the default:
> >>>>
> >>>> $ sysctl  vm.mmap_min_addr
> >>>> vm.mmap_min_addr = 65536
> >>>>
> >>>> usually prohibits it for good reason.
> >>>>
> >>>>> This has to do more with the ABI I had for pkvm and shared memory
> >>>>> implementations, in which you don't need to specify the userspace
> >>>>> address for memory in a guestmem memslot. The issue is there is no
> >>>>> obvious address to map it to. This would be the case in kvm:arm64 for
> >>>>> tracking paravirtualized time, which the userspace doesn't necessarily
> >>>>> need to interact with, but kvm does.
> >>>>
> >>>> So I understand correctly: userspace wouldn't have to mmap it because it
> >>>> is not interested in accessing it, but there is nothing speaking against
> >>>> mmaping it, at least in the first shot.
> >>>>
> >>>> I assume it would not be a private memslot (so far, my understanding is
> >>>> that internal memslots never have a guest_memfd attached).
> >>>> kvm_gmem_create() is only called via KVM_CREATE_GUEST_MEMFD, to be set
> >>>> on user-created memslots.
> >>>>
> >>>>>
> >>>>> That said, we could always have a userspace address dedicated to
> >>>>> mapping shared locations, and use that address when the necessity
> >>>>> arises. Or we could always require that memslots have a userspace
> >>>>> address, even if not used. I don't really have a strong preference.
> >>>>
> >>>> So, the simpler version where user space would simply mmap guest_memfd
> >>>> to provide the address via userspace_addr would at least work for the
> >>>> use case of paravirtualized time?
> >>>
> >>> fwiw, I'm currently prototyping something like this for x86 (although
> >>> not by putting the gmem address into userspace_addr, but by adding a new
> >>> field to memslots, so that memory attributes continue working), based on
> >>> what we talked about at the last guest_memfd sync meeting (the whole
> >>> "how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
> >>> story). So I guess if we're going down this route for x86, maybe it
> >>> makes sense to do the same on ARM, for consistency?
> >>>
> >>>> It would get rid of the immediate need for this patch and patch #4 to
> >>>> get it flying.
> >>>>
> >>>>
> >>>> One interesting question is: when would you want shared memory in
> >>>> guest_memfd and *not* provide it as part of the same memslot.
> >>>
> >>> In my testing of non-CoCo gmem VMs on ARM, I've been able to get quite
> >>> far without giving KVM a way to internally access shared parts of gmem -
> >>> it's why I was probing Fuad for this simplified series, because
> >>> KVM_SW_PROTECTED_VM + mmap (for loading guest kernel) is enough to get a
> >>> working non-CoCo VM on ARM (although I admittedly never looked at clocks
> >>> inside the guest - maybe that's one thing that breaks if KVM can't
> >>> access gmem. How to guest and host agree on the guest memory range
> >>> used to exchange paravirtual timekeeping information? Could that exchange
> >>> be intercepted in userspace, and set to shared via memory attributes (e.g.
> >>> placed outside gmem)? That's the route I'm going down the paravirtual
> >>> time on x86).
> >>
> >> For an idea of what it looks like on arm64, here's how kvmtool handles it:
> >> https://github.com/kvmtool/kvmtool/blob/master/arm/aarch64/pvtime.c
> >>
> >> Cheers,
> >> /fuad
> >
> > Thanks! In that example, kvmtool actually allocates a separate memslot for
> > the pvclock stuff, so I guess it's always possible to simply put it into
> > a non-gmem memslot, which indeed sidesteps this issue as you mention in
> > your reply to David :D
>
> Does that work on CC where all memory defaults to private first, and the
> VM explicitly has to opt into marking it shared first, or how exactly
> would the flow of operations be in the cases of the non-gmem ("good
> old") memslot?

We use a normal memslot, without the KVM_MEM_GUEST_MEMFD flag, and
consider that kind of slot to be shared by default.

Cheers,
/fuad

> --
> Cheers,
>
> David / dhildenb
>

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[David Hildenbrand]

>>>
>>> That said, we could always have a userspace address dedicated to
>>> mapping shared locations, and use that address when the necessity
>>> arises. Or we could always require that memslots have a userspace
>>> address, even if not used. I don't really have a strong preference.
>>
>> So, the simpler version where user space would simply mmap guest_memfd
>> to provide the address via userspace_addr would at least work for the
>> use case of paravirtualized time?
> 
> fwiw, I'm currently prototyping something like this for x86 (although
> not by putting the gmem address into userspace_addr, but by adding a new
> field to memslots, so that memory attributes continue working), based on
> what we talked about at the last guest_memfd sync meeting (the whole
> "how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
> story).

Yes, I recall that discussion. Can you elaborate why the separate field 
is required to keep memory attributes working? (could it be sorted out 
differently, by reusing userspace_addr?).

> So I guess if we're going down this route for x86, maybe it
> makes sense to do the same on ARM, for consistency?
> 
>> It would get rid of the immediate need for this patch and patch #4 to
>> get it flying.
>>
>>
>> One interesting question is: when would you want shared memory in
>> guest_memfd and *not* provide it as part of the same memslot.
> 
> In my testing of non-CoCo gmem VMs on ARM, I've been able to get quite
> far without giving KVM a way to internally access shared parts of gmem -
> it's why I was probing Fuad for this simplified series, because
> KVM_SW_PROTECTED_VM + mmap (for loading guest kernel) is enough to get a
> working non-CoCo VM on ARM (although I admittedly never looked at clocks
> inside the guest - maybe that's one thing that breaks if KVM can't
> access gmem. How to guest and host agree on the guest memory range
> used to exchange paravirtual timekeeping information? Could that exchange
> be intercepted in userspace, and set to shared via memory attributes (e.g.
> placed outside gmem)? That's the route I'm going down the paravirtual
> time on x86).

Sounds reasonable to me.

> 
>> One nice thing about the mmap might be that access go via user-space
>> page tables: E.g., __kvm_read_guest_page can just access the memory
>> without requiring the folio lock and an additional temporary folio
>> reference on every access -- it's handled implicitly via the mapcount.
>>
>> (of course, to map the page we still need that once on the fault path)
> 
> Doing a direct map access in kvm_{read,write}_guest() and friends will
> also get tricky if guest_memfd folios ever don't have direct map
> entries. On-demand restoration is painful, both complexity and
> performance wise [1], while going through a userspace mapping of
> guest_memfd would "just work".

Indeed.

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Patrick Roy]

On Thu, 2025-01-23 at 14:18 +0000, David Hildenbrand wrote:
>>>>
>>>> That said, we could always have a userspace address dedicated to
>>>> mapping shared locations, and use that address when the necessity
>>>> arises. Or we could always require that memslots have a userspace
>>>> address, even if not used. I don't really have a strong preference.
>>>
>>> So, the simpler version where user space would simply mmap guest_memfd
>>> to provide the address via userspace_addr would at least work for the
>>> use case of paravirtualized time?
>>
>> fwiw, I'm currently prototyping something like this for x86 (although
>> not by putting the gmem address into userspace_addr, but by adding a new
>> field to memslots, so that memory attributes continue working), based on
>> what we talked about at the last guest_memfd sync meeting (the whole
>> "how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
>> story).
> 
> Yes, I recall that discussion. Can you elaborate why the separate field
> is required to keep memory attributes working? (could it be sorted out
> differently, by reusing userspace_addr?).

The scenario I ran into was that within the same memslots, I wanted some
gfns to be backed by guest_memfd, and others by traditional memory, so
that KVM can GUP some parts of guest memory even if guest_memfd itself
is direct map removed.

It actually also has to do with paravirtual time, but on x86. Here, the
guest chooses where in guest memory the clock structure is placed via an
MSR write (so I can't a priori use a traditional memslot, like we can on
ARM).  KVM internally wants to GUP the hva that corresponds to the gfn
the guest chooses, but if the hva is in a mapping of direct map removed
gmem, that won't work. So what I did was just intercept the MSR write in
userspace, and clear KVM_MEMORY_ATTRIBUTES_PRIVATE for the gfn. But for
this, I need userspace_addr to not point to the guest_memfd hva.
Although maybe it'd be possible to instead reconfigure the memslots when
intercepting the MSR? Not sure where we stand on KVM_MEM_GUEST_MEMFD
memslots though.

But also conceptually, doesn't KVM_MEMORY_ATTRIBUTES_PRIVATE kinda loose
any meaning if userspace_addr also points towards gmem? E.g. no matter
what we set, we'd get gmem mapped into the guest.

> -- 
> Cheers,
> 
> David / dhildenb
> 

Best, 
Patrick

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[David Hildenbrand]

On 23.01.25 16:22, Patrick Roy wrote:
> On Thu, 2025-01-23 at 14:18 +0000, David Hildenbrand wrote:
>>>>>
>>>>> That said, we could always have a userspace address dedicated to
>>>>> mapping shared locations, and use that address when the necessity
>>>>> arises. Or we could always require that memslots have a userspace
>>>>> address, even if not used. I don't really have a strong preference.
>>>>
>>>> So, the simpler version where user space would simply mmap guest_memfd
>>>> to provide the address via userspace_addr would at least work for the
>>>> use case of paravirtualized time?
>>>
>>> fwiw, I'm currently prototyping something like this for x86 (although
>>> not by putting the gmem address into userspace_addr, but by adding a new
>>> field to memslots, so that memory attributes continue working), based on
>>> what we talked about at the last guest_memfd sync meeting (the whole
>>> "how to get MMIO emulation working for non-CoCo VMs in guest_memfd"
>>> story).
>>
>> Yes, I recall that discussion. Can you elaborate why the separate field
>> is required to keep memory attributes working? (could it be sorted out
>> differently, by reusing userspace_addr?).
> 
> The scenario I ran into was that within the same memslots, I wanted some
> gfns to be backed by guest_memfd, and others by traditional memory, so
> that KVM can GUP some parts of guest memory even if guest_memfd itself
> is direct map removed.

Just summarizing what we discussed yesterday:

GUP will not be allowed if the direct map was removed, and paravirt time 
on x86 uses GUP to access the guest page right now.

> 
> It actually also has to do with paravirtual time, but on x86. Here, the
> guest chooses where in guest memory the clock structure is placed via an
> MSR write (so I can't a priori use a traditional memslot, like we can on
> ARM).  KVM internally wants to GUP the hva that corresponds to the gfn
> the guest chooses, but if the hva is in a mapping of direct map removed
> gmem, that won't work.

And as discussed here, Sean raised that these hypervisor updates happen 
rarely.

In the first step, it might be good enough to just update using the 
user-space page table mappings (user access), to avoid messing with 
kmap/direct-map reinstalling.

With that in place, it looks like that user space could simply mmap 
guest_memfd and provide the mmaped area "ordinarily" as the "shared 
memory" part of the guest_memfd -- using userspace_address.

GUP would fail, bit paravirt time would not be using GUP.

So we could handle the guest_memfd mmap just like on other 
architectures, without the need for other memslot fields.

Please correct me if I misunderstood something :)

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[Fuad Tabba]

Hi David,

On Thu, 23 Jan 2025 at 11:39, David Hildenbrand <david@redhat.com> wrote:
>
> On 23.01.25 10:48, Fuad Tabba wrote:
> > On Wed, 22 Jan 2025 at 22:10, David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 22.01.25 16:27, Fuad Tabba wrote:
> >>> Make kvm_(read|/write)_guest_page() capable of accessing guest
> >>> memory for slots that don't have a userspace address, but only if
> >>> the memory is mappable, which also indicates that it is
> >>> accessible by the host.
> >>
> >> Interesting. So far my assumption was that, for shared memory, user
> >> space would simply mmap() guest_memdd and pass it as userspace address
> >> to the same memslot that has this guest_memfd for private memory.
> >>
> >> Wouldn't that be easier in the first shot? (IOW, not require this patch
> >> with the cost of faulting the shared page into the page table on access)
> >

Before answering your questions in detail, in light of this
discussion, and in other discussions I've had, I think the more
reasonable thing to do is to let the memslot for in-place shared
memory look the same as the other guest_memfd memslots, i.e., have a
userspace address regardless if it's needed or not. This makes the
interface the same across all guest_memfd implementations, and avoids
the need for patches such as this one, at least initially until we
realize that we need it.

That said...

>
> In light of:
>
> https://lkml.kernel.org/r/20250117190938.93793-4-imbrenda@linux.ibm.com
>
> there can, in theory, be memslots that start at address 0 and have a
> "valid" mapping. This case is done from the kernel (and on special s390x
> hardware), though, so it does not apply here at all so far.
>
> In practice, getting address 0 as a valid address is unlikely, because
> the default:
>
> $ sysctl  vm.mmap_min_addr
> vm.mmap_min_addr = 65536
>
> usually prohibits it for good reason.

Ack.

> > This has to do more with the ABI I had for pkvm and shared memory
> > implementations, in which you don't need to specify the userspace
> > address for memory in a guestmem memslot. The issue is there is no
> > obvious address to map it to. This would be the case in kvm:arm64 for
> > tracking paravirtualized time, which the userspace doesn't necessarily
> > need to interact with, but kvm does.
>
> So I understand correctly: userspace wouldn't have to mmap it because it
> is not interested in accessing it, but there is nothing speaking against
> mmaping it, at least in the first shot.

That's right.

> I assume it would not be a private memslot (so far, my understanding is
> that internal memslots never have a guest_memfd attached).
> kvm_gmem_create() is only called via KVM_CREATE_GUEST_MEMFD, to be set
> on user-created memslots.

Right, it wouldn't be a private memslot, but a user created one.

> >
> > That said, we could always have a userspace address dedicated to
> > mapping shared locations, and use that address when the necessity
> > arises. Or we could always require that memslots have a userspace
> > address, even if not used. I don't really have a strong preference.
>
> So, the simpler version where user space would simply mmap guest_memfd
> to provide the address via userspace_addr would at least work for the
> use case of paravirtualized time?
>
> It would get rid of the immediate need for this patch and patch #4 to
> get it flying.

I agree now.

>
> One interesting question is: when would you want shared memory in
> guest_memfd and *not* provide it as part of the same memslot.
>

My original thinking wasn't that we wouldn't _want_ to, rather that we
don't _need_ to. I mistakenly thought that it would simplify things,
but, especially after this discussion, I now realize that it makes
things more complicated instead.

> One nice thing about the mmap might be that access go via user-space
> page tables: E.g., __kvm_read_guest_page can just access the memory
> without requiring the folio lock and an additional temporary folio
> reference on every access -- it's handled implicitly via the mapcount.
>
> (of course, to map the page we still need that once on the fault path)

Ack.

Thanks,
/fuad


> --
> Cheers,
>
> David / dhildenb
>

Re: [RFC PATCH v1 2/9] KVM: guest_memfd: Add guest_memfd support to kvm_(read|/write)_guest_page()

[David Hildenbrand]

>>
>> One interesting question is: when would you want shared memory in
>> guest_memfd and *not* provide it as part of the same memslot.
>>
> 
> My original thinking wasn't that we wouldn't _want_ to, rather that we
> don't _need_ to. I mistakenly thought that it would simplify things,
> but, especially after this discussion, I now realize that it makes
> things more complicated instead.

Right, and it's not off the table: we could always add support for this 
"no mmaped shared guest_memfd" memory later if there is good reason to 
have it.

-- 
Cheers,

David / dhildenb

[RFC PATCH v1 3/9] KVM: guest_memfd: Add KVM capability to check if guest_memfd is host mappable

[Fuad Tabba]

Add the KVM capability KVM_CAP_GUEST_MEMFD_MAPPABLE, which is
true if mapping guest memory is supported by the host.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 include/uapi/linux/kvm.h | 1 +
 virt/kvm/kvm_main.c      | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 502ea63b5d2e..021f8ef9979b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -933,6 +933,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_PRE_FAULT_MEMORY 236
 #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237
 #define KVM_CAP_X86_GUEST_MODE 238
+#define KVM_CAP_GUEST_MEMFD_MAPPABLE 239
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ad9802012a3f..9cd6690b7955 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4872,6 +4872,10 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 #ifdef CONFIG_KVM_PRIVATE_MEM
 	case KVM_CAP_GUEST_MEMFD:
 		return !kvm || kvm_arch_has_private_mem(kvm);
+#endif
+#ifdef CONFIG_KVM_GMEM_MAPPABLE
+	case KVM_CAP_GUEST_MEMFD_MAPPABLE:
+		return !kvm || kvm_arch_private_mem_inplace(kvm);
 #endif
 	default:
 		break;
-- 
2.48.0.rc2.279.g1de40edade-goog

Re: [RFC PATCH v1 3/9] KVM: guest_memfd: Add KVM capability to check if guest_memfd is host mappable

[David Hildenbrand]

On 22.01.25 16:27, Fuad Tabba wrote:
> Add the KVM capability KVM_CAP_GUEST_MEMFD_MAPPABLE, which is
> true if mapping guest memory is supported by the host.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>   include/uapi/linux/kvm.h | 1 +
>   virt/kvm/kvm_main.c      | 4 ++++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 502ea63b5d2e..021f8ef9979b 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -933,6 +933,7 @@ struct kvm_enable_cap {
>   #define KVM_CAP_PRE_FAULT_MEMORY 236
>   #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237
>   #define KVM_CAP_X86_GUEST_MODE 238
> +#define KVM_CAP_GUEST_MEMFD_MAPPABLE 239

Maybe similarly consider calling this something like

KVM_CAP_GUEST_MEMFD_SHARED_MEM

No strong opinion on this one, though.

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 3/9] KVM: guest_memfd: Add KVM capability to check if guest_memfd is host mappable

[Fuad Tabba]

On Thu, 23 Jan 2025 at 11:42, David Hildenbrand <david@redhat.com> wrote:
>
> On 22.01.25 16:27, Fuad Tabba wrote:
> > Add the KVM capability KVM_CAP_GUEST_MEMFD_MAPPABLE, which is
> > true if mapping guest memory is supported by the host.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >   include/uapi/linux/kvm.h | 1 +
> >   virt/kvm/kvm_main.c      | 4 ++++
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 502ea63b5d2e..021f8ef9979b 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -933,6 +933,7 @@ struct kvm_enable_cap {
> >   #define KVM_CAP_PRE_FAULT_MEMORY 236
> >   #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237
> >   #define KVM_CAP_X86_GUEST_MODE 238
> > +#define KVM_CAP_GUEST_MEMFD_MAPPABLE 239
>
> Maybe similarly consider calling this something like
>
> KVM_CAP_GUEST_MEMFD_SHARED_MEM
>
> No strong opinion on this one, though.

Ack. That would make the naming consistent.
/fuad

> --
> Cheers,
>
> David / dhildenb
>

[RFC PATCH v1 4/9] KVM: arm64: Skip VMA checks for slots without userspace address

[Fuad Tabba]

Memory slots backed by guest memory might be created with no
intention of being mapped by the host. These are recognized by
not having a userspace address in the memory slot.

VMA checks are neither possible nor necessary for this kind of
slot, so skip them.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/mmu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index c9d46ad57e52..342a9bd3848f 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -988,6 +988,10 @@ static void stage2_unmap_memslot(struct kvm *kvm,
 	phys_addr_t size = PAGE_SIZE * memslot->npages;
 	hva_t reg_end = hva + size;
 
+	/* Host will not map this private memory without a userspace address. */
+	if (kvm_slot_can_be_private(memslot) && !hva)
+		return;
+
 	/*
 	 * A memory region could potentially cover multiple VMAs, and any holes
 	 * between them, so iterate over all of them to find out if we should
@@ -2133,6 +2137,10 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
 	hva = new->userspace_addr;
 	reg_end = hva + (new->npages << PAGE_SHIFT);
 
+	/* Host will not map this private memory without a userspace address. */
+	if ((kvm_slot_can_be_private(new)) && !hva)
+		return 0;
+
 	mmap_read_lock(current->mm);
 	/*
 	 * A memory region could potentially cover multiple VMAs, and any holes
-- 
2.48.0.rc2.279.g1de40edade-goog

[RFC PATCH v1 5/9] KVM: arm64: Refactor user_mem_abort() calculation of force_pte

[Fuad Tabba]

To simplify the code and to make the assumptions clearer,
refactor user_mem_abort() by immediately setting force_pte to
true if logging_active is true. Also, add a check to ensure that
the assumption that logging_active is guaranteed to never be true
for VM_PFNMAP memslot is true.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/mmu.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 342a9bd3848f..9b1921c1a1a0 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1440,7 +1440,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			  bool fault_is_perm)
 {
 	int ret = 0;
-	bool write_fault, writable, force_pte = false;
+	bool write_fault, writable;
 	bool exec_fault, mte_allowed;
 	bool device = false, vfio_allow_any_uc = false;
 	unsigned long mmu_seq;
@@ -1452,6 +1452,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	gfn_t gfn;
 	kvm_pfn_t pfn;
 	bool logging_active = memslot_is_logging(memslot);
+	bool force_pte = logging_active;
 	long vma_pagesize, fault_granule;
 	enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R;
 	struct kvm_pgtable *pgt;
@@ -1497,12 +1498,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	 * logging_active is guaranteed to never be true for VM_PFNMAP
 	 * memslots.
 	 */
-	if (logging_active) {
-		force_pte = true;
+	if (WARN_ON_ONCE(logging_active && (vma->vm_flags & VM_PFNMAP)))
+		return -EFAULT;
+
+	if (force_pte)
 		vma_shift = PAGE_SHIFT;
-	} else {
+	else
 		vma_shift = get_vma_page_shift(vma, hva);
-	}
 
 	switch (vma_shift) {
 #ifndef __PAGETABLE_PMD_FOLDED
-- 
2.48.0.rc2.279.g1de40edade-goog

[RFC PATCH v1 6/9] KVM: arm64: Handle guest_memfd()-backed guest page faults

[Fuad Tabba]

Add arm64 support for resolving guest page faults on
guest_memfd() backed memslots. This support is not contingent on
pKVM, or other confidential computing support, and works in both
VHE and nVHE modes.

Without confidential computing, this support is useful for
testing and debugging. In the future, it might also be useful
should a user want to use guest_memfd() for all code, whether
it's for a protected guest or not.

For now, the fault granule is restricted to PAGE_SIZE.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/mmu.c     | 86 ++++++++++++++++++++++++++++------------
 include/linux/kvm_host.h |  5 +++
 virt/kvm/kvm_main.c      |  5 ---
 3 files changed, 66 insertions(+), 30 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 9b1921c1a1a0..adf23618e2a0 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1434,6 +1434,39 @@ static bool kvm_vma_mte_allowed(struct vm_area_struct *vma)
 	return vma->vm_flags & VM_MTE_ALLOWED;
 }
 
+static kvm_pfn_t faultin_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
+			     gfn_t gfn, bool write_fault, bool *writable,
+			     struct page **page, bool is_private)
+{
+	kvm_pfn_t pfn;
+	int ret;
+
+	if (!is_private)
+		return __kvm_faultin_pfn(slot, gfn, write_fault ? FOLL_WRITE : 0, writable, page);
+
+	*writable = false;
+
+	if (WARN_ON_ONCE(write_fault && memslot_is_readonly(slot)))
+		return KVM_PFN_ERR_NOSLOT_MASK;
+
+	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, page, NULL);
+	if (!ret) {
+		*writable = write_fault;
+		return pfn;
+	}
+
+	if (ret == -EHWPOISON)
+		return KVM_PFN_ERR_HWPOISON;
+
+	return KVM_PFN_ERR_NOSLOT_MASK;
+}
+
+static bool is_private_mem(struct kvm *kvm, struct kvm_memory_slot *memslot, phys_addr_t ipa)
+{
+	return kvm_arch_has_private_mem(kvm) && kvm_slot_can_be_private(memslot) &&
+	       (kvm_mem_is_private(kvm, ipa >> PAGE_SHIFT) || !memslot->userspace_addr);
+}
+
 static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			  struct kvm_s2_trans *nested,
 			  struct kvm_memory_slot *memslot, unsigned long hva,
@@ -1441,24 +1474,25 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 {
 	int ret = 0;
 	bool write_fault, writable;
-	bool exec_fault, mte_allowed;
+	bool exec_fault, mte_allowed = false;
 	bool device = false, vfio_allow_any_uc = false;
 	unsigned long mmu_seq;
 	phys_addr_t ipa = fault_ipa;
 	struct kvm *kvm = vcpu->kvm;
 	struct kvm_mmu_memory_cache *memcache = &vcpu->arch.mmu_page_cache;
-	struct vm_area_struct *vma;
+	struct vm_area_struct *vma = NULL;
 	short vma_shift;
 	gfn_t gfn;
 	kvm_pfn_t pfn;
 	bool logging_active = memslot_is_logging(memslot);
-	bool force_pte = logging_active;
-	long vma_pagesize, fault_granule;
+	bool is_private = is_private_mem(kvm, memslot, fault_ipa);
+	bool force_pte = logging_active || is_private;
+	long vma_pagesize, fault_granule = PAGE_SIZE;
 	enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R;
 	struct kvm_pgtable *pgt;
 	struct page *page;
 
-	if (fault_is_perm)
+	if (fault_is_perm && !is_private)
 		fault_granule = kvm_vcpu_trap_get_perm_fault_granule(vcpu);
 	write_fault = kvm_is_write_fault(vcpu);
 	exec_fault = kvm_vcpu_trap_is_exec_fault(vcpu);
@@ -1482,24 +1516,30 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			return ret;
 	}
 
+	mmap_read_lock(current->mm);
+
 	/*
 	 * Let's check if we will get back a huge page backed by hugetlbfs, or
 	 * get block mapping for device MMIO region.
 	 */
-	mmap_read_lock(current->mm);
-	vma = vma_lookup(current->mm, hva);
-	if (unlikely(!vma)) {
-		kvm_err("Failed to find VMA for hva 0x%lx\n", hva);
-		mmap_read_unlock(current->mm);
-		return -EFAULT;
-	}
+	if (!is_private) {
+		vma = vma_lookup(current->mm, hva);
+		if (unlikely(!vma)) {
+			kvm_err("Failed to find VMA for hva 0x%lx\n", hva);
+			mmap_read_unlock(current->mm);
+			return -EFAULT;
+		}
 
-	/*
-	 * logging_active is guaranteed to never be true for VM_PFNMAP
-	 * memslots.
-	 */
-	if (WARN_ON_ONCE(logging_active && (vma->vm_flags & VM_PFNMAP)))
-		return -EFAULT;
+		/*
+		 * logging_active is guaranteed to never be true for VM_PFNMAP
+		 * memslots.
+		 */
+		if (WARN_ON_ONCE(logging_active && (vma->vm_flags & VM_PFNMAP)))
+			return -EFAULT;
+
+		vfio_allow_any_uc = vma->vm_flags & VM_ALLOW_ANY_UNCACHED;
+		mte_allowed = kvm_vma_mte_allowed(vma);
+	}
 
 	if (force_pte)
 		vma_shift = PAGE_SHIFT;
@@ -1570,17 +1610,14 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	}
 
 	gfn = ipa >> PAGE_SHIFT;
-	mte_allowed = kvm_vma_mte_allowed(vma);
-
-	vfio_allow_any_uc = vma->vm_flags & VM_ALLOW_ANY_UNCACHED;
 
 	/* Don't use the VMA after the unlock -- it may have vanished */
 	vma = NULL;
 
 	/*
 	 * Read mmu_invalidate_seq so that KVM can detect if the results of
-	 * vma_lookup() or __kvm_faultin_pfn() become stale prior to
-	 * acquiring kvm->mmu_lock.
+	 * vma_lookup() or faultin_pfn() become stale prior to acquiring
+	 * kvm->mmu_lock.
 	 *
 	 * Rely on mmap_read_unlock() for an implicit smp_rmb(), which pairs
 	 * with the smp_wmb() in kvm_mmu_invalidate_end().
@@ -1588,8 +1625,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	mmu_seq = vcpu->kvm->mmu_invalidate_seq;
 	mmap_read_unlock(current->mm);
 
-	pfn = __kvm_faultin_pfn(memslot, gfn, write_fault ? FOLL_WRITE : 0,
-				&writable, &page);
+	pfn = faultin_pfn(kvm, memslot, gfn, write_fault, &writable, &page, is_private);
 	if (pfn == KVM_PFN_ERR_HWPOISON) {
 		kvm_send_hwpoison_signal(hva, vma_shift);
 		return 0;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ebca0ab4c5e2..f059958b98fd 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1864,6 +1864,11 @@ static inline int memslot_id(struct kvm *kvm, gfn_t gfn)
 	return gfn_to_memslot(kvm, gfn)->id;
 }
 
+static inline bool memslot_is_readonly(const struct kvm_memory_slot *slot)
+{
+	return slot->flags & KVM_MEM_READONLY;
+}
+
 static inline gfn_t
 hva_to_gfn_memslot(unsigned long hva, struct kvm_memory_slot *slot)
 {
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9cd6690b7955..10c3168db473 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2622,11 +2622,6 @@ unsigned long kvm_host_page_size(struct kvm_vcpu *vcpu, gfn_t gfn)
 	return size;
 }
 
-static bool memslot_is_readonly(const struct kvm_memory_slot *slot)
-{
-	return slot->flags & KVM_MEM_READONLY;
-}
-
 static unsigned long __gfn_to_hva_many(const struct kvm_memory_slot *slot, gfn_t gfn,
 				       gfn_t *nr_pages, bool write)
 {
-- 
2.48.0.rc2.279.g1de40edade-goog

[RFC PATCH v1 7/9] KVM: arm64: Introduce KVM_VM_TYPE_ARM_SW_PROTECTED machine type

[Fuad Tabba]

Introduce a new virtual machine type,
KVM_VM_TYPE_ARM_SW_PROTECTED, to serve as a development and
testing vehicle for Confidential (CoCo) VMs, similar to the x86
KVM_X86_SW_PROTECTED_VM type.

Initially, this is used to test guest_memfd without needing any
underlying protection.

Similar to the x86 type, this is currently only for development
and testing.  Do not use KVM_VM_TYPE_ARM_SW_PROTECTED for "real"
VMs, and especially not in production. The behavior and effective
ABI for software-protected VMs is unstable.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 Documentation/virt/kvm/api.rst    |  5 +++++
 arch/arm64/include/asm/kvm_host.h | 10 ++++++++++
 arch/arm64/kvm/arm.c              |  5 +++++
 arch/arm64/kvm/mmu.c              |  3 ---
 include/uapi/linux/kvm.h          |  6 ++++++
 5 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index f15b61317aad..7953b07c8c2b 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -214,6 +214,11 @@ exposed by the guest CPUs in ID_AA64MMFR0_EL1[PARange]. It only affects
 size of the address translated by the stage2 level (guest physical to
 host physical address translations).
 
+KVM_VM_TYPE_ARM_SW_PROTECTED is currently only for development and testing of
+confidential VMs without having underlying support. Do not use
+KVM_VM_TYPE_ARM_SW_PROTECTED for "real" VMs, and especially not in production.
+The behavior and effective ABI for software-protected VMs is unstable.
+
 
 4.3 KVM_GET_MSR_INDEX_LIST, KVM_GET_MSR_FEATURE_INDEX_LIST
 ----------------------------------------------------------
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index e18e9244d17a..2fdc7e24ae8e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -380,6 +380,8 @@ struct kvm_arch {
 	 * the associated pKVM instance in the hypervisor.
 	 */
 	struct kvm_protected_vm pkvm;
+
+	unsigned long vm_type;
 };
 
 struct kvm_vcpu_fault_info {
@@ -1529,4 +1531,12 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
 #define kvm_has_s1poe(k)				\
 	(kvm_has_feat((k), ID_AA64MMFR3_EL1, S1POE, IMP))
 
+#define kvm_arch_has_private_mem(kvm)			\
+	(IS_ENABLED(CONFIG_KVM_PRIVATE_MEM) &&		\
+	 ((kvm)->arch.vm_type & KVM_VM_TYPE_ARM_SW_PROTECTED))
+
+#define kvm_arch_private_mem_inplace(kvm)		\
+	(IS_ENABLED(CONFIG_KVM_GMEM_MAPPABLE) &&	\
+	 ((kvm)->arch.vm_type & KVM_VM_TYPE_ARM_SW_PROTECTED))
+
 #endif /* __ARM64_KVM_HOST_H__ */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index a102c3aebdbc..ecdb8db619d8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -171,6 +171,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
 	int ret;
 
+	if (type & ~KVM_VM_TYPE_MASK)
+		return -EINVAL;
+
 	mutex_init(&kvm->arch.config_lock);
 
 #ifdef CONFIG_LOCKDEP
@@ -212,6 +215,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 
 	bitmap_zero(kvm->arch.vcpu_features, KVM_VCPU_MAX_FEATURES);
 
+	kvm->arch.vm_type = type;
+
 	return 0;
 
 err_free_cpumask:
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index adf23618e2a0..b6cbe11dea48 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -869,9 +869,6 @@ static int kvm_init_ipa_range(struct kvm_s2_mmu *mmu, unsigned long type)
 	u64 mmfr0, mmfr1;
 	u32 phys_shift;
 
-	if (type & ~KVM_VM_TYPE_ARM_IPA_SIZE_MASK)
-		return -EINVAL;
-
 	phys_shift = KVM_VM_TYPE_ARM_IPA_SIZE(type);
 	if (is_protected_kvm_enabled()) {
 		phys_shift = kvm_ipa_limit;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 021f8ef9979b..5e10a5903a58 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -656,6 +656,12 @@ struct kvm_enable_cap {
 #define KVM_VM_TYPE_ARM_IPA_SIZE_MASK	0xffULL
 #define KVM_VM_TYPE_ARM_IPA_SIZE(x)		\
 	((x) & KVM_VM_TYPE_ARM_IPA_SIZE_MASK)
+
+#define KVM_VM_TYPE_ARM_SW_PROTECTED	(1UL << 9)
+
+#define KVM_VM_TYPE_MASK	(KVM_VM_TYPE_ARM_IPA_SIZE_MASK | \
+				 KVM_VM_TYPE_ARM_SW_PROTECTED)
+
 /*
  * ioctls for /dev/kvm fds:
  */
-- 
2.48.0.rc2.279.g1de40edade-goog

[RFC PATCH v1 8/9] KVM: guest_memfd: selftests: guest_memfd mmap() test when mapping is allowed

[Fuad Tabba]

Expand the guest_memfd selftests to include testing mapping guest
memory if the capability is supported, and that still checks that
memory is not mappable if the capability isn't supported.

Also, build the guest_memfd selftest for aarch64.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 tools/testing/selftests/kvm/Makefile          |  1 +
 .../testing/selftests/kvm/guest_memfd_test.c  | 60 +++++++++++++++++--
 tools/testing/selftests/kvm/lib/kvm_util.c    |  3 +-
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 41593d2e7de9..c998eb3c3b77 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -174,6 +174,7 @@ TEST_GEN_PROGS_aarch64 += coalesced_io_test
 TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
 TEST_GEN_PROGS_aarch64 += dirty_log_perf_test
+TEST_GEN_PROGS_aarch64 += guest_memfd_test
 TEST_GEN_PROGS_aarch64 += guest_print_test
 TEST_GEN_PROGS_aarch64 += get-reg-list
 TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index ce687f8d248f..6c501cb865f3 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -34,12 +34,55 @@ static void test_file_read_write(int fd)
 		    "pwrite on a guest_mem fd should fail");
 }
 
-static void test_mmap(int fd, size_t page_size)
+static void test_mmap_allowed(int fd, size_t total_size)
 {
+	size_t page_size = getpagesize();
+	char *mem;
+	int ret;
+	int i;
+
+	mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+	TEST_ASSERT(mem != MAP_FAILED, "mmaping() guest memory should pass.");
+
+	memset(mem, 0xaa, total_size);
+	for (i = 0; i < total_size; i++)
+		TEST_ASSERT_EQ(mem[i], 0xaa);
+
+	ret = fallocate(fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE, 0,
+			page_size);
+	TEST_ASSERT(!ret, "fallocate the first page should succeed");
+
+	for (i = 0; i < page_size; i++)
+		TEST_ASSERT_EQ(mem[i], 0x00);
+	for (; i < total_size; i++)
+		TEST_ASSERT_EQ(mem[i], 0xaa);
+
+	memset(mem, 0xaa, total_size);
+	for (i = 0; i < total_size; i++)
+		TEST_ASSERT_EQ(mem[i], 0xaa);
+
+	ret = munmap(mem, total_size);
+	TEST_ASSERT(!ret, "munmap should succeed");
+}
+
+static void test_mmap_denied(int fd, size_t total_size)
+{
+	size_t page_size = getpagesize();
 	char *mem;
 
 	mem = mmap(NULL, page_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
 	TEST_ASSERT_EQ(mem, MAP_FAILED);
+
+	mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+	TEST_ASSERT_EQ(mem, MAP_FAILED);
+}
+
+static void test_mmap(int fd, size_t total_size)
+{
+	if (kvm_has_cap(KVM_CAP_GUEST_MEMFD_MAPPABLE))
+		test_mmap_allowed(fd, total_size);
+	else
+		test_mmap_denied(fd, total_size);
 }
 
 static void test_file_size(int fd, size_t page_size, size_t total_size)
@@ -172,17 +215,24 @@ static void test_create_guest_memfd_multiple(struct kvm_vm *vm)
 
 int main(int argc, char *argv[])
 {
-	size_t page_size;
+	unsigned long type = VM_TYPE_DEFAULT;
+	struct kvm_vm *vm;
 	size_t total_size;
+	size_t page_size;
 	int fd;
-	struct kvm_vm *vm;
 
 	TEST_REQUIRE(kvm_has_cap(KVM_CAP_GUEST_MEMFD));
 
+	if (kvm_has_cap(KVM_CAP_GUEST_MEMFD_MAPPABLE)) {
+#ifdef __aarch64__
+		type = KVM_VM_TYPE_ARM_SW_PROTECTED;
+#endif
+	}
+
 	page_size = getpagesize();
 	total_size = page_size * 4;
 
-	vm = vm_create_barebones();
+	vm = vm_create_barebones_type(type);
 
 	test_create_guest_memfd_invalid(vm);
 	test_create_guest_memfd_multiple(vm);
@@ -190,7 +240,7 @@ int main(int argc, char *argv[])
 	fd = vm_create_guest_memfd(vm, total_size, 0);
 
 	test_file_read_write(fd);
-	test_mmap(fd, page_size);
+	test_mmap(fd, total_size);
 	test_file_size(fd, page_size, total_size);
 	test_fallocate(fd, page_size, total_size);
 	test_invalid_punch_hole(fd, page_size, total_size);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 480e3a40d197..098ea04ec099 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -347,9 +347,8 @@ struct kvm_vm *____vm_create(struct vm_shape shape)
 	}
 
 #ifdef __aarch64__
-	TEST_ASSERT(!vm->type, "ARM doesn't support test-provided types");
 	if (vm->pa_bits != 40)
-		vm->type = KVM_VM_TYPE_ARM_IPA_SIZE(vm->pa_bits);
+		vm->type |= KVM_VM_TYPE_ARM_IPA_SIZE(vm->pa_bits);
 #endif
 
 	vm_open(vm);
-- 
2.48.0.rc2.279.g1de40edade-goog

[RFC PATCH v1 9/9] KVM: arm64: Enable mapping guest_memfd in arm64

[Fuad Tabba]

Enabled mapping guest_memfd in arm64, which would only apply to
VMs with the type KVM_VM_TYPE_ARM_SW_PROTECTED.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index ead632ad01b4..fe3451f244b5 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -38,6 +38,7 @@ menuconfig KVM
 	select HAVE_KVM_VCPU_RUN_PID_CHANGE
 	select SCHED_INFO
 	select GUEST_PERF_EVENTS if PERF_EVENTS
+	select KVM_GMEM_MAPPABLE
 	help
 	  Support hosting virtualized guest machines.
 
-- 
2.48.0.rc2.279.g1de40edade-goog

Re: [RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[David Hildenbrand]

On 22.01.25 16:27, Fuad Tabba wrote:
> The purpose of this series is to serve as a potential base for
> restricted mmap() support for guest_memfd [1]. It would allow
> experimentation with what that support would be like, in the safe
> environment of a new VM type used for testing.
> 
> This series adds a new VM type for arm64,
> KVM_VM_TYPE_ARM_SW_PROTECTED, analogous to the x86
> KVM_X86_SW_PROTECTED_VM. This type is to serve as a development
> and testing vehicle for Confidential (CoCo) VMs.
> 
> Similar to the x86 type, this is currently only for development
> and testing. It's not meant to be used for "real" VMs, and
> especially not in production. The behavior and effective ABI for
> software-protected VMs is unstable.
> 
> This series enables mmap() support for guest_memfd specifically
> for the new software-protected VM type, only when explicitly
> enabled in the config.

Hi!

IIUC, in this series, there is no "private" vs "shared" distinction, 
right? So all pages are mappable, and "conversion" does not exist?

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[David Hildenbrand]

On 22.01.25 16:35, David Hildenbrand wrote:
> On 22.01.25 16:27, Fuad Tabba wrote:
>> The purpose of this series is to serve as a potential base for
>> restricted mmap() support for guest_memfd [1]. It would allow
>> experimentation with what that support would be like, in the safe
>> environment of a new VM type used for testing.
>>
>> This series adds a new VM type for arm64,
>> KVM_VM_TYPE_ARM_SW_PROTECTED, analogous to the x86
>> KVM_X86_SW_PROTECTED_VM. This type is to serve as a development
>> and testing vehicle for Confidential (CoCo) VMs.
>>
>> Similar to the x86 type, this is currently only for development
>> and testing. It's not meant to be used for "real" VMs, and
>> especially not in production. The behavior and effective ABI for
>> software-protected VMs is unstable.
>>
>> This series enables mmap() support for guest_memfd specifically
>> for the new software-protected VM type, only when explicitly
>> enabled in the config.
> 
> Hi!
> 
> IIUC, in this series, there is no "private" vs "shared" distinction,
> right? So all pages are mappable, and "conversion" does not exist?

Ah, I spot:

+#define kvm_arch_private_mem_inplace(kvm)		\
+	(IS_ENABLED(CONFIG_KVM_GMEM_MAPPABLE) &&	\
+	 ((kvm)->arch.vm_type & KVM_VM_TYPE_ARM_SW_PROTECTED))

Which makes me wonder, why don't we need the same way of making sure all 
references/mappings are gone (+ new page type) when doing the shared -> 
private conversion? Or is this somewhere in here where I didn't spot it yet?

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[Fuad Tabba]

Hi David,

On Wed, 22 Jan 2025 at 15:41, David Hildenbrand <david@redhat.com> wrote:
>
> On 22.01.25 16:35, David Hildenbrand wrote:
> > On 22.01.25 16:27, Fuad Tabba wrote:
> >> The purpose of this series is to serve as a potential base for
> >> restricted mmap() support for guest_memfd [1]. It would allow
> >> experimentation with what that support would be like, in the safe
> >> environment of a new VM type used for testing.
> >>
> >> This series adds a new VM type for arm64,
> >> KVM_VM_TYPE_ARM_SW_PROTECTED, analogous to the x86
> >> KVM_X86_SW_PROTECTED_VM. This type is to serve as a development
> >> and testing vehicle for Confidential (CoCo) VMs.
> >>
> >> Similar to the x86 type, this is currently only for development
> >> and testing. It's not meant to be used for "real" VMs, and
> >> especially not in production. The behavior and effective ABI for
> >> software-protected VMs is unstable.
> >>
> >> This series enables mmap() support for guest_memfd specifically
> >> for the new software-protected VM type, only when explicitly
> >> enabled in the config.
> >
> > Hi!
> >
> > IIUC, in this series, there is no "private" vs "shared" distinction,
> > right? So all pages are mappable, and "conversion" does not exist?

You're right. This is a simplified version of my series that allows
mmaping of the new KVM_VM_TYPE_ARM_SW_PROTECTED vms to use for
experimentation.

Cheers,
/fuad

>
> Ah, I spot:
>
> +#define kvm_arch_private_mem_inplace(kvm)              \
> +       (IS_ENABLED(CONFIG_KVM_GMEM_MAPPABLE) &&        \
> +        ((kvm)->arch.vm_type & KVM_VM_TYPE_ARM_SW_PROTECTED))
>
> Which makes me wonder, why don't we need the same way of making sure all
> references/mappings are gone (+ new page type) when doing the shared ->
> private conversion? Or is this somewhere in here where I didn't spot it yet?

This is new to this series. The idea, based on a request from Patrick
Roy, was to have a VM in arm64 we could use to experiment with. Since
it allows the unconditional mmaping, it's only useful for experiments
or for non-confidential VMs that want to use guest_memfd.

This series isn't meant to replace the other one, more to supplement
it and facilitate experimentation while that's going.

Cheers,
/fuad

> --
> Cheers,
>
> David / dhildenb
>

Re: [RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[David Hildenbrand]

On 22.01.25 18:16, Fuad Tabba wrote:
> Hi David,
> 
> On Wed, 22 Jan 2025 at 15:41, David Hildenbrand <david@redhat.com> wrote:
>>
>> On 22.01.25 16:35, David Hildenbrand wrote:
>>> On 22.01.25 16:27, Fuad Tabba wrote:
>>>> The purpose of this series is to serve as a potential base for
>>>> restricted mmap() support for guest_memfd [1]. It would allow
>>>> experimentation with what that support would be like, in the safe
>>>> environment of a new VM type used for testing.
>>>>
>>>> This series adds a new VM type for arm64,
>>>> KVM_VM_TYPE_ARM_SW_PROTECTED, analogous to the x86
>>>> KVM_X86_SW_PROTECTED_VM. This type is to serve as a development
>>>> and testing vehicle for Confidential (CoCo) VMs.
>>>>
>>>> Similar to the x86 type, this is currently only for development
>>>> and testing. It's not meant to be used for "real" VMs, and
>>>> especially not in production. The behavior and effective ABI for
>>>> software-protected VMs is unstable.
>>>>
>>>> This series enables mmap() support for guest_memfd specifically
>>>> for the new software-protected VM type, only when explicitly
>>>> enabled in the config.
>>>
>>> Hi!
>>>
>>> IIUC, in this series, there is no "private" vs "shared" distinction,
>>> right? So all pages are mappable, and "conversion" does not exist?
> 
> You're right. This is a simplified version of my series that allows
> mmaping of the new KVM_VM_TYPE_ARM_SW_PROTECTED vms to use for
> experimentation.
> 
> Cheers,
> /fuad
> 
>>
>> Ah, I spot:
>>
>> +#define kvm_arch_private_mem_inplace(kvm)              \
>> +       (IS_ENABLED(CONFIG_KVM_GMEM_MAPPABLE) &&        \
>> +        ((kvm)->arch.vm_type & KVM_VM_TYPE_ARM_SW_PROTECTED))
>>
>> Which makes me wonder, why don't we need the same way of making sure all
>> references/mappings are gone (+ new page type) when doing the shared ->
>> private conversion? Or is this somewhere in here where I didn't spot it yet?
> 
> This is new to this series. The idea, based on a request from Patrick
> Roy, was to have a VM in arm64 we could use to experiment with. Since
> it allows the unconditional mmaping, it's only useful for experiments
> or for non-confidential VMs that want to use guest_memfd.
> 
> This series isn't meant to replace the other one, more to supplement
> it and facilitate experimentation while that's going.

Heh, so "kvm_arch_private_mem_inplace" in this series means "no 
conversion at all" ? :)

-- 
Cheers,

David / dhildenb

Re: [RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[Fuad Tabba]

On Wed, 22 Jan 2025 at 21:42, David Hildenbrand <david@redhat.com> wrote:
>
> On 22.01.25 18:16, Fuad Tabba wrote:
> > Hi David,
> >
> > On Wed, 22 Jan 2025 at 15:41, David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 22.01.25 16:35, David Hildenbrand wrote:
> >>> On 22.01.25 16:27, Fuad Tabba wrote:
> >>>> The purpose of this series is to serve as a potential base for
> >>>> restricted mmap() support for guest_memfd [1]. It would allow
> >>>> experimentation with what that support would be like, in the safe
> >>>> environment of a new VM type used for testing.
> >>>>
> >>>> This series adds a new VM type for arm64,
> >>>> KVM_VM_TYPE_ARM_SW_PROTECTED, analogous to the x86
> >>>> KVM_X86_SW_PROTECTED_VM. This type is to serve as a development
> >>>> and testing vehicle for Confidential (CoCo) VMs.
> >>>>
> >>>> Similar to the x86 type, this is currently only for development
> >>>> and testing. It's not meant to be used for "real" VMs, and
> >>>> especially not in production. The behavior and effective ABI for
> >>>> software-protected VMs is unstable.
> >>>>
> >>>> This series enables mmap() support for guest_memfd specifically
> >>>> for the new software-protected VM type, only when explicitly
> >>>> enabled in the config.
> >>>
> >>> Hi!
> >>>
> >>> IIUC, in this series, there is no "private" vs "shared" distinction,
> >>> right? So all pages are mappable, and "conversion" does not exist?
> >
> > You're right. This is a simplified version of my series that allows
> > mmaping of the new KVM_VM_TYPE_ARM_SW_PROTECTED vms to use for
> > experimentation.
> >
> > Cheers,
> > /fuad
> >
> >>
> >> Ah, I spot:
> >>
> >> +#define kvm_arch_private_mem_inplace(kvm)              \
> >> +       (IS_ENABLED(CONFIG_KVM_GMEM_MAPPABLE) &&        \
> >> +        ((kvm)->arch.vm_type & KVM_VM_TYPE_ARM_SW_PROTECTED))
> >>
> >> Which makes me wonder, why don't we need the same way of making sure all
> >> references/mappings are gone (+ new page type) when doing the shared ->
> >> private conversion? Or is this somewhere in here where I didn't spot it yet?
> >
> > This is new to this series. The idea, based on a request from Patrick
> > Roy, was to have a VM in arm64 we could use to experiment with. Since
> > it allows the unconditional mmaping, it's only useful for experiments
> > or for non-confidential VMs that want to use guest_memfd.
> >
> > This series isn't meant to replace the other one, more to supplement
> > it and facilitate experimentation while that's going.
>
> Heh, so "kvm_arch_private_mem_inplace" in this series means "no
> conversion at all" ? :)

Yes, just for experimenting with the protected software VM.

Thanks,
/fuad

> --
> Cheers,
>
> David / dhildenb
>

Re: [RFC PATCH v1 0/9] KVM: Mapping of guest_memfd at the host and a software protected VM type

[David Hildenbrand]

On 23.01.25 10:09, Fuad Tabba wrote:
> On Wed, 22 Jan 2025 at 21:42, David Hildenbrand <david@redhat.com> wrote:
>>
>> On 22.01.25 18:16, Fuad Tabba wrote:
>>> Hi David,
>>>
>>> On Wed, 22 Jan 2025 at 15:41, David Hildenbrand <david@redhat.com> wrote:
>>>>
>>>> On 22.01.25 16:35, David Hildenbrand wrote:
>>>>> On 22.01.25 16:27, Fuad Tabba wrote:
>>>>>> The purpose of this series is to serve as a potential base for
>>>>>> restricted mmap() support for guest_memfd [1]. It would allow
>>>>>> experimentation with what that support would be like, in the safe
>>>>>> environment of a new VM type used for testing.
>>>>>>
>>>>>> This series adds a new VM type for arm64,
>>>>>> KVM_VM_TYPE_ARM_SW_PROTECTED, analogous to the x86
>>>>>> KVM_X86_SW_PROTECTED_VM. This type is to serve as a development
>>>>>> and testing vehicle for Confidential (CoCo) VMs.
>>>>>>
>>>>>> Similar to the x86 type, this is currently only for development
>>>>>> and testing. It's not meant to be used for "real" VMs, and
>>>>>> especially not in production. The behavior and effective ABI for
>>>>>> software-protected VMs is unstable.
>>>>>>
>>>>>> This series enables mmap() support for guest_memfd specifically
>>>>>> for the new software-protected VM type, only when explicitly
>>>>>> enabled in the config.
>>>>>
>>>>> Hi!
>>>>>
>>>>> IIUC, in this series, there is no "private" vs "shared" distinction,
>>>>> right? So all pages are mappable, and "conversion" does not exist?
>>>
>>> You're right. This is a simplified version of my series that allows
>>> mmaping of the new KVM_VM_TYPE_ARM_SW_PROTECTED vms to use for
>>> experimentation.
>>>
>>> Cheers,
>>> /fuad
>>>
>>>>
>>>> Ah, I spot:
>>>>
>>>> +#define kvm_arch_private_mem_inplace(kvm)              \
>>>> +       (IS_ENABLED(CONFIG_KVM_GMEM_MAPPABLE) &&        \
>>>> +        ((kvm)->arch.vm_type & KVM_VM_TYPE_ARM_SW_PROTECTED))
>>>>
>>>> Which makes me wonder, why don't we need the same way of making sure all
>>>> references/mappings are gone (+ new page type) when doing the shared ->
>>>> private conversion? Or is this somewhere in here where I didn't spot it yet?
>>>
>>> This is new to this series. The idea, based on a request from Patrick
>>> Roy, was to have a VM in arm64 we could use to experiment with. Since
>>> it allows the unconditional mmaping, it's only useful for experiments
>>> or for non-confidential VMs that want to use guest_memfd.
>>>
>>> This series isn't meant to replace the other one, more to supplement
>>> it and facilitate experimentation while that's going.
>>
>> Heh, so "kvm_arch_private_mem_inplace" in this series means "no
>> conversion at all" ? :)
> 
> Yes, just for experimenting with the protected software VM.

Got it, thanks!

-- 
Cheers,

David / dhildenb