All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Steven Sistare <steven.sistare@oracle.com>
Cc: Peter Xu <peterx@redhat.com>,
	qemu-devel@nongnu.org, Fabiano Rosas <farosas@suse.de>,
	David Hildenbrand <david@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Eduardo Habkost <eduardo@habkost.net>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Philippe Mathieu-Daude <philmd@linaro.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH V1 17/26] machine: memfd-alloc option
Date: Mon, 3 Jun 2024 11:17:38 +0100	[thread overview]
Message-ID: <Zl2YQhv97Ez3_FGn@redhat.com> (raw)
In-Reply-To: <79a8023d-2e19-4d80-821d-a03818a5372e@oracle.com>

On Wed, May 29, 2024 at 01:31:38PM -0400, Steven Sistare wrote:
> On 5/28/2024 5:12 PM, Peter Xu wrote:
> > On Mon, Apr 29, 2024 at 08:55:26AM -0700, Steve Sistare wrote:
> > > Allocate anonymous memory using memfd_create if the memfd-alloc machine
> > > option is set.
> > > 
> > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> > > ---
> > >   hw/core/machine.c   | 22 ++++++++++++++++++++++
> > >   include/hw/boards.h |  1 +
> > >   qemu-options.hx     |  6 ++++++
> > >   system/memory.c     |  9 ++++++---
> > >   system/physmem.c    | 18 +++++++++++++++++-
> > >   system/trace-events |  1 +
> > >   6 files changed, 53 insertions(+), 4 deletions(-)

> > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > index cf61f6b..f0dfda5 100644
> > > --- a/qemu-options.hx
> > > +++ b/qemu-options.hx
> > > @@ -32,6 +32,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
> > >       "                vmport=on|off|auto controls emulation of vmport (default: auto)\n"
> > >       "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
> > >       "                mem-merge=on|off controls memory merge support (default: on)\n"
> > > +    "                memfd-alloc=on|off controls allocating anonymous guest RAM using memfd_create (default: off)\n"
> > >       "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
> > >       "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
> > >       "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
> > > @@ -79,6 +80,11 @@ SRST
> > >           supported by the host, de-duplicates identical memory pages
> > >           among VMs instances (enabled by default).
> > > +    ``memfd-alloc=on|off``
> > > +        Enables or disables allocation of anonymous guest RAM using
> > > +        memfd_create.  Any associated memory-backend objects are created with
> > > +        share=on.  The memfd-alloc default is off.
> > > +
> > >       ``aes-key-wrap=on|off``
> > >           Enables or disables AES key wrapping support on s390-ccw hosts.
> > >           This feature controls whether AES wrapping keys will be created
> > > diff --git a/system/memory.c b/system/memory.c
> > > index 49f1cb2..ca04a0e 100644
> > > --- a/system/memory.c
> > > +++ b/system/memory.c
> > > @@ -1552,8 +1552,9 @@ bool memory_region_init_ram_nomigrate(MemoryRegion *mr,
> > >                                         uint64_t size,
> > >                                         Error **errp)
> > >   {
> > > +    uint32_t flags = current_machine->memfd_alloc ? RAM_SHARED : 0;
> > 
> > If there's a machine option to "use memfd for allocations", then it's
> > shared mem... Hmm..
> > 
> > It is a bit confusing to me in quite a few levels:
> > 
> >    - Why memory allocation method will be defined by a machine property,
> >      even if we have memory-backend-* which should cover everything?
> 
> Some memory regions are implicitly created, and have no explicit representation
> on the qemu command line.  memfd-alloc affects those.
> 
> More generally, memfd-alloc affects all ramblock allocations that are
> not explicitly represented by memory-backend object.  Thus the simple
> command line "qemu -m 1G" does not explicitly describe an object, so it
> goes through the anonymous allocation path, and is affected by memfd-alloc.
> 
> Internally, create_default_memdev does create a memory-backend object.
> That is what my doc comment above refers to:
>   Any associated memory-backend objects are created with share=on
> 
> An explicit "qemu -object memory-backend-*" is not affected by memfd-alloc.
> 
> The qapi comments in patch "migration: cpr-exec mode" attempt to say all that:
> 
> +#     Memory backend objects must have the share=on attribute, and
> +#     must be mmap'able in the new QEMU process.  For example,
> +#     memory-backend-file is acceptable, but memory-backend-ram is
> +#     not.
> +#
> +#     The VM must be started with the '-machine memfd-alloc=on'
> +#     option.  This causes implicit ram blocks -- those not explicitly
> +#     described by a memory-backend object -- to be allocated by
> +#     mmap'ing a memfd.  Examples include VGA, ROM, and even guest
> +#     RAM when it is specified without a memory-backend object.
> 
> >    - Even if we have such a machine property, why setting "memfd" will
> >      always imply shared?  why not private?  After all it's not called
> >      "memfd-shared-alloc", and we can create private mappings using
> >      e.g. memory-backend-memfd,share=off.
> 
> There is no use case for memfd-alloc with share=off, so no point IMO in
> making the option more verbose.  For cpr, the mapping with all its modifications
> must be visible to new qemu when qemu mmaps it.


So IIUC, cpr doesn't care about the use of 'memfd' as the specific impl,
it only cares that the memory is share=on.

Rather than having a machine type option "memfd-alloc" which is named after
a Linux specific impl detail, how about having a machine type option
"mem-share=on", which just happens to trigger use of memfd internally on
Linux ? That gives us freedom to use non-memfd options if appropriate in
the future.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2024-06-03 10:18 UTC|newest]

