All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: Michael Roth <michael.roth@amd.com>,
	qemu-devel@nongnu.org, jmarcin@redhat.com, david@kernel.org,
	pbonzini@redhat.com, chenyi.qiang@intel.com, farosas@suse.de,
	aik@amd.com, xiaoyao.li@intel.com
Subject: Re: [PATCH v4 08/12] hostmem: Support fully shared guest memfd to back a VM
Date: Thu, 13 Aug 2026 13:48:21 +0100	[thread overview]
Message-ID: <an29FY93x45qnJuR@redhat.com> (raw)
In-Reply-To: <an24aI4sAILoxncp@x1.local>

On Thu, Aug 13, 2026 at 08:28:24AM -0400, Peter Xu wrote:
> On Thu, Aug 13, 2026 at 09:24:22AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Aug 12, 2026 at 03:16:46PM -0500, Michael Roth wrote:
> > > From: Peter Xu <peterx@redhat.com>
> > > 
> > > Host backends supports guest-memfd now by detecting whether it's a
> > > confidential VM.  There's no way to choose it yet from the memory level to
> > > use it fully shared.  If we use guest-memfd, it so far always implies we
> > > need two layers of memory backends, while the guest-memfd only provides the
> > > private set of pages.
> > > 
> > > This patch introduces a way so that QEMU can consume guest memfd as the
> > > only source of memory to back the object (aka, fully shared).
> > > 
> > > To use the fully shared guest-memfd, one can add a memfd object with:
> > > 
> > >   -object memory-backend-memfd,guest-memfd=on,share=on
> > > 
> > > Note that share=on is required with fully shared guest_memfd.
> > > 
> > > PS: there's a trivial touch-up on fd<0 check, because the stub to create
> > > guest-memfd may return negative but not -1.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > > ---
> > >  backends/hostmem-memfd.c | 56 ++++++++++++++++++++++++++++++++++++----
> > >  qapi/qom.json            |  6 ++++-
> > >  2 files changed, 56 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > > index ea93f034e4..fbe65b00be 100644
> > > --- a/backends/hostmem-memfd.c
> > > +++ b/backends/hostmem-memfd.c
> > > @@ -18,6 +18,8 @@
> > >  #include "qapi/error.h"
> > >  #include "qom/object.h"
> > >  #include "migration/cpr.h"
> > > +#include "system/kvm.h"
> > > +#include <linux/kvm.h>
> > >  
> > >  OBJECT_DECLARE_SIMPLE_TYPE(HostMemoryBackendMemfd, MEMORY_BACKEND_MEMFD)
> > >  
> > > @@ -28,6 +30,13 @@ struct HostMemoryBackendMemfd {
> > >      bool hugetlb;
> > >      uint64_t hugetlbsize;
> > >      bool seal;
> > > +    /*
> > > +     * NOTE: this differs from HostMemoryBackend's guest_memfd_private,
> > > +     * which represents an internally private guest-memfd that only backs
> > > +     * private pages.  Instead, this flag marks the memory backend will
> > > +     * 100% use the guest-memfd pages in-place.
> > > +     */
> > > +    bool guest_memfd;
> > >  };
> > >  
> > >  static bool
> > > @@ -47,11 +56,29 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > >          goto have_fd;
> > >      }
> > >  
> > > -    fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size,
> > > -                           m->hugetlb, m->hugetlbsize, m->seal ?
> > > -                           F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL : 0,
> > > -                           errp);
> > > -    if (fd == -1) {
> > > +    if (m->guest_memfd) {
> > > +        if (!backend->share) {
> > > +            error_setg(errp, "guest-memfd=on must be used with share=on");
> > > +            return false;
> > > +        } else if (m->seal) {
> > > +            error_setg(errp, "guest-memfd=on must be used with seal=off");
> > > +            return false;
> > > +        } else if (m->hugetlb) {
> > > +            error_setg(errp, "guest-memfd=on must be used with hugetlb=off");
> > 
> > Reporting an error without returning false like the other cases.
> > 
> > > +        }
> > > +
> > > +        fd = kvm_create_guest_memfd(backend->size,
> > > +                                    GUEST_MEMFD_FLAG_MMAP |
> > > +                                    GUEST_MEMFD_FLAG_INIT_SHARED,
> > > +                                    errp);
> > > +    } else {
> > > +        fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size,
> > > +                               m->hugetlb, m->hugetlbsize, m->seal ?
> > > +                               F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL : 0,
> > > +                               errp);
> > > +    }
> > > +
> > > +    if (fd < 0) {
> > >          return false;
> > >      }
> > >      cpr_save_fd(name, 0, fd);
> > > @@ -65,6 +92,18 @@ have_fd:
> > >                                            backend->size, ram_flags, fd, 0, errp);
> > >  }
> > >  
> > > +static bool
> > > +memfd_backend_get_guest_memfd(Object *o, Error **errp)
> > > +{
> > > +    return MEMORY_BACKEND_MEMFD(o)->guest_memfd;
> > > +}
> > > +
> > > +static void
> > > +memfd_backend_set_guest_memfd(Object *o, bool value, Error **errp)
> > > +{
> > > +    MEMORY_BACKEND_MEMFD(o)->guest_memfd = value;
> > > +}
> > > +
> > >  static bool
> > >  memfd_backend_get_hugetlb(Object *o, Error **errp)
> > >  {
> > > @@ -152,6 +191,13 @@ memfd_backend_class_init(ObjectClass *oc, const void *data)
> > >          object_class_property_set_description(oc, "hugetlbsize",
> > >                                                "Huge pages size (ex: 2M, 1G)");
> > >      }
> > > +
> > > +    object_class_property_add_bool(oc, "guest-memfd",
> > > +                                   memfd_backend_get_guest_memfd,
> > > +                                   memfd_backend_set_guest_memfd);
> > > +    object_class_property_set_description(oc, "guest-memfd",
> > > +                                          "Use guest memfd");
> > > +
> > >      object_class_property_add_bool(oc, "seal",
> > >                                     memfd_backend_get_seal,
> > >                                     memfd_backend_set_seal);
> > > diff --git a/qapi/qom.json b/qapi/qom.json
> > > index c55776af7d..ee981fc44c 100644
> > > --- a/qapi/qom.json
> > > +++ b/qapi/qom.json
> > > @@ -771,13 +771,17 @@
> > >  # @seal: if true, create a sealed-file, which will block further
> > >  #     resizing of the memory (default: true)
> > >  #
> > > +# @guest-memfd: if true, use guest-memfd to back the memory region.
> > > +#     (default: false, since: 11.2)
> > > +#
> > >  # Since: 2.12
> > >  ##
> > >  { 'struct': 'MemoryBackendMemfdProperties',
> > >    'base': 'MemoryBackendProperties',
> > >    'data': { '*hugetlb': 'bool',
> > >              '*hugetlbsize': 'size',
> > > -            '*seal': 'bool' },
> > > +            '*seal': 'bool',
> > > +            '*guest-memfd': 'bool' },
> > >    'if': 'CONFIG_LINUX' }
> > 
> > We're reusing the 'memory-backend-memfd' class, and then at runtime
> > refusing allow the user to control any of properties in
> > MemoryBackendProperties.
> 
> gmemfd should be able to use all ultimately.
> 
> For seal, IMHO it's already implied, kind of forced seal=on but it doesn't
> matter, gmemfd was introduced with sealing, at least what QEMU implies with
> "F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL".  So IMHO we could ignore what
> user selected and assume it's ON.

