Re: [PATCH v7 4/9] KVM: guest_memfd: Handle in-place shared memory as guest_memfd backed memory

[Patrick Roy <roypat@amazon.co.uk>]

Hi!

On Mon, 2025-04-14 at 12:51 +0100, David Hildenbrand wrote:
>

[...]

> On top of that, I was wondering if we could look into doing something like
> the following. It would also allow for pulling pages out of gmem for
> existing SW-protected VMs once they enable shared memory for GMEM IIUC.
> 
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 08eebd24a0e18..6f878cab0f466 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4495,11 +4495,6 @@ static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
>  {
>         int max_order, r;
> 
> -       if (!kvm_slot_has_gmem(fault->slot)) {
> -               kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
> -               return -EFAULT;
> -       }
> -
>         r = kvm_gmem_get_pfn(vcpu->kvm, fault->slot, fault->gfn, &fault->pfn,
>                              &fault->refcounted_page, &max_order);
>         if (r) {
> @@ -4518,8 +4513,19 @@ static int __kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
>                                  struct kvm_page_fault *fault)
>  {
>         unsigned int foll = fault->write ? FOLL_WRITE : 0;
> +       bool use_gmem = false;
> +
> +       if (fault->is_private) {
> +               if (!kvm_slot_has_gmem(fault->slot)) {
> +                       kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
> +                       return -EFAULT;
> +               }
> +               use_gmem = true;
> +       } else if (kvm_slot_has_gmem_with_shared(fault->slot)) {
> +               use_gmem = true;
> +       }
> 
> -       if (fault->is_private)
> +       if (use_gmem)
>                 return kvm_mmu_faultin_pfn_gmem(vcpu, fault);
> 
>         foll |= FOLL_NOWAIT;
> 
> 
> That is, we'd not claim that things are private when they are not, but instead
> teach the code about shared memory coming from gmem.
> 
> There might be some more missing, just throwing it out there if I am completely off.

I think I arrived at the need for this as well while experimenting with
building a Firecracker version that works with my direct map removal
patches.

With this patch series, on ARM, as soon as a memslot has a guest_memfd
associated with it, all guest faults go through kvm_gmem_get_pfn, but on
x86, they go through slot->userspace_addr by default, as
CONFIG_KVM_SW_PROTECTED_VM selects CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES.
There's no real difference between these if slot->userspace_addr can be
GUP'd, but if its a VMA of a guest_memfd without direct map entries,
faulting through slot->userspace_addr wont work. So on x86 Firecracker
has to formally set the memory attributes to private, while on ARM it
doesn't [1], which is a bit awkward.

David, I couldn't find an implementation of
kvm_slot_has_gmem_with_shared() in the branch you shared, but would it
be something like "slot->userspace_addr points to a gmem VMA,
particularly to a VMA of the gmem that's associated with this memslot,
mapped at the same offset"?

Best, 
Patrick

[1]: https://github.com/firecracker-microvm/firecracker/blob/feature/secret-hiding/src/vmm/src/builder.rs#L268

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