| Message ID | 20250328153133.3504118-8-tabba@google.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | KVM: Restricted mapping of guest_memfd at the host and arm64 support | expand |
Hi Ackerley, On Thu, 3 Apr 2025 at 00:03, Ackerley Tng <ackerleytng@google.com> wrote: > > Fuad Tabba <tabba@google.com> writes: > > > Not all use cases require guest_memfd() to be shared with the host when > > first created. Add a new flag, GUEST_MEMFD_FLAG_INIT_SHARED, which when > > set on KVM_CREATE_GUEST_MEMFD initializes the memory as shared with the > > host, and therefore mappable by it. Otherwise, memory is private until > > explicitly shared by the guest with the host. > > > > Signed-off-by: Fuad Tabba <tabba@google.com> > > --- > > Documentation/virt/kvm/api.rst | 4 ++++ > > include/uapi/linux/kvm.h | 1 + > > tools/testing/selftests/kvm/guest_memfd_test.c | 7 +++++-- > > virt/kvm/guest_memfd.c | 12 ++++++++++++ > > 4 files changed, 22 insertions(+), 2 deletions(-) > > > > <snip> > > > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > > index eec9d5e09f09..32e149478b04 100644 > > --- a/virt/kvm/guest_memfd.c > > +++ b/virt/kvm/guest_memfd.c > > @@ -1069,6 +1069,15 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) > > goto err_gmem; > > } > > > > + if (IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM) && > > + (flags & GUEST_MEMFD_FLAG_INIT_SHARED)) { > > + err = kvm_gmem_offset_range_set_shared(file_inode(file), 0, size >> PAGE_SHIFT); > > I think if GUEST_MEMFD_FLAG_INIT_SHARED is not set, we should call > kvm_gmem_offset_range_clear_shared(); so that there is always some > shareability defined for all offsets in a file. Otherwise, when reading > shareability, we'd have to check against GUEST_MEMFD_FLAG_INIT_SHARED > to find out what to initialize it to. Ack. Thanks! /fuad > > + if (err) { > > + fput(file); > > + goto err_gmem; > > + } > > + } > > + > > kvm_get_kvm(kvm); > > gmem->kvm = kvm; > > xa_init(&gmem->bindings); > > @@ -1090,6 +1099,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args) > > u64 flags = args->flags; > > u64 valid_flags = 0; > > > > + if (IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM)) > > + valid_flags |= GUEST_MEMFD_FLAG_INIT_SHARED; > > + > > if (flags & ~valid_flags) > > return -EINVAL;
Hi Ackerley, On Wed, 2 Apr 2025 at 23:47, Ackerley Tng <ackerleytng@google.com> wrote: > > Fuad Tabba <tabba@google.com> writes: > > > Not all use cases require guest_memfd() to be shared with the host when > > first created. Add a new flag, GUEST_MEMFD_FLAG_INIT_SHARED, which when > > set on KVM_CREATE_GUEST_MEMFD initializes the memory as shared with the > > host, and therefore mappable by it. Otherwise, memory is private until > > explicitly shared by the guest with the host. > > > > Signed-off-by: Fuad Tabba <tabba@google.com> > > --- > > Documentation/virt/kvm/api.rst | 4 ++++ > > include/uapi/linux/kvm.h | 1 + > > tools/testing/selftests/kvm/guest_memfd_test.c | 7 +++++-- > > virt/kvm/guest_memfd.c | 12 ++++++++++++ > > 4 files changed, 22 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > > index 2b52eb77e29c..a5496d7d323b 100644 > > --- a/Documentation/virt/kvm/api.rst > > +++ b/Documentation/virt/kvm/api.rst > > @@ -6386,6 +6386,10 @@ most one mapping per page, i.e. binding multiple memory regions to a single > > guest_memfd range is not allowed (any number of memory regions can be bound to > > a single guest_memfd file, but the bound ranges must not overlap). > > > > +If the capability KVM_CAP_GMEM_SHARED_MEM is supported, then the flags field > > +supports GUEST_MEMFD_FLAG_INIT_SHARED, which initializes the memory as shared > > +with the host, and thereby, mappable by it. > > + > > See KVM_SET_USER_MEMORY_REGION2 for additional details. > > > > 4.143 KVM_PRE_FAULT_MEMORY > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > > index 117937a895da..22d7e33bf09c 100644 > > --- a/include/uapi/linux/kvm.h > > +++ b/include/uapi/linux/kvm.h > > @@ -1566,6 +1566,7 @@ struct kvm_memory_attributes { > > #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3) > > > > #define KVM_CREATE_GUEST_MEMFD _IOWR(KVMIO, 0xd4, struct kvm_create_guest_memfd) > > +#define GUEST_MEMFD_FLAG_INIT_SHARED (1UL << 0) > > > > struct kvm_create_guest_memfd { > > __u64 size; > > diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c > > index 38c501e49e0e..4a7fcd6aa372 100644 > > --- a/tools/testing/selftests/kvm/guest_memfd_test.c > > +++ b/tools/testing/selftests/kvm/guest_memfd_test.c > > @@ -159,7 +159,7 @@ static void test_invalid_punch_hole(int fd, size_t page_size, size_t total_size) > > static void test_create_guest_memfd_invalid(struct kvm_vm *vm) > > { > > size_t page_size = getpagesize(); > > - uint64_t flag; > > + uint64_t flag = BIT(0); > > size_t size; > > int fd; > > > > @@ -170,7 +170,10 @@ static void test_create_guest_memfd_invalid(struct kvm_vm *vm) > > size); > > } > > > > - for (flag = BIT(0); flag; flag <<= 1) { > > + if (kvm_has_cap(KVM_CAP_GMEM_SHARED_MEM)) > > + flag = GUEST_MEMFD_FLAG_INIT_SHARED << 1; > > + > > + for (; flag; flag <<= 1) { > > This would end up shifting the GUEST_MEMFD_FLAG_INIT_SHARED flag for > each loop. Yes. That's my intention. This tests whether the flags are invalid. Without GUEST_MEMFD_FLAG_INIT_SHARED, no flag is valid, so it starts with bit zero and goes through all the flags. If KVM_CAP_GMEM_SHARED_MEM is available, then we want to start from bit 1 (i.e., skip bit 0, which is GUEST_MEMFD_FLAG_INIT_SHARED). Cheers, /fuad > > fd = __vm_create_guest_memfd(vm, page_size, flag); > > TEST_ASSERT(fd == -1 && errno == EINVAL, > > "guest_memfd() with flag '0x%lx' should fail with EINVAL", > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > > index eec9d5e09f09..32e149478b04 100644 > > --- a/virt/kvm/guest_memfd.c > > +++ b/virt/kvm/guest_memfd.c > > @@ -1069,6 +1069,15 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) > > goto err_gmem; > > } > > > > + if (IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM) && > > + (flags & GUEST_MEMFD_FLAG_INIT_SHARED)) { > > + err = kvm_gmem_offset_range_set_shared(file_inode(file), 0, size >> PAGE_SHIFT); > > + if (err) { > > + fput(file); > > + goto err_gmem; > > + } > > + } > > + > > kvm_get_kvm(kvm); > > gmem->kvm = kvm; > > xa_init(&gmem->bindings); > > @@ -1090,6 +1099,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args) > > u64 flags = args->flags; > > u64 valid_flags = 0; > > > > + if (IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM)) > > + valid_flags |= GUEST_MEMFD_FLAG_INIT_SHARED; > > + > > if (flags & ~valid_flags) > > return -EINVAL;
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 2b52eb77e29c..a5496d7d323b 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -6386,6 +6386,10 @@ most one mapping per page, i.e. binding multiple memory regions to a single guest_memfd range is not allowed (any number of memory regions can be bound to a single guest_memfd file, but the bound ranges must not overlap). +If the capability KVM_CAP_GMEM_SHARED_MEM is supported, then the flags field +supports GUEST_MEMFD_FLAG_INIT_SHARED, which initializes the memory as shared +with the host, and thereby, mappable by it. + See KVM_SET_USER_MEMORY_REGION2 for additional details. 4.143 KVM_PRE_FAULT_MEMORY diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 117937a895da..22d7e33bf09c 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1566,6 +1566,7 @@ struct kvm_memory_attributes { #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3) #define KVM_CREATE_GUEST_MEMFD _IOWR(KVMIO, 0xd4, struct kvm_create_guest_memfd) +#define GUEST_MEMFD_FLAG_INIT_SHARED (1UL << 0) struct kvm_create_guest_memfd { __u64 size; diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c index 38c501e49e0e..4a7fcd6aa372 100644 --- a/tools/testing/selftests/kvm/guest_memfd_test.c +++ b/tools/testing/selftests/kvm/guest_memfd_test.c @@ -159,7 +159,7 @@ static void test_invalid_punch_hole(int fd, size_t page_size, size_t total_size) static void test_create_guest_memfd_invalid(struct kvm_vm *vm) { size_t page_size = getpagesize(); - uint64_t flag; + uint64_t flag = BIT(0); size_t size; int fd; @@ -170,7 +170,10 @@ static void test_create_guest_memfd_invalid(struct kvm_vm *vm) size); } - for (flag = BIT(0); flag; flag <<= 1) { + if (kvm_has_cap(KVM_CAP_GMEM_SHARED_MEM)) + flag = GUEST_MEMFD_FLAG_INIT_SHARED << 1; + + for (; flag; flag <<= 1) { fd = __vm_create_guest_memfd(vm, page_size, flag); TEST_ASSERT(fd == -1 && errno == EINVAL, "guest_memfd() with flag '0x%lx' should fail with EINVAL", diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index eec9d5e09f09..32e149478b04 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -1069,6 +1069,15 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) goto err_gmem; } + if (IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM) && + (flags & GUEST_MEMFD_FLAG_INIT_SHARED)) { + err = kvm_gmem_offset_range_set_shared(file_inode(file), 0, size >> PAGE_SHIFT); + if (err) { + fput(file); + goto err_gmem; + } + } + kvm_get_kvm(kvm); gmem->kvm = kvm; xa_init(&gmem->bindings); @@ -1090,6 +1099,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args) u64 flags = args->flags; u64 valid_flags = 0; + if (IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM)) + valid_flags |= GUEST_MEMFD_FLAG_INIT_SHARED; + if (flags & ~valid_flags) return -EINVAL;
Not all use cases require guest_memfd() to be shared with the host when first created. Add a new flag, GUEST_MEMFD_FLAG_INIT_SHARED, which when set on KVM_CREATE_GUEST_MEMFD initializes the memory as shared with the host, and therefore mappable by it. Otherwise, memory is private until explicitly shared by the guest with the host. Signed-off-by: Fuad Tabba <tabba@google.com> --- Documentation/virt/kvm/api.rst | 4 ++++ include/uapi/linux/kvm.h | 1 + tools/testing/selftests/kvm/guest_memfd_test.c | 7 +++++-- virt/kvm/guest_memfd.c | 12 ++++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-)