Then we should not have a 'seal' property defined for guest memfd
at all. Defining a property and then ignoring it, or only ever
allowing 1 value to be set is a design mistake. The property should
not exist if it can't ever be changed by the user/app.

> For hugetlb, we will support hugetlb (and allow specify hugetlb size) for
> gmem in the future I believe.  It's only that this is done one step at a
> time so we haven't supported it yet, while the kernel support is still in
> progress.

The problem with this idea is that it makes it impossible for a mgmt
app to know if hugetlb is supported or not, as QEMU will always
report it supported against memory-backend-memfd.

Having a memory-backend-guest-memfd ensures the public interface
matches what is actually implemented/permitted for guest memfd.

> > "memory-backend-memfd,guest-memfd=on|off" is switching between two
> > separate implementations of the class.
> > 
> > This whole thing is just shouting "use a different class".
> > 
> > There is no meaningful sharing of code here, and the sharing of the
> > public interface is offering apps no value as the impl prevents them
> > from choosing the value of the properties - they have to be set of
> > certain values which are not introspectable.
> > 
> > Please introduce a "memory-backend-guest-memfd" backend instead.
> 
> This is indeed what Michael used to suggest, and we were discussing in
> previous version on which is better,
> 
> https://lore.kernel.org/r/rjqfiwh57gip3u3psqg33jhmo7ixaj2qwzupc7zdk7f3d26qnu@tglactz67ogk
> 
> The hope is this is also easier for either libvirt or most users, but
> please correct me if it's not the case, especially for libvirt.  The plan
> is when CoCo flags are provided, all things will automatically switch to a
> CoCo-friendly implementation within QEMU.
> 
> It also means here the guest-memfd= parameter shouldn't be needed in real
> CoCo contexts because they'll simply be implied (no cmdline change needed
> for the same "-object memory-backend-memfd" one used to use without CoCo).
> It's only needed for only special use of guest-memfd, in this case
> init-shared is the special case where CoCo doesn't use.

Reading all this, IMHO reusing memory-backend-memfd for the current
Coco support was a design mistake, it should have have a
memory-backend-guest-memfd object from the start.

Given that we need to be able to control memfd vs guest-memfd for
the non-Coco case, it is still worth introducing the new object
class today.

Even if the two classes shared all their properties (which they
don't given the comment about 'seal' being always on), then a
"foo=on|off" that toggles two separate impls is still creating
a pair of sub-classes by the backdoor. 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



  reply	other threads:[~2026-08-13 12:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12 20:16 [PATCH v4 00/12] KVM/hostmem: Support init-shared guest-memfd as VM backends Michael Roth
2026-08-12 20:16 ` [PATCH v4 01/12] kvm: Decouple memory attribute check from kvm_guest_memfd_supported Michael Roth
2026-08-12 20:16 ` [PATCH v4 02/12] kvm: Detect guest-memfd flags supported Michael Roth
2026-08-12 20:16 ` [PATCH v4 03/12] kvm: Provide explicit error for kvm_create_guest_memfd() Michael Roth
2026-08-12 20:16 ` [PATCH v4 04/12] ramblock: Rename guest_memfd to guest_memfd_private Michael Roth
2026-08-12 20:16 ` [PATCH v4 05/12] memory: Rename RAM_GUEST_MEMFD to RAM_GUEST_MEMFD_PRIVATE Michael Roth
2026-08-12 20:16 ` [PATCH v4 06/12] memory: Rename memory_region_has_guest_memfd() to *_private() Michael Roth
2026-08-12 20:16 ` [PATCH v4 07/12] hostmem: Rename guest_memfd to guest_memfd_private Michael Roth
2026-08-12 20:16 ` [PATCH v4 08/12] hostmem: Support fully shared guest memfd to back a VM Michael Roth
2026-08-13  8:24   ` Daniel P. Berrangé
2026-08-13 12:28     ` Peter Xu
2026-08-13 12:48       ` Daniel P. Berrangé [this message]
2026-08-13 14:06         ` Peter Xu
2026-08-12 20:16 ` [PATCH v4 09/12] machine: Rename machine_require_guest_memfd() to *_private() Michael Roth
2026-08-12 20:16 ` [PATCH v4 10/12] memory: Rename memory_region_init_ram_guest_memfd() " Michael Roth
2026-08-12 20:16 ` [PATCH v4 11/12] tests/migration-test: Support guest-memfd init shared mem type Michael Roth
2026-08-12 20:16 ` [PATCH v4 12/12] tests/migration-test: Add a precopy test for guest-memfd Michael Roth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=an29FY93x45qnJuR@redhat.com \
    --to=berrange@redhat.com \
    --cc=aik@amd.com \
    --cc=chenyi.qiang@intel.com \
    --cc=david@kernel.org \
    --cc=farosas@suse.de \
    --cc=jmarcin@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=xiaoyao.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.