Thread overview: 122+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-29 15:55 [PATCH V1 00/26] Live update: cpr-exec Steve Sistare
2024-04-29 15:55 ` [PATCH V1 01/26] oslib: qemu_clear_cloexec Steve Sistare
2024-05-06 23:27   ` Fabiano Rosas
2024-05-07  8:56     ` Daniel P. Berrangé
2024-05-07 13:54       ` Fabiano Rosas
2024-04-29 15:55 ` [PATCH V1 02/26] vl: helper to request re-exec Steve Sistare
2024-04-29 15:55 ` [PATCH V1 03/26] migration: SAVEVM_FOREACH Steve Sistare
2024-05-06 23:17   ` Fabiano Rosas
2024-05-13 19:27     ` Steven Sistare
2024-05-27 18:14       ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 04/26] migration: delete unused parameter mis Steve Sistare
2024-05-06 21:50   ` Fabiano Rosas
2024-05-27 18:02   ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 05/26] migration: precreate vmstate Steve Sistare
2024-05-07 21:02   ` Fabiano Rosas
2024-05-13 19:28     ` Steven Sistare
2024-05-24 13:56   ` Fabiano Rosas
2024-05-27 18:16   ` Peter Xu
2024-05-28 15:09     ` Steven Sistare via
2024-05-29 18:39       ` Peter Xu
2024-05-30 17:04         ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 06/26] migration: precreate vmstate for exec Steve Sistare
2024-05-06 23:34   ` Fabiano Rosas
2024-05-13 19:28     ` Steven Sistare
2024-05-13 21:21       ` Fabiano Rosas
2024-04-29 15:55 ` [PATCH V1 07/26] migration: VMStateId Steve Sistare
2024-05-07 21:03   ` Fabiano Rosas
2024-05-27 18:20   ` Peter Xu
2024-05-28 15:10     ` Steven Sistare via
2024-05-28 17:44       ` Peter Xu
2024-05-29 17:30         ` Steven Sistare via
2024-05-29 18:53           ` Peter Xu
2024-05-30 17:11             ` Steven Sistare via
2024-05-30 18:03               ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 08/26] migration: vmstate_info_void_ptr Steve Sistare
2024-05-07 21:33   ` Fabiano Rosas
2024-05-27 18:31   ` Peter Xu
2024-05-28 15:10     ` Steven Sistare via
2024-05-28 18:21       ` Peter Xu
2024-05-29 17:30         ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 09/26] migration: vmstate_register_named Steve Sistare
2024-05-09 14:19   ` Fabiano Rosas
2024-05-09 14:32     ` Fabiano Rosas
2024-05-13 19:29       ` Steven Sistare
2024-04-29 15:55 ` [PATCH V1 10/26] migration: vmstate_unregister_named Steve Sistare
2024-04-29 15:55 ` [PATCH V1 11/26] migration: vmstate_register at init time Steve Sistare
2024-04-29 15:55 ` [PATCH V1 12/26] migration: vmstate factory object Steve Sistare
2024-04-29 15:55 ` [PATCH V1 13/26] physmem: ram_block_create Steve Sistare
2024-05-13 18:37   ` Fabiano Rosas
2024-05-13 19:30     ` Steven Sistare
2024-04-29 15:55 ` [PATCH V1 14/26] physmem: hoist guest_memfd creation Steve Sistare
2024-04-29 15:55 ` [PATCH V1 15/26] physmem: hoist host memory allocation Steve Sistare
2024-04-29 15:55 ` [PATCH V1 16/26] physmem: set ram block idstr earlier Steve Sistare
2024-04-29 15:55 ` [PATCH V1 17/26] machine: memfd-alloc option Steve Sistare
2024-05-28 21:12   ` Peter Xu
2024-05-29 17:31     ` Steven Sistare via
2024-05-29 19:14       ` Peter Xu
2024-05-30 17:11         ` Steven Sistare via
2024-05-30 18:14           ` Peter Xu
2024-05-31 19:32             ` Steven Sistare via
2024-06-03 21:48               ` Peter Xu
2024-06-04  7:13                 ` Daniel P. Berrangé
2024-06-04 15:58                   ` Peter Xu
2024-06-04 16:14                     ` David Hildenbrand
2024-06-04 16:41                       ` Peter Xu
2024-06-04 17:16                         ` David Hildenbrand
2024-06-03 10:17       ` Daniel P. Berrangé [this message]
2024-06-03 11:59         ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 18/26] migration: cpr-exec-args parameter Steve Sistare
2024-05-02 12:23   ` Markus Armbruster
2024-05-02 16:00     ` Steven Sistare
2024-05-21  8:13   ` Daniel P. Berrangé
2024-04-29 15:55 ` [PATCH V1 19/26] physmem: preserve ram blocks for cpr Steve Sistare
2024-05-28 21:44   ` Peter Xu
2024-05-29 17:31     ` Steven Sistare via
2024-05-29 19:25       ` Peter Xu
2024-05-30 17:12         ` Steven Sistare via
2024-05-30 18:39           ` Peter Xu
2024-05-31 19:32             ` Steven Sistare via
2024-06-03 22:29               ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 20/26] migration: cpr-exec mode Steve Sistare
2024-05-02 12:23   ` Markus Armbruster
2024-05-02 16:00     ` Steven Sistare
2024-05-03  6:26       ` Markus Armbruster
2024-05-21  8:20   ` Daniel P. Berrangé
2024-05-24 14:58   ` Fabiano Rosas
2024-05-27 18:54     ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 21/26] migration: migrate_add_blocker_mode Steve Sistare
2024-05-09 17:47   ` Fabiano Rosas
2024-04-29 15:55 ` [PATCH V1 22/26] migration: ram block cpr-exec blockers Steve Sistare
2024-05-09 18:01   ` Fabiano Rosas
2024-05-13 19:29     ` Steven Sistare
2024-04-29 15:55 ` [PATCH V1 23/26] migration: misc " Steve Sistare
2024-05-09 18:05   ` Fabiano Rosas
2024-05-24 12:40   ` Fabiano Rosas
2024-05-27 19:02     ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 24/26] seccomp: cpr-exec blocker Steve Sistare
2024-05-09 18:16   ` Fabiano Rosas
2024-05-10  7:54   ` Daniel P. Berrangé
2024-05-13 19:29     ` Steven Sistare
2024-05-21  7:14       ` Daniel P. Berrangé
2024-04-29 15:55 ` [PATCH V1 25/26] migration: fix mismatched GPAs during cpr-exec Steve Sistare
2024-05-09 18:39   ` Fabiano Rosas
2024-04-29 15:55 ` [PATCH V1 26/26] migration: only-migratable-modes Steve Sistare
2024-05-09 19:14   ` Fabiano Rosas
2024-05-13 19:48     ` Steven Sistare
2024-05-13 21:57       ` Fabiano Rosas
2024-05-21  8:05   ` Daniel P. Berrangé
2024-05-02 16:13 ` cpr-exec doc (was Re: [PATCH V1 00/26] Live update: cpr-exec) Steven Sistare
2024-05-02 18:15   ` Peter Xu
2024-05-20 18:30 ` [PATCH V1 00/26] Live update: cpr-exec Steven Sistare
2024-05-20 22:28   ` Fabiano Rosas
2024-05-21  2:31     ` Peter Xu
2024-05-21 11:46       ` Steven Sistare
2024-05-27 17:45         ` Peter Xu
2024-05-28 15:10           ` Steven Sistare via
2024-05-28 16:42             ` Peter Xu
2024-05-30 17:17               ` Steven Sistare via
2024-05-30 19:23                 ` Peter Xu
2024-05-24 13:02 ` Fabiano Rosas
2024-05-24 14:07   ` Steven Sistare
2024-05-27 18:07 ` Peter Xu

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=Zl2YQhv97Ez3_FGn@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=david@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=farosas@suse.de \
    --cc=imammedo@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=steven.sistare@oracle.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.