Add a guest_memfd flag to allow userspace to state that the underlying
memory should be configured to be shared by default, and reject user page
faults if the guest_memfd instance's memory isn't shared by default.
Because KVM doesn't yet support in-place private<=>shared conversions, all
guest_memfd memory effectively follows the default state.
Alternatively, KVM could deduce the default state based on MMAP, which for
all intents and purposes is what KVM currently does. However, implicitly
deriving the default state based on MMAP will result in a messy ABI when
support for in-place conversions is added.
For x86 CoCo VMs, which don't yet support MMAP, memory is currently private
by default (otherwise the memory would be unusable). If MMAP implies
memory is shared by default, then the default state for CoCo VMs will vary
based on MMAP, and from userspace's perspective, will change when in-place
conversion support is added. I.e. to maintain guest<=>host ABI, userspace
would need to immediately convert all memory from shared=>private, which
is both ugly and inefficient. The inefficiency could be avoided by adding
a flag to state that memory is _private_ by default, irrespective of MMAP,
but that would lead to an equally messy and hard to document ABI.
Bite the bullet and immediately add a flag to control the default state so
that the effective behavior is explicit and straightforward.
Fixes: 3d3a04fad25a ("KVM: Allow and advertise support for host mmap() on guest_memfd files")
Cc: David Hildenbrand <david@redhat.com>
Cc: Fuad Tabba <tabba@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Documentation/virt/kvm/api.rst | 10 ++++++++--
include/uapi/linux/kvm.h | 3 ++-
tools/testing/selftests/kvm/guest_memfd_test.c | 5 +++--
virt/kvm/guest_memfd.c | 6 +++++-
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index c17a87a0a5ac..4dfe156bbe3c 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6415,8 +6415,14 @@ 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).
When the capability KVM_CAP_GUEST_MEMFD_MMAP is supported, the 'flags' field
-supports GUEST_MEMFD_FLAG_MMAP. Setting this flag on guest_memfd creation
-enables mmap() and faulting of guest_memfd memory to host userspace.
+supports GUEST_MEMFD_FLAG_MMAP and GUEST_MEMFD_FLAG_DEFAULT_SHARED. Setting
+the MMAP flag on guest_memfd creation enables mmap() and faulting of guest_memfd
+memory to host userspace (so long as the memory is currently shared). Setting
+DEFAULT_SHARED makes all guest_memfd memory shared by default (versus private
+by default). Note! Because KVM doesn't yet support in-place private<=>shared
+conversions, DEFAULT_SHARED must be specified in order to fault memory into
+userspace page tables. This limitation will go away when in-place conversions
+are supported.
When the KVM MMU performs a PFN lookup to service a guest fault and the backing
guest_memfd has the GUEST_MEMFD_FLAG_MMAP set, then the fault will always be
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 6efa98a57ec1..38a2c083b6aa 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1599,7 +1599,8 @@ 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_MMAP (1ULL << 0)
+#define GUEST_MEMFD_FLAG_MMAP (1ULL << 0)
+#define GUEST_MEMFD_FLAG_DEFAULT_SHARED (1ULL << 1)
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 b3ca6737f304..81b11a958c7a 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -274,7 +274,7 @@ static void test_guest_memfd(unsigned long vm_type)
vm = vm_create_barebones_type(vm_type);
if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
- flags |= GUEST_MEMFD_FLAG_MMAP;
+ flags |= GUEST_MEMFD_FLAG_MMAP | GUEST_MEMFD_FLAG_DEFAULT_SHARED;
test_create_guest_memfd_multiple(vm);
test_create_guest_memfd_invalid_sizes(vm, flags, page_size);
@@ -337,7 +337,8 @@ static void test_guest_memfd_guest(void)
"Default VM type should always support guest_memfd mmap()");
size = vm->page_size;
- fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP);
+ fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP |
+ GUEST_MEMFD_FLAG_DEFAULT_SHARED);
vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 08a6bc7d25b6..19f05a45be04 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -328,6 +328,9 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode))
return VM_FAULT_SIGBUS;
+ if (!((u64)inode->i_private & GUEST_MEMFD_FLAG_DEFAULT_SHARED))
+ return VM_FAULT_SIGBUS;
+
folio = kvm_gmem_get_folio(inode, vmf->pgoff);
if (IS_ERR(folio)) {
int err = PTR_ERR(folio);
@@ -525,7 +528,8 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
u64 valid_flags = 0;
if (kvm_arch_supports_gmem_mmap(kvm))
- valid_flags |= GUEST_MEMFD_FLAG_MMAP;
+ valid_flags |= GUEST_MEMFD_FLAG_MMAP |
+ GUEST_MEMFD_FLAG_DEFAULT_SHARED;
if (flags & ~valid_flags)
return -EINVAL;
--
2.51.0.536.g15c5d4f767-googHi Sean,
On Fri, 26 Sept 2025 at 17:31, Sean Christopherson <seanjc@google.com> wrote:
>
> Add a guest_memfd flag to allow userspace to state that the underlying
> memory should be configured to be shared by default, and reject user page
> faults if the guest_memfd instance's memory isn't shared by default.
> Because KVM doesn't yet support in-place private<=>shared conversions, all
> guest_memfd memory effectively follows the default state.
>
> Alternatively, KVM could deduce the default state based on MMAP, which for
> all intents and purposes is what KVM currently does. However, implicitly
> deriving the default state based on MMAP will result in a messy ABI when
> support for in-place conversions is added.
>
> For x86 CoCo VMs, which don't yet support MMAP, memory is currently private
> by default (otherwise the memory would be unusable). If MMAP implies
> memory is shared by default, then the default state for CoCo VMs will vary
> based on MMAP, and from userspace's perspective, will change when in-place
> conversion support is added. I.e. to maintain guest<=>host ABI, userspace
> would need to immediately convert all memory from shared=>private, which
> is both ugly and inefficient. The inefficiency could be avoided by adding
> a flag to state that memory is _private_ by default, irrespective of MMAP,
> but that would lead to an equally messy and hard to document ABI.
>
> Bite the bullet and immediately add a flag to control the default state so
> that the effective behavior is explicit and straightforward.
>
> Fixes: 3d3a04fad25a ("KVM: Allow and advertise support for host mmap() on guest_memfd files")
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Fuad Tabba <tabba@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> Documentation/virt/kvm/api.rst | 10 ++++++++--
> include/uapi/linux/kvm.h | 3 ++-
> tools/testing/selftests/kvm/guest_memfd_test.c | 5 +++--
> virt/kvm/guest_memfd.c | 6 +++++-
> 4 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index c17a87a0a5ac..4dfe156bbe3c 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6415,8 +6415,14 @@ 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).
>
> When the capability KVM_CAP_GUEST_MEMFD_MMAP is supported, the 'flags' field
> -supports GUEST_MEMFD_FLAG_MMAP. Setting this flag on guest_memfd creation
> -enables mmap() and faulting of guest_memfd memory to host userspace.
> +supports GUEST_MEMFD_FLAG_MMAP and GUEST_MEMFD_FLAG_DEFAULT_SHARED. Setting
There's an extra space between `and` and `GUEST_MEMFD_FLAG_DEFAULT_SHARED`.
> +the MMAP flag on guest_memfd creation enables mmap() and faulting of guest_memfd
> +memory to host userspace (so long as the memory is currently shared). Setting
> +DEFAULT_SHARED makes all guest_memfd memory shared by default (versus private
> +by default). Note! Because KVM doesn't yet support in-place private<=>shared
> +conversions, DEFAULT_SHARED must be specified in order to fault memory into
> +userspace page tables. This limitation will go away when in-place conversions
> +are supported.
I think that a more accurate (and future proof) description of the
mmap flag could be something along the lines of:
+ Setting GUEST_MEMFD_FLAG_MMAP enables using mmap() on the file descriptor.
+ Setting GUEST_MEMFD_FLAG_DEFAULT_SHARED makes all memory in the file shared
+ by default, as opposed to private. Shared memory can be faulted into host
+ userspace page tables. Private memory cannot.
> When the KVM MMU performs a PFN lookup to service a guest fault and the backing
> guest_memfd has the GUEST_MEMFD_FLAG_MMAP set, then the fault will always be
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 6efa98a57ec1..38a2c083b6aa 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1599,7 +1599,8 @@ 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_MMAP (1ULL << 0)
> +#define GUEST_MEMFD_FLAG_MMAP (1ULL << 0)
> +#define GUEST_MEMFD_FLAG_DEFAULT_SHARED (1ULL << 1)
>
> 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 b3ca6737f304..81b11a958c7a 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -274,7 +274,7 @@ static void test_guest_memfd(unsigned long vm_type)
> vm = vm_create_barebones_type(vm_type);
>
> if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
> - flags |= GUEST_MEMFD_FLAG_MMAP;
> + flags |= GUEST_MEMFD_FLAG_MMAP | GUEST_MEMFD_FLAG_DEFAULT_SHARED;
>
> test_create_guest_memfd_multiple(vm);
> test_create_guest_memfd_invalid_sizes(vm, flags, page_size);
> @@ -337,7 +337,8 @@ static void test_guest_memfd_guest(void)
> "Default VM type should always support guest_memfd mmap()");
>
> size = vm->page_size;
> - fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP);
> + fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP |
> + GUEST_MEMFD_FLAG_DEFAULT_SHARED);
> vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
>
> mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 08a6bc7d25b6..19f05a45be04 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -328,6 +328,9 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
> if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode))
> return VM_FAULT_SIGBUS;
>
> + if (!((u64)inode->i_private & GUEST_MEMFD_FLAG_DEFAULT_SHARED))
> + return VM_FAULT_SIGBUS;
> +
> folio = kvm_gmem_get_folio(inode, vmf->pgoff);
> if (IS_ERR(folio)) {
> int err = PTR_ERR(folio);
> @@ -525,7 +528,8 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
> u64 valid_flags = 0;
>
> if (kvm_arch_supports_gmem_mmap(kvm))
> - valid_flags |= GUEST_MEMFD_FLAG_MMAP;
> + valid_flags |= GUEST_MEMFD_FLAG_MMAP |
> + GUEST_MEMFD_FLAG_DEFAULT_SHARED;
At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and
GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth
checking for that, at least until we have in-place conversion? Having
only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP,
isn't a useful combination.
That said, these are all nits, I'll leave it to you. With that:
Reviewed-by: Fuad Tabba <tabba@google.com>
Tested-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
>
> if (flags & ~valid_flags)
> return -EINVAL;
> --
> 2.51.0.536.g15c5d4f767-goog
>
Fuad Tabba <tabba@google.com> writes:
> Hi Sean,
>
> On Fri, 26 Sept 2025 at 17:31, Sean Christopherson <seanjc@google.com> wrote:
>>
>> Add a guest_memfd flag to allow userspace to state that the underlying
>> memory should be configured to be shared by default, and reject user page
>> faults if the guest_memfd instance's memory isn't shared by default.
>> Because KVM doesn't yet support in-place private<=>shared conversions, all
>> guest_memfd memory effectively follows the default state.
>>
>> Alternatively, KVM could deduce the default state based on MMAP, which for
>> all intents and purposes is what KVM currently does. However, implicitly
>> deriving the default state based on MMAP will result in a messy ABI when
>> support for in-place conversions is added.
>>
>> For x86 CoCo VMs, which don't yet support MMAP, memory is currently private
>> by default (otherwise the memory would be unusable). If MMAP implies
>> memory is shared by default, then the default state for CoCo VMs will vary
>> based on MMAP, and from userspace's perspective, will change when in-place
>> conversion support is added. I.e. to maintain guest<=>host ABI, userspace
>> would need to immediately convert all memory from shared=>private, which
>> is both ugly and inefficient. The inefficiency could be avoided by adding
>> a flag to state that memory is _private_ by default, irrespective of MMAP,
>> but that would lead to an equally messy and hard to document ABI.
>>
>> Bite the bullet and immediately add a flag to control the default state so
>> that the effective behavior is explicit and straightforward.
>>
I like having this flag, but didn't propose this because I thought folks
depending on the default being shared (Patrick/Nikita) might have their
usage broken.
>> Fixes: 3d3a04fad25a ("KVM: Allow and advertise support for host mmap() on guest_memfd files")
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Fuad Tabba <tabba@google.com>
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>> ---
>> Documentation/virt/kvm/api.rst | 10 ++++++++--
>> include/uapi/linux/kvm.h | 3 ++-
>> tools/testing/selftests/kvm/guest_memfd_test.c | 5 +++--
>> virt/kvm/guest_memfd.c | 6 +++++-
>> 4 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
>> index c17a87a0a5ac..4dfe156bbe3c 100644
>> --- a/Documentation/virt/kvm/api.rst
>> +++ b/Documentation/virt/kvm/api.rst
>> @@ -6415,8 +6415,14 @@ 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).
>>
>> When the capability KVM_CAP_GUEST_MEMFD_MMAP is supported, the 'flags' field
>> -supports GUEST_MEMFD_FLAG_MMAP. Setting this flag on guest_memfd creation
>> -enables mmap() and faulting of guest_memfd memory to host userspace.
>> +supports GUEST_MEMFD_FLAG_MMAP and GUEST_MEMFD_FLAG_DEFAULT_SHARED. Setting
>
> There's an extra space between `and` and `GUEST_MEMFD_FLAG_DEFAULT_SHARED`.
>
+1 on this. Also, would you consider putting the concept of "at creation
time" or "at initialization time" into the name of the flag?
"Default" could be interpreted as "whenever a folio is allocated for
this guest_memfd", the memory the folio represents is by default
shared.
What we want to represent is that when the guest_memfd is created,
memory at all indices are initialized as shared.
Looking a bit further, when conversion is supported, if this flag is not
specified, then all the indices are initialized as private, right?
>> +the MMAP flag on guest_memfd creation enables mmap() and faulting of guest_memfd
>> +memory to host userspace (so long as the memory is currently shared). Setting
>> +DEFAULT_SHARED makes all guest_memfd memory shared by default (versus private
>> +by default). Note! Because KVM doesn't yet support in-place private<=>shared
>> +conversions, DEFAULT_SHARED must be specified in order to fault memory into
>> +userspace page tables. This limitation will go away when in-place conversions
>> +are supported.
>
> I think that a more accurate (and future proof) description of the
> mmap flag could be something along the lines of:
>
+1 on these suggestions, I agree that making the concepts of SHARED vs
MMAP orthogonal from the start is more future proof.
> + Setting GUEST_MEMFD_FLAG_MMAP enables using mmap() on the file descriptor.
>
> + Setting GUEST_MEMFD_FLAG_DEFAULT_SHARED makes all memory in the file shared
> + by default
See above, I'd prefer clarifying this as "at initialization time" or
something similar.
> , as opposed to private. Shared memory can be faulted into host
> + userspace page tables. Private memory cannot.
>
>> When the KVM MMU performs a PFN lookup to service a guest fault and the backing
>> guest_memfd has the GUEST_MEMFD_FLAG_MMAP set, then the fault will always be
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 6efa98a57ec1..38a2c083b6aa 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1599,7 +1599,8 @@ 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_MMAP (1ULL << 0)
>> +#define GUEST_MEMFD_FLAG_MMAP (1ULL << 0)
>> +#define GUEST_MEMFD_FLAG_DEFAULT_SHARED (1ULL << 1)
>>
>> 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 b3ca6737f304..81b11a958c7a 100644
>> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
>> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
>> @@ -274,7 +274,7 @@ static void test_guest_memfd(unsigned long vm_type)
>> vm = vm_create_barebones_type(vm_type);
>>
>> if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
>> - flags |= GUEST_MEMFD_FLAG_MMAP;
>> + flags |= GUEST_MEMFD_FLAG_MMAP | GUEST_MEMFD_FLAG_DEFAULT_SHARED;
>>
>> test_create_guest_memfd_multiple(vm);
>> test_create_guest_memfd_invalid_sizes(vm, flags, page_size);
>> @@ -337,7 +337,8 @@ static void test_guest_memfd_guest(void)
>> "Default VM type should always support guest_memfd mmap()");
>>
>> size = vm->page_size;
>> - fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP);
>> + fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP |
>> + GUEST_MEMFD_FLAG_DEFAULT_SHARED);
>> vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
>>
>> mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index 08a6bc7d25b6..19f05a45be04 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -328,6 +328,9 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
>> if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode))
>> return VM_FAULT_SIGBUS;
>>
>> + if (!((u64)inode->i_private & GUEST_MEMFD_FLAG_DEFAULT_SHARED))
>> + return VM_FAULT_SIGBUS;
>> +
>> folio = kvm_gmem_get_folio(inode, vmf->pgoff);
>> if (IS_ERR(folio)) {
>> int err = PTR_ERR(folio);
>> @@ -525,7 +528,8 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
>> u64 valid_flags = 0;
>>
>> if (kvm_arch_supports_gmem_mmap(kvm))
>> - valid_flags |= GUEST_MEMFD_FLAG_MMAP;
>> + valid_flags |= GUEST_MEMFD_FLAG_MMAP |
>> + GUEST_MEMFD_FLAG_DEFAULT_SHARED;
>
> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and
> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth
> checking for that, at least until we have in-place conversion? Having
> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP,
> isn't a useful combination.
>
I think it's okay to have the two flags be orthogonal from the start.
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
> That said, these are all nits, I'll leave it to you. With that:
>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Tested-by: Fuad Tabba <tabba@google.com>
>
> Cheers,
> /fuad
>
>
>
>>
>> if (flags & ~valid_flags)
>> return -EINVAL;
>> --
>> 2.51.0.536.g15c5d4f767-goog
>>
On Mon, Sep 29, 2025, Ackerley Tng wrote:
> Fuad Tabba <tabba@google.com> writes:
>
> > Hi Sean,
> >
> > On Fri, 26 Sept 2025 at 17:31, Sean Christopherson <seanjc@google.com> wrote:
> >>
> >> Add a guest_memfd flag to allow userspace to state that the underlying
> >> memory should be configured to be shared by default, and reject user page
> >> faults if the guest_memfd instance's memory isn't shared by default.
> >> Because KVM doesn't yet support in-place private<=>shared conversions, all
> >> guest_memfd memory effectively follows the default state.
> >>
> >> Alternatively, KVM could deduce the default state based on MMAP, which for
> >> all intents and purposes is what KVM currently does. However, implicitly
> >> deriving the default state based on MMAP will result in a messy ABI when
> >> support for in-place conversions is added.
> >>
> >> For x86 CoCo VMs, which don't yet support MMAP, memory is currently private
> >> by default (otherwise the memory would be unusable). If MMAP implies
> >> memory is shared by default, then the default state for CoCo VMs will vary
> >> based on MMAP, and from userspace's perspective, will change when in-place
> >> conversion support is added. I.e. to maintain guest<=>host ABI, userspace
> >> would need to immediately convert all memory from shared=>private, which
> >> is both ugly and inefficient. The inefficiency could be avoided by adding
> >> a flag to state that memory is _private_ by default, irrespective of MMAP,
> >> but that would lead to an equally messy and hard to document ABI.
> >>
> >> Bite the bullet and immediately add a flag to control the default state so
> >> that the effective behavior is explicit and straightforward.
> >>
>
> I like having this flag, but didn't propose this because I thought folks
> depending on the default being shared (Patrick/Nikita) might have their
> usage broken.
mmap() support hasn't landed upstream, so as far as the upstream kernel is
concerned, there is no userspace to break. Which is exactly why I want to land
this (or something like it) in 6.18, before GUEST_MEMFD_FLAG_MMAP is officially
released.
> >> Fixes: 3d3a04fad25a ("KVM: Allow and advertise support for host mmap() on guest_memfd files")
> >> Cc: David Hildenbrand <david@redhat.com>
> >> Cc: Fuad Tabba <tabba@google.com>
> >> Signed-off-by: Sean Christopherson <seanjc@google.com>
> >> ---
> >> Documentation/virt/kvm/api.rst | 10 ++++++++--
> >> include/uapi/linux/kvm.h | 3 ++-
> >> tools/testing/selftests/kvm/guest_memfd_test.c | 5 +++--
> >> virt/kvm/guest_memfd.c | 6 +++++-
> >> 4 files changed, 18 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> >> index c17a87a0a5ac..4dfe156bbe3c 100644
> >> --- a/Documentation/virt/kvm/api.rst
> >> +++ b/Documentation/virt/kvm/api.rst
> >> @@ -6415,8 +6415,14 @@ 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).
> >>
> >> When the capability KVM_CAP_GUEST_MEMFD_MMAP is supported, the 'flags' field
> >> -supports GUEST_MEMFD_FLAG_MMAP. Setting this flag on guest_memfd creation
> >> -enables mmap() and faulting of guest_memfd memory to host userspace.
> >> +supports GUEST_MEMFD_FLAG_MMAP and GUEST_MEMFD_FLAG_DEFAULT_SHARED. Setting
> >
> > There's an extra space between `and` and `GUEST_MEMFD_FLAG_DEFAULT_SHARED`.
> >
>
> +1 on this. Also, would you consider putting the concept of "at creation
> time" or "at initialization time" into the name of the flag?
Yah, GUEST_MEMFD_FLAG_INIT_SHARED?
> "Default" could be interpreted as "whenever a folio is allocated for
> this guest_memfd", the memory the folio represents is by default
> shared.
>
> What we want to represent is that when the guest_memfd is created,
> memory at all indices are initialized as shared.
>
> Looking a bit further, when conversion is supported, if this flag is not
> specified, then all the indices are initialized as private, right?
Correct, which is the current (pre-6.18) behavior.
> >> +the MMAP flag on guest_memfd creation enables mmap() and faulting of guest_memfd
> >> +memory to host userspace (so long as the memory is currently shared). Setting
> >> +DEFAULT_SHARED makes all guest_memfd memory shared by default (versus private
> >> +by default). Note! Because KVM doesn't yet support in-place private<=>shared
> >> +conversions, DEFAULT_SHARED must be specified in order to fault memory into
> >> +userspace page tables. This limitation will go away when in-place conversions
> >> +are supported.
> >
> > I think that a more accurate (and future proof) description of the
> > mmap flag could be something along the lines of:
> >
>
> +1 on these suggestions, I agree that making the concepts of SHARED vs
> MMAP orthogonal from the start is more future proof.
>
> > + Setting GUEST_MEMFD_FLAG_MMAP enables using mmap() on the file descriptor.
> >
> > + Setting GUEST_MEMFD_FLAG_DEFAULT_SHARED makes all memory in the file shared
> > + by default
>
> See above, I'd prefer clarifying this as "at initialization time" or
> something similar.
Roger that.
> > At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and GUEST_MEMFD_FLAG_MMAP
> > don't make sense without each other. Is it worth checking for that, at
> > least until we have in-place conversion? Having only
> > GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP, isn't a
> > useful combination.
Heh, that's exactly how I coded things up to start:
/*
* TODO: Drop the restriction that memory must be shared by default
* once in-place conversions are supported.
*/
if (flags & GUEST_MEMFD_FLAG_MMAP &&
!(flags & GUEST_MEMFD_FLAG_DEFAULT_SHARED))
return -EINVAL;
but if we go that route, then dropping the restriction would result in an ABI
change for non-CoCo VMs. The odds of such an ABI changes breaking userspace are
basically zero, but I couldn't think of any reason to risk it; userspace would
need to specify MMAP+SHARED either way.
And on the flip side, not enforcing the flags at the time of creation allows us
to test that user page faults to private memory are rejected. It's not a ton of
meaningful coverage, but it's not nothing either. And from a code perspective,
the diffs when in-place conversions are added are quite nice, as the concepts
don't change (user faults to private memory are disallowed), only the mechanics
change, i.e. the diffs highlight what all needs to happen to support conversions
without the extra noise of a change in overall semantics.
Hi Ackerley!
On Mon, 2025-09-29 at 10:43 +0100, Ackerley Tng wrote:
> Fuad Tabba <tabba@google.com> writes:
>
>> Hi Sean,
>>
>> On Fri, 26 Sept 2025 at 17:31, Sean Christopherson <seanjc@google.com> wrote:
>>>
>>> Add a guest_memfd flag to allow userspace to state that the underlying
>>> memory should be configured to be shared by default, and reject user page
>>> faults if the guest_memfd instance's memory isn't shared by default.
>>> Because KVM doesn't yet support in-place private<=>shared conversions, all
>>> guest_memfd memory effectively follows the default state.
>>>
>>> Alternatively, KVM could deduce the default state based on MMAP, which for
>>> all intents and purposes is what KVM currently does. However, implicitly
>>> deriving the default state based on MMAP will result in a messy ABI when
>>> support for in-place conversions is added.
>>>
>>> For x86 CoCo VMs, which don't yet support MMAP, memory is currently private
>>> by default (otherwise the memory would be unusable). If MMAP implies
>>> memory is shared by default, then the default state for CoCo VMs will vary
>>> based on MMAP, and from userspace's perspective, will change when in-place
>>> conversion support is added. I.e. to maintain guest<=>host ABI, userspace
>>> would need to immediately convert all memory from shared=>private, which
>>> is both ugly and inefficient. The inefficiency could be avoided by adding
>>> a flag to state that memory is _private_ by default, irrespective of MMAP,
>>> but that would lead to an equally messy and hard to document ABI.
>>>
>>> Bite the bullet and immediately add a flag to control the default state so
>>> that the effective behavior is explicit and straightforward.
>>>
>
> I like having this flag, but didn't propose this because I thought folks
> depending on the default being shared (Patrick/Nikita) might have their
> usage broken.
We'll just need to pass the new flag in Firecracker, that's not a problem
at all :) We aren't running this anywhere in production yet, so nothing
would break on our end.
>>> Fixes: 3d3a04fad25a ("KVM: Allow and advertise support for host mmap() on guest_memfd files")
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Fuad Tabba <tabba@google.com>
>>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>>> ---
>>> Documentation/virt/kvm/api.rst | 10 ++++++++--
>>> include/uapi/linux/kvm.h | 3 ++-
>>> tools/testing/selftests/kvm/guest_memfd_test.c | 5 +++--
>>> virt/kvm/guest_memfd.c | 6 +++++-
>>> 4 files changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
>>> index c17a87a0a5ac..4dfe156bbe3c 100644
>>> --- a/Documentation/virt/kvm/api.rst
>>> +++ b/Documentation/virt/kvm/api.rst
>>> @@ -6415,8 +6415,14 @@ 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).
>>>
>>> When the capability KVM_CAP_GUEST_MEMFD_MMAP is supported, the 'flags' field
>>> -supports GUEST_MEMFD_FLAG_MMAP. Setting this flag on guest_memfd creation
>>> -enables mmap() and faulting of guest_memfd memory to host userspace.
>>> +supports GUEST_MEMFD_FLAG_MMAP and GUEST_MEMFD_FLAG_DEFAULT_SHARED. Setting
>>
>> There's an extra space between `and` and `GUEST_MEMFD_FLAG_DEFAULT_SHARED`.
>>
>
> +1 on this. Also, would you consider putting the concept of "at creation
> time" or "at initialization time" into the name of the flag?
>
> "Default" could be interpreted as "whenever a folio is allocated for
> this guest_memfd", the memory the folio represents is by default
> shared.
>
> What we want to represent is that when the guest_memfd is created,
> memory at all indices are initialized as shared.
>
> Looking a bit further, when conversion is supported, if this flag is not
> specified, then all the indices are initialized as private, right?
>
>>> +the MMAP flag on guest_memfd creation enables mmap() and faulting of guest_memfd
>>> +memory to host userspace (so long as the memory is currently shared). Setting
>>> +DEFAULT_SHARED makes all guest_memfd memory shared by default (versus private
>>> +by default). Note! Because KVM doesn't yet support in-place private<=>shared
>>> +conversions, DEFAULT_SHARED must be specified in order to fault memory into
>>> +userspace page tables. This limitation will go away when in-place conversions
>>> +are supported.
>>
>> I think that a more accurate (and future proof) description of the
>> mmap flag could be something along the lines of:
>>
>
> +1 on these suggestions, I agree that making the concepts of SHARED vs
> MMAP orthogonal from the start is more future proof.
>
>> + Setting GUEST_MEMFD_FLAG_MMAP enables using mmap() on the file descriptor.
>>
>> + Setting GUEST_MEMFD_FLAG_DEFAULT_SHARED makes all memory in the file shared
>> + by default
>
> See above, I'd prefer clarifying this as "at initialization time" or
> something similar.
>
>> , as opposed to private. Shared memory can be faulted into host
>> + userspace page tables. Private memory cannot.
>>
>>> When the KVM MMU performs a PFN lookup to service a guest fault and the backing
>>> guest_memfd has the GUEST_MEMFD_FLAG_MMAP set, then the fault will always be
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index 6efa98a57ec1..38a2c083b6aa 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -1599,7 +1599,8 @@ 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_MMAP (1ULL << 0)
>>> +#define GUEST_MEMFD_FLAG_MMAP (1ULL << 0)
>>> +#define GUEST_MEMFD_FLAG_DEFAULT_SHARED (1ULL << 1)
>>>
>>> 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 b3ca6737f304..81b11a958c7a 100644
>>> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
>>> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
>>> @@ -274,7 +274,7 @@ static void test_guest_memfd(unsigned long vm_type)
>>> vm = vm_create_barebones_type(vm_type);
>>>
>>> if (vm_check_cap(vm, KVM_CAP_GUEST_MEMFD_MMAP))
>>> - flags |= GUEST_MEMFD_FLAG_MMAP;
>>> + flags |= GUEST_MEMFD_FLAG_MMAP | GUEST_MEMFD_FLAG_DEFAULT_SHARED;
>>>
>>> test_create_guest_memfd_multiple(vm);
>>> test_create_guest_memfd_invalid_sizes(vm, flags, page_size);
>>> @@ -337,7 +337,8 @@ static void test_guest_memfd_guest(void)
>>> "Default VM type should always support guest_memfd mmap()");
>>>
>>> size = vm->page_size;
>>> - fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP);
>>> + fd = vm_create_guest_memfd(vm, size, GUEST_MEMFD_FLAG_MMAP |
>>> + GUEST_MEMFD_FLAG_DEFAULT_SHARED);
>>> vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
>>>
>>> mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>> index 08a6bc7d25b6..19f05a45be04 100644
>>> --- a/virt/kvm/guest_memfd.c
>>> +++ b/virt/kvm/guest_memfd.c
>>> @@ -328,6 +328,9 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
>>> if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode))
>>> return VM_FAULT_SIGBUS;
>>>
>>> + if (!((u64)inode->i_private & GUEST_MEMFD_FLAG_DEFAULT_SHARED))
>>> + return VM_FAULT_SIGBUS;
>>> +
>>> folio = kvm_gmem_get_folio(inode, vmf->pgoff);
>>> if (IS_ERR(folio)) {
>>> int err = PTR_ERR(folio);
>>> @@ -525,7 +528,8 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
>>> u64 valid_flags = 0;
>>>
>>> if (kvm_arch_supports_gmem_mmap(kvm))
>>> - valid_flags |= GUEST_MEMFD_FLAG_MMAP;
>>> + valid_flags |= GUEST_MEMFD_FLAG_MMAP |
>>> + GUEST_MEMFD_FLAG_DEFAULT_SHARED;
>>
>> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and
>> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth
>> checking for that, at least until we have in-place conversion? Having
>> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP,
>> isn't a useful combination.
>>
>
> I think it's okay to have the two flags be orthogonal from the start.
I think I dimly remember someone at one of the guest_memfd syncs
bringing up a usecase for having a VMA even if all memory is private,
not for faulting anything in, but to do madvise or something? Maybe it
was the NUMA stuff? (+Shivank)
So for that, having the flags be orthogonal would be useful even without
conversion support.
> Reviewed-by: Ackerley Tng <ackerleytng@google.com>
>
>> That said, these are all nits, I'll leave it to you. With that:
>>
>> Reviewed-by: Fuad Tabba <tabba@google.com>
>> Tested-by: Fuad Tabba <tabba@google.com>
>>
>> Cheers,
>> /fuad
>>
>>
>>
>>>
>>> if (flags & ~valid_flags)
>>> return -EINVAL;
>>> --
>>> 2.51.0.536.g15c5d4f767-goog
>>>
Best,
Patrick
GUEST_MEMFD_FLAG_DEFAULT_SHARED; >>> >>> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and >>> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth >>> checking for that, at least until we have in-place conversion? Having >>> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP, >>> isn't a useful combination. >>> >> >> I think it's okay to have the two flags be orthogonal from the start. > > I think I dimly remember someone at one of the guest_memfd syncs > bringing up a usecase for having a VMA even if all memory is private, > not for faulting anything in, but to do madvise or something? Maybe it > was the NUMA stuff? (+Shivank) Yes, that should be it. But we're never faulting in these pages, we only need the VMA (for the time being, until there is the in-place conversion). -- Cheers David / dhildenb
David Hildenbrand <david@redhat.com> writes: > GUEST_MEMFD_FLAG_DEFAULT_SHARED; >>>> >>>> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and >>>> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth >>>> checking for that, at least until we have in-place conversion? Having >>>> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP, >>>> isn't a useful combination. >>>> >>> >>> I think it's okay to have the two flags be orthogonal from the start. >> >> I think I dimly remember someone at one of the guest_memfd syncs >> bringing up a usecase for having a VMA even if all memory is private, >> not for faulting anything in, but to do madvise or something? Maybe it >> was the NUMA stuff? (+Shivank) > > Yes, that should be it. But we're never faulting in these pages, we only > need the VMA (for the time being, until there is the in-place conversion). > Yup, Sean's patch disables faulting if GUEST_MEMFD_FLAG_DEFAULT_SHARED is not set, but mmap() is always enabled so madvise() still works. Requiring GUEST_MEMFD_FLAG_DEFAULT_SHARED to be set together with GUEST_MEMFD_FLAG_MMAP would still allow madvise() to work since GUEST_MEMFD_FLAG_DEFAULT_SHARED only gates faulting. To clarify, I'm still for making GUEST_MEMFD_FLAG_DEFAULT_SHARED orthogonal to GUEST_MEMFD_FLAG_MMAP with no additional checks on top of whatever's in this patch. :) > -- > Cheers > > David / dhildenb
On Mon, Sep 29, 2025, Ackerley Tng wrote: > David Hildenbrand <david@redhat.com> writes: > > > GUEST_MEMFD_FLAG_DEFAULT_SHARED; > >>>> > >>>> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and > >>>> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth > >>>> checking for that, at least until we have in-place conversion? Having > >>>> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP, > >>>> isn't a useful combination. > >>>> > >>> > >>> I think it's okay to have the two flags be orthogonal from the start. > >> > >> I think I dimly remember someone at one of the guest_memfd syncs > >> bringing up a usecase for having a VMA even if all memory is private, > >> not for faulting anything in, but to do madvise or something? Maybe it > >> was the NUMA stuff? (+Shivank) > > > > Yes, that should be it. But we're never faulting in these pages, we only > > need the VMA (for the time being, until there is the in-place conversion). > > > > Yup, Sean's patch disables faulting if GUEST_MEMFD_FLAG_DEFAULT_SHARED > is not set, but mmap() is always enabled so madvise() still works. Hah! I totally intended that :-D > Requiring GUEST_MEMFD_FLAG_DEFAULT_SHARED to be set together with > GUEST_MEMFD_FLAG_MMAP would still allow madvise() to work since > GUEST_MEMFD_FLAG_DEFAULT_SHARED only gates faulting. > > To clarify, I'm still for making GUEST_MEMFD_FLAG_DEFAULT_SHARED > orthogonal to GUEST_MEMFD_FLAG_MMAP with no additional checks on top of > whatever's in this patch. :)
On Mon, Sep 29, 2025, Sean Christopherson wrote:
> On Mon, Sep 29, 2025, Ackerley Tng wrote:
> > David Hildenbrand <david@redhat.com> writes:
> >
> > > GUEST_MEMFD_FLAG_DEFAULT_SHARED;
> > >>>>
> > >>>> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and
> > >>>> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth
> > >>>> checking for that, at least until we have in-place conversion? Having
> > >>>> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP,
> > >>>> isn't a useful combination.
> > >>>>
> > >>>
> > >>> I think it's okay to have the two flags be orthogonal from the start.
> > >>
> > >> I think I dimly remember someone at one of the guest_memfd syncs
> > >> bringing up a usecase for having a VMA even if all memory is private,
> > >> not for faulting anything in, but to do madvise or something? Maybe it
> > >> was the NUMA stuff? (+Shivank)
> > >
> > > Yes, that should be it. But we're never faulting in these pages, we only
> > > need the VMA (for the time being, until there is the in-place conversion).
> > >
> >
> > Yup, Sean's patch disables faulting if GUEST_MEMFD_FLAG_DEFAULT_SHARED
> > is not set, but mmap() is always enabled so madvise() still works.
>
> Hah! I totally intended that :-D
>
> > Requiring GUEST_MEMFD_FLAG_DEFAULT_SHARED to be set together with
> > GUEST_MEMFD_FLAG_MMAP would still allow madvise() to work since
> > GUEST_MEMFD_FLAG_DEFAULT_SHARED only gates faulting.
> >
> > To clarify, I'm still for making GUEST_MEMFD_FLAG_DEFAULT_SHARED
> > orthogonal to GUEST_MEMFD_FLAG_MMAP with no additional checks on top of
> > whatever's in this patch. :)
Oh! This got me looking at kvm_arch_supports_gmem_mmap() and thus
KVM_CAP_GUEST_MEMFD_MMAP. Two things:
1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so
that we don't need to add a capability every time a new flag comes along,
and so that userspace can gather all flags in a single ioctl. If gmem ever
supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but
that's a non-issue relatively speaking.
2. We should allow mmap() for x86 CoCo VMs right away. As evidenced by this
series, mmap() on private memory is totally fine. It's not useful until the
NUMA and/or in-place conversion support comes along, but's not dangerous in
any way. The actual restriction is on initializing memory to be shared,
because allowing memory to be shared from gmem's perspective while it's
private from the VM's perspective would be all kinds of broken.
E.g. with a s/kvm_arch_supports_gmem_mmap/kvm_arch_supports_gmem_init_shared:
case KVM_CAP_GUEST_MEMFD_FLAGS:
if (!kvm || kvm_arch_supports_init_shared(kvm))
return GUEST_MEMFD_FLAG_MMAP |
GUEST_MEMFD_FLAG_INIT_SHARED;
return GUEST_MEMFD_FLAG_MMAP;
#2 is also a good reason to add INIT_SHARED straightaway. Without INIT_SHARED,
we'd have to INIT_PRIVATE to make the NUMA support useful for x86 CoCo VMs, i.e.
it's not just in-place conversion that's affected, IIUC.
I'll add this in v2.
On Mon, Sep 29, 2025 at 5:15 PM Sean Christopherson <seanjc@google.com> wrote: > > Oh! This got me looking at kvm_arch_supports_gmem_mmap() and thus > KVM_CAP_GUEST_MEMFD_MMAP. Two things: > > 1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so > that we don't need to add a capability every time a new flag comes along, > and so that userspace can gather all flags in a single ioctl. If gmem ever > supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but > that's a non-issue relatively speaking. > Guest_memfd capabilities don't necessarily translate into flags, so ideally: 1) There should be two caps, KVM_CAP_GUEST_MEMFD_FLAGS and KVM_CAP_GUEST_MEMFD_CAPS. 2) IMO they should both support namespace of 64 values at least from the get go. 3) The reservation scheme for upstream should ideally be LSB's first for the new caps/flags. guest_memfd will achieve multiple features in future, both upstream and in out-of-tree versions to deploy features before they make their way upstream. Generally the scheme followed by out-of-tree versions is to define a custom UAPI that won't conflict with upstream UAPIs in near future. Having a namespace of 32 values gives little space to avoid the conflict, e.g. features like hugetlb support will have to eat up at least 5 bits from the flags [1]. [1] https://elixir.bootlin.com/linux/v6.17/source/include/uapi/asm-generic/hugetlb_encode.h#L20
On Wed, Oct 01, 2025, Vishal Annapurve wrote: > On Mon, Sep 29, 2025 at 5:15 PM Sean Christopherson <seanjc@google.com> wrote: > > > > Oh! This got me looking at kvm_arch_supports_gmem_mmap() and thus > > KVM_CAP_GUEST_MEMFD_MMAP. Two things: > > > > 1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so > > that we don't need to add a capability every time a new flag comes along, > > and so that userspace can gather all flags in a single ioctl. If gmem ever > > supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but > > that's a non-issue relatively speaking. > > > > Guest_memfd capabilities don't necessarily translate into flags, so ideally: > 1) There should be two caps, KVM_CAP_GUEST_MEMFD_FLAGS and > KVM_CAP_GUEST_MEMFD_CAPS. I'm not saying we can't have another GUEST_MEMFD capability or three, all I'm saying is that for enumerating what flags can be passed to KVM_CREATE_GUEST_MEMFD, KVM_CAP_GUEST_MEMFD_FLAGS is a better fit than a one-off KVM_CAP_GUEST_MEMFD_MMAP. > 2) IMO they should both support namespace of 64 values at least from the get go. It's a limitation of KVM_CHECK_EXTENSION, and all of KVM's plumbing for ioctls. Because KVM still supports 32-bit architectures, direct returns from ioctls are forced to fit in 32-bit values to avoid unintentionally creating different ABI for 32-bit vs. 64-bit kernels. We could add KVM_CHECK_EXTENSION2 or KVM_CHECK_EXTENSION64 or something, but I honestly don't see the point. The odds of guest_memfd supporting >32 flags is small, and the odds of that happening in the next ~5 years is basically zero. All so that userspace can make one syscall instead of two for a path that isn't remotely performance critical. So while I agree that being able to enumerate 64 flags from the get-go would be nice to have, it's simply not worth the effort (unless someone has a clever idea). > 3) The reservation scheme for upstream should ideally be LSB's first > for the new caps/flags. We're getting way ahead of ourselves. Nothing needs KVM_CAP_GUEST_MEMFD_CAPS at this time, so there's nothing to discuss. > guest_memfd will achieve multiple features in future, both upstream > and in out-of-tree versions to deploy features before they make their When it comes to upstream uAPI and uABI, out-of-tree kernel code is irrelevant. > way upstream. Generally the scheme followed by out-of-tree versions is > to define a custom UAPI that won't conflict with upstream UAPIs in > near future. Having a namespace of 32 values gives little space to > avoid the conflict, e.g. features like hugetlb support will have to > eat up at least 5 bits from the flags [1]. Why on earth would out-of-tree code use KVM_CAP_GUEST_MEMFD_FLAGS? Providing infrastructure to support an infinite (quite literally) number of out-of-tree capabilities and sub-ioctls, with practically zero chance of conflict, is not difficult. See internal b/378111418. But as above, this is not upstream's problem to solve. > [1] https://elixir.bootlin.com/linux/v6.17/source/include/uapi/asm-generic/hugetlb_encode.h#L20
On Wed, Oct 1, 2025 at 9:15 AM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Oct 01, 2025, Vishal Annapurve wrote: > > On Mon, Sep 29, 2025 at 5:15 PM Sean Christopherson <seanjc@google.com> wrote: > > > > > > Oh! This got me looking at kvm_arch_supports_gmem_mmap() and thus > > > KVM_CAP_GUEST_MEMFD_MMAP. Two things: > > > > > > 1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so > > > that we don't need to add a capability every time a new flag comes along, > > > and so that userspace can gather all flags in a single ioctl. If gmem ever > > > supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but > > > that's a non-issue relatively speaking. > > > > > > > Guest_memfd capabilities don't necessarily translate into flags, so ideally: > > 1) There should be two caps, KVM_CAP_GUEST_MEMFD_FLAGS and > > KVM_CAP_GUEST_MEMFD_CAPS. > > I'm not saying we can't have another GUEST_MEMFD capability or three, all I'm > saying is that for enumerating what flags can be passed to KVM_CREATE_GUEST_MEMFD, > KVM_CAP_GUEST_MEMFD_FLAGS is a better fit than a one-off KVM_CAP_GUEST_MEMFD_MMAP. Ah, ok. Then do you envision the guest_memfd caps to still be separate KVM caps per guest_memfd feature? > > > 2) IMO they should both support namespace of 64 values at least from the get go. > > It's a limitation of KVM_CHECK_EXTENSION, and all of KVM's plumbing for ioctls. > Because KVM still supports 32-bit architectures, direct returns from ioctls are > forced to fit in 32-bit values to avoid unintentionally creating different ABI > for 32-bit vs. 64-bit kernels. > > We could add KVM_CHECK_EXTENSION2 or KVM_CHECK_EXTENSION64 or something, but I > honestly don't see the point. The odds of guest_memfd supporting >32 flags is > small, and the odds of that happening in the next ~5 years is basically zero. > All so that userspace can make one syscall instead of two for a path that isn't > remotely performance critical. > > So while I agree that being able to enumerate 64 flags from the get-go would be > nice to have, it's simply not worth the effort (unless someone has a clever idea). Ack. > > > 3) The reservation scheme for upstream should ideally be LSB's first > > for the new caps/flags. > > We're getting way ahead of ourselves. Nothing needs KVM_CAP_GUEST_MEMFD_CAPS at > this time, so there's nothing to discuss. > > > guest_memfd will achieve multiple features in future, both upstream > > and in out-of-tree versions to deploy features before they make their > > When it comes to upstream uAPI and uABI, out-of-tree kernel code is irrelevant. > > > way upstream. Generally the scheme followed by out-of-tree versions is > > to define a custom UAPI that won't conflict with upstream UAPIs in > > near future. Having a namespace of 32 values gives little space to > > avoid the conflict, e.g. features like hugetlb support will have to > > eat up at least 5 bits from the flags [1]. > > Why on earth would out-of-tree code use KVM_CAP_GUEST_MEMFD_FLAGS? Providing I can imagine a scenario where KVM_CAP_GUEST_MEMFD_FLAGS is upstreamed and more flags landing in KVM_CAP_GUEST_MEMFD_FLAGS as supported over time afterwards. out-of-tree code may ingest KVM_CAP_GUEST_MEMFD_FLAGS in between. > infrastructure to support an infinite (quite literally) number of out-of-tree > capabilities and sub-ioctls, with practically zero chance of conflict, is not > difficult. See internal b/378111418. > > But as above, this is not upstream's problem to solve. > > > [1] https://elixir.bootlin.com/linux/v6.17/source/include/uapi/asm-generic/hugetlb_encode.h#L20
On Wed, Oct 01, 2025, Vishal Annapurve wrote: > On Wed, Oct 1, 2025 at 9:15 AM Sean Christopherson <seanjc@google.com> wrote: > > > > On Wed, Oct 01, 2025, Vishal Annapurve wrote: > > > On Mon, Sep 29, 2025 at 5:15 PM Sean Christopherson <seanjc@google.com> wrote: > > > > > > > > Oh! This got me looking at kvm_arch_supports_gmem_mmap() and thus > > > > KVM_CAP_GUEST_MEMFD_MMAP. Two things: > > > > > > > > 1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so > > > > that we don't need to add a capability every time a new flag comes along, > > > > and so that userspace can gather all flags in a single ioctl. If gmem ever > > > > supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but > > > > that's a non-issue relatively speaking. > > > > > > > > > > Guest_memfd capabilities don't necessarily translate into flags, so ideally: > > > 1) There should be two caps, KVM_CAP_GUEST_MEMFD_FLAGS and > > > KVM_CAP_GUEST_MEMFD_CAPS. > > > > I'm not saying we can't have another GUEST_MEMFD capability or three, all I'm > > saying is that for enumerating what flags can be passed to KVM_CREATE_GUEST_MEMFD, > > KVM_CAP_GUEST_MEMFD_FLAGS is a better fit than a one-off KVM_CAP_GUEST_MEMFD_MMAP. > > Ah, ok. Then do you envision the guest_memfd caps to still be separate > KVM caps per guest_memfd feature? Yes? No? It depends on the feature and the actual implementation. E.g. KVM_CAP_IRQCHIP enumerates support for a whole pile of ioctls.
On Wed, Oct 1, 2025 at 10:16 AM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Oct 01, 2025, Vishal Annapurve wrote: > > On Wed, Oct 1, 2025 at 9:15 AM Sean Christopherson <seanjc@google.com> wrote: > > > > > > On Wed, Oct 01, 2025, Vishal Annapurve wrote: > > > > On Mon, Sep 29, 2025 at 5:15 PM Sean Christopherson <seanjc@google.com> wrote: > > > > > > > > > > Oh! This got me looking at kvm_arch_supports_gmem_mmap() and thus > > > > > KVM_CAP_GUEST_MEMFD_MMAP. Two things: > > > > > > > > > > 1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so > > > > > that we don't need to add a capability every time a new flag comes along, > > > > > and so that userspace can gather all flags in a single ioctl. If gmem ever > > > > > supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but > > > > > that's a non-issue relatively speaking. > > > > > > > > > > > > > Guest_memfd capabilities don't necessarily translate into flags, so ideally: > > > > 1) There should be two caps, KVM_CAP_GUEST_MEMFD_FLAGS and > > > > KVM_CAP_GUEST_MEMFD_CAPS. > > > > > > I'm not saying we can't have another GUEST_MEMFD capability or three, all I'm > > > saying is that for enumerating what flags can be passed to KVM_CREATE_GUEST_MEMFD, > > > KVM_CAP_GUEST_MEMFD_FLAGS is a better fit than a one-off KVM_CAP_GUEST_MEMFD_MMAP. > > > > Ah, ok. Then do you envision the guest_memfd caps to still be separate > > KVM caps per guest_memfd feature? > > Yes? No? It depends on the feature and the actual implementation. E.g. > KVM_CAP_IRQCHIP enumerates support for a whole pile of ioctls. I think I am confused. Is the proposal here as follows? * Use KVM_CAP_GUEST_MEMFD_FLAGS for features that map to guest_memfd creation flags. * Use KVM caps for guest_memfd features that don't map to any flags. I think in general it would be better to have a KVM cap for each feature irrespective of the flags as the feature may also need additional UAPIs like IOCTLs. I fail to see the benefits of KVM_CAP_GUEST_MEMFD_FLAGS over KVM_CAP_GUEST_MEMFD_MMAP: 1) It limits the possible values to 32 even though we could pass 64 flags to the original ioctl. 2) Userspace has to anyways assume the semantics of each bit position. 3) Userspace still has to check for caps for features that carry extra UAPI baggage. KVM_CAP_GUEST_MEMFD_MMAP allows userspace to assume that mmap is supported and userspace can just pass in the mmap flag that it anyways has to assume.
Sean Christopherson <seanjc@google.com> writes:
> On Mon, Sep 29, 2025, Sean Christopherson wrote:
>> On Mon, Sep 29, 2025, Ackerley Tng wrote:
>> > David Hildenbrand <david@redhat.com> writes:
>> >
>> > > GUEST_MEMFD_FLAG_DEFAULT_SHARED;
>> > >>>>
>> > >>>> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and
>> > >>>> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth
>> > >>>> checking for that, at least until we have in-place conversion? Having
>> > >>>> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP,
>> > >>>> isn't a useful combination.
>> > >>>>
>> > >>>
>> > >>> I think it's okay to have the two flags be orthogonal from the start.
>> > >>
>> > >> I think I dimly remember someone at one of the guest_memfd syncs
>> > >> bringing up a usecase for having a VMA even if all memory is private,
>> > >> not for faulting anything in, but to do madvise or something? Maybe it
>> > >> was the NUMA stuff? (+Shivank)
>> > >
>> > > Yes, that should be it. But we're never faulting in these pages, we only
>> > > need the VMA (for the time being, until there is the in-place conversion).
>> > >
>> >
>> > Yup, Sean's patch disables faulting if GUEST_MEMFD_FLAG_DEFAULT_SHARED
>> > is not set, but mmap() is always enabled so madvise() still works.
>>
>> Hah! I totally intended that :-D
>>
>> > Requiring GUEST_MEMFD_FLAG_DEFAULT_SHARED to be set together with
>> > GUEST_MEMFD_FLAG_MMAP would still allow madvise() to work since
>> > GUEST_MEMFD_FLAG_DEFAULT_SHARED only gates faulting.
>> >
>> > To clarify, I'm still for making GUEST_MEMFD_FLAG_DEFAULT_SHARED
>> > orthogonal to GUEST_MEMFD_FLAG_MMAP with no additional checks on top of
>> > whatever's in this patch. :)
>
> Oh! This got me looking at kvm_arch_supports_gmem_mmap() and thus
> KVM_CAP_GUEST_MEMFD_MMAP. Two things:
>
> 1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so
> that we don't need to add a capability every time a new flag comes along,
> and so that userspace can gather all flags in a single ioctl. If gmem ever
> supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but
> that's a non-issue relatively speaking.
>
This is a good idea. In my internal WIP series I have 3 flags and 4
CAPs, lol. Some of those CAPs are not for new flags, though.
Would like to check your rationale for future reference: how about
generalizing beyong flags and having KVM_CAP_GUEST_MEMFD_CAPS which
returns 32 bits, one bit for every guest_memfd-related (not necessarily
flags-related) cap?
> 2. We should allow mmap() for x86 CoCo VMs right away. As evidenced by this
> series, mmap() on private memory is totally fine. It's not useful until the
> NUMA and/or in-place conversion support comes along, but's not dangerous in
> any way. The actual restriction is on initializing memory to be shared,
The actual restriction is that private memory must not be mapped to host
userspace, so it's not really about initializing, though before
conversion, initialization state is the only state.
With GUEST_MEMFD_FLAG_INIT_SHARED, the entire guest_memfd is shared and
mappable; without GUEST_MEMFD_FLAG_INIT_SHARED the entire guest_memfd is
private and not mappable (gated in kvm_gmem_fault_user_mapping()).
So yes, I agree that CoCo VMs should be allowed mmap() but not
GUEST_MEMFD_FLAG_INIT_SHARED, since GUEST_MEMFD_FLAG_INIT_SHARED makes
the entire guest_memfd take the shared state for the lifetime of
guest_memfd.
This is turning out to be a much nicer cleanup :)
> because allowing memory to be shared from gmem's perspective while it's
> private from the VM's perspective would be all kinds of broken.
>
>
> E.g. with a s/kvm_arch_supports_gmem_mmap/kvm_arch_supports_gmem_init_shared:
>
> case KVM_CAP_GUEST_MEMFD_FLAGS:
> if (!kvm || kvm_arch_supports_init_shared(kvm))
> return GUEST_MEMFD_FLAG_MMAP |
> GUEST_MEMFD_FLAG_INIT_SHARED;
>
> return GUEST_MEMFD_FLAG_MMAP;
>
You might end up with this while actually coding v2 up, but how about
case KVM_CAP_GUEST_MEMFD_FLAGS: {
int flag_caps = GUEST_MEMFD_FLAG_MMAP;
if (!kvm || kvm_arch_supports_init_shared(kvm))
flag_caps |= GUEST_MEMFD_FLAG_INIT_SHARED;
return flag_caps;
}
Then all the new non-optional CAPs can be or-ed onto flag_caps from the
start.
> #2 is also a good reason to add INIT_SHARED straightaway. Without INIT_SHARED,
> we'd have to INIT_PRIVATE to make the NUMA support useful for x86 CoCo VMs, i.e.
> it's not just in-place conversion that's affected, IIUC.
>
> I'll add this in v2.
On 26.09.25 18:31, Sean Christopherson wrote:
> Add a guest_memfd flag to allow userspace to state that the underlying
> memory should be configured to be shared by default, and reject user page
> faults if the guest_memfd instance's memory isn't shared by default.
> Because KVM doesn't yet support in-place private<=>shared conversions, all
> guest_memfd memory effectively follows the default state.
I recall we discussed exactly that in the past (e.g., on April 17) in the call:
"Current plan:
* guest_memfd creation flag to specify “all memory starts as shared”
* Compatible with the old behavior where all memory started as private
* Initially, only these can be mmap (no in-place conversion)
"
>
> Alternatively, KVM could deduce the default state based on MMAP, which for
> all intents and purposes is what KVM currently does. However, implicitly
> deriving the default state based on MMAP will result in a messy ABI when
> support for in-place conversions is added.
I don't recall the details, but I faintly remember that we discussed later that with
mmap support, the default will be shared for now, and that no other flag would be
required for the time being.
We could always add a "DEFAULT_PRIVATE" flag when we realize that we would have
to change the default later.
Ackerley might remember more details.
--
Cheers
David / dhildenb
Hi David. On Mon, 29 Sept 2025 at 09:38, David Hildenbrand <david@redhat.com> wrote: > > On 26.09.25 18:31, Sean Christopherson wrote: > > Add a guest_memfd flag to allow userspace to state that the underlying > > memory should be configured to be shared by default, and reject user page > > faults if the guest_memfd instance's memory isn't shared by default. > > Because KVM doesn't yet support in-place private<=>shared conversions, all > > guest_memfd memory effectively follows the default state. > > I recall we discussed exactly that in the past (e.g., on April 17) in the call: > > "Current plan: > * guest_memfd creation flag to specify “all memory starts as shared” > * Compatible with the old behavior where all memory started as private > * Initially, only these can be mmap (no in-place conversion) > " > > > > > Alternatively, KVM could deduce the default state based on MMAP, which for > > all intents and purposes is what KVM currently does. However, implicitly > > deriving the default state based on MMAP will result in a messy ABI when > > support for in-place conversions is added. > > I don't recall the details, but I faintly remember that we discussed later that with > mmap support, the default will be shared for now, and that no other flag would be > required for the time being. > > We could always add a "DEFAULT_PRIVATE" flag when we realize that we would have > to change the default later. I remember discussing this. For many confidential computing usecases, e.g., pKVM and TDX, it would make more sense for the default case to be private, since it's the more common state, and the initial state. It also makes sense since sharing is usually triggered by the guest. Ensuring that the initial state is private reduces the changes of the VMM forgetting to convert the memory to being private later on, potentially exposing all guest memory from the get go. I think it makes sense to clarify things now. Especially since with memory attributes, the default attribute is KVM_MEMORY_ATTRIBUTE_SHARED, which adds even more confusion. Cheers, /fuad > > Ackerley might remember more details. > > -- > Cheers > > David / dhildenb >
On 29.09.25 10:57, Fuad Tabba wrote: > Hi David. > > On Mon, 29 Sept 2025 at 09:38, David Hildenbrand <david@redhat.com> wrote: >> >> On 26.09.25 18:31, Sean Christopherson wrote: >>> Add a guest_memfd flag to allow userspace to state that the underlying >>> memory should be configured to be shared by default, and reject user page >>> faults if the guest_memfd instance's memory isn't shared by default. >>> Because KVM doesn't yet support in-place private<=>shared conversions, all >>> guest_memfd memory effectively follows the default state. >> >> I recall we discussed exactly that in the past (e.g., on April 17) in the call: >> >> "Current plan: >> * guest_memfd creation flag to specify “all memory starts as shared” >> * Compatible with the old behavior where all memory started as private >> * Initially, only these can be mmap (no in-place conversion) >> " >> >>> >>> Alternatively, KVM could deduce the default state based on MMAP, which for >>> all intents and purposes is what KVM currently does. However, implicitly >>> deriving the default state based on MMAP will result in a messy ABI when >>> support for in-place conversions is added. >> >> I don't recall the details, but I faintly remember that we discussed later that with >> mmap support, the default will be shared for now, and that no other flag would be >> required for the time being. >> >> We could always add a "DEFAULT_PRIVATE" flag when we realize that we would have >> to change the default later. > > I remember discussing this. For many confidential computing usecases, > e.g., pKVM and TDX, it would make more sense for the default case to > be private, since it's the more common state, and the initial state. > It also makes sense since sharing is usually triggered by the guest. > Ensuring that the initial state is private reduces the changes of the > VMM forgetting to convert the memory to being private later on, > potentially exposing all guest memory from the get go. > > I think it makes sense to clarify things now. Especially since with > memory attributes, the default attribute is > KVM_MEMORY_ATTRIBUTE_SHARED, which adds even more confusion. Makes sense to me then, thanks. -- Cheers David / dhildenb
© 2016 - 2025 Red Hat, Inc.