Re: [PATCH v13 13/22] KVM: selftests: Set first memory region as shared if guest_memfd

[Ackerley Tng]

Lisa Wang <wyi...@google.com> writes:

> Set the initial state of the first memory region as shared if it is
> backed by guest_memfd, so that the KVM selftest framework functions can
> populate mmap()-ed guest_memfd memory the same way memory from other
> memory providers are populated.
>
> For CoCo VMs, pages that need to be private are explicitly set to
> private before executing the VM.
>
>
> [...snip...]
>
> @@ -495,14 +497,16 @@ struct kvm_vm *__vm_create(struct vm_shape shape, u32 
> nr_runnable_vcpus,
>       vm = ____vm_create(shape);
>
>       /*
> -      * Force GUEST_MEMFD for the primary memory region if necessary, e.g.
> -      * for CoCo VMs that require GUEST_MEMFD backed private memory.
> +      * Force GUEST_MEMFD for the primary memory region if necessary, and
> +      * initialize it as shared so the selftest framework can populate it
> +      * exactly like other memory providers.
>        */
> -     flags = 0;
> -     if (is_guest_memfd_required(shape))
> +     if (is_guest_memfd_required(shape)) {
>               flags |= KVM_MEM_GUEST_MEMFD;
> +             gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
> +     }
>

Just noticed this while hacking some SNP tests.

> -     vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, 
> flags);
> +     vm_mem_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, flags, -1, 0, 
> gmem_flags);
>       for (i = 0; i < NR_MEM_REGIONS; i++)
>               vm->memslots[i] = 0;
>
>
> --
> 2.54.0.746.g67dd491aae-goog

I think this patch should fully buy into in-place conversions, so we
need to also set GUEST_MEMFD_FLAG_MMAP:

@@ -483,6 +483,7 @@ struct kvm_vm *__vm_create(struct vm_shape shape,
u32 nr_runnable_vcpus,
 {
        u64 nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
                                                 nr_extra_pages);
+       enum vm_mem_backing_src_type src_type = VM_MEM_SRC_ANONYMOUS;
        struct userspace_mem_region *slot0;
        u64 gmem_flags = 0;
        struct kvm_vm *vm;
@@ -503,10 +504,16 @@ struct kvm_vm *__vm_create(struct vm_shape
shape, u32 nr_runnable_vcpus,
         */
        if (is_guest_memfd_required(shape)) {
                flags |= KVM_MEM_GUEST_MEMFD;
-               gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
+               gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED | 
GUEST_MEMFD_FLAG_MMAP;
+               /*
+                * TODO: Clean this up together with vm_mem_add(). Use
+                * VM_MEM_SRC_SHMEM to tell vm_mem_add() to mmap
+                * guest_memfd with MAP_SHARED.
+                */
+               src_type = VM_MEM_SRC_SHMEM;
        }

-       vm_mem_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, flags, -1, 0,
gmem_flags);
+       vm_mem_add(vm, src_type, 0, 0, nr_pages, flags, -1, 0, gmem_flags);
        for (i = 0; i < NR_MEM_REGIONS; i++)
                vm->memslots[i] = 0;

The VM_MEM_SRC_SHMEM is a bit of a hack but imo that refactor should go
with another series to clean up vm_mem_add().

The above code was working for TDX because vm_mem_add() without the
guest_memfd MMAP flag would leave region->fd as -1. Without
setting src_type to VM_MEM_SRC_ANONYMOUS, the assertion in vm_mem_add()

  region->fd == -1 || backing_src_is_shared(src_type)

would pass.

tdx_init_mem_region() was called with the anonymously mmap()-ed address,
which turns out fine.

With the above change, we would also need to call tdx_init_mem_region()
with NULL for source_address.