Re: [PATCH V2 01/11] machine: alloc-anon option

[Peter Xu <peterx@redhat.com>]

On Sat, Jul 20, 2024 at 04:35:58PM -0400, Steven Sistare wrote:
> On 7/17/2024 3:24 PM, Peter Xu wrote:
> > On Tue, Jul 16, 2024 at 11:19:55AM +0200, Igor Mammedov wrote:
> > > On Sun, 30 Jun 2024 12:40:24 -0700
> > > Steve Sistare <steven.sistare@oracle.com> wrote:
> > > 
> > > > Allocate anonymous memory using mmap MAP_ANON or memfd_create depending
> > > > on the value of the anon-alloc machine property.  This affects
> > > > memory-backend-ram objects, guest RAM created with the global -m option
> > > > but without an associated memory-backend object and without the -mem-path
> > > > option
> > > nowadays, all machines were converted to use memory backend for VM RAM.
> > > so -m option implicitly creates memory-backend object,
> > > which will be either MEMORY_BACKEND_FILE if -mem-path present
> > > or MEMORY_BACKEND_RAM otherwise.
> > > 
> > > 
> > > > To access the same memory in the old and new QEMU processes, the memory
> > > > must be mapped shared.  Therefore, the implementation always sets
> > > 
> > > > RAM_SHARED if alloc-anon=memfd, except for memory-backend-ram, where the
> > > > user must explicitly specify the share option.  In lieu of defining a new
> > > so statement at the top that memory-backend-ram is affected is not
> > > really valid?
> > > 
> > > > RAM flag, at the lowest level the implementation uses RAM_SHARED with fd=-1
> > > > as the condition for calling memfd_create.
> > > 
> > > In general I do dislike adding yet another option that will affect
> > > guest RAM allocation (memory-backends  should be sufficient).
> > 
> > I shared the same concern when reviewing the previous version, and I keep
> > having so.
> > 
> > > 
> > > However I do see that you need memfd for device memory (vram, roms, ...).
> > > Can we just use memfd/shared unconditionally for those and
> > > avoid introducing a new confusing option?
> > 
> > ROMs should be fine IIUC, as they shouldn't be large, and they can be
> > migrated normally (because they're not DMA target from VFIO assigned
> > devices).  IOW, per my understanding what must be shared via memfd is
> > writable memories that can be DMAed from a VFIO device.
> > 
> > I raised such question on whether / why vram can be a DMA target, but I
> > didn't get a response.  So I would like to redo this comment: I think we
> > should figure out what is missing when we switch all backends to use
> > -object, rather than adding this flag easily.  When added, we should be
> > crystal clear on which RAM region will be applicable by this flag.
> 
> All RAM regions that are mapped by the guest are registered for vfio DMA by
> a memory listener and could potentially be DMA'd, either read or written.
> That is defined by the architecture.  We are not allowed to make value
> judgements and decide to not support the architecture for some segments
> such as ROM.

You're right.  However the problem is we have pretty good grasp of the
major DMA target here (guest mem), so what I feel like is some missing work
in this area, that we're not sure what this new parameter is applying to.

It's not the case where we know "OK we have a million use case of RAM, and
we're 100% sure we want to make them all fd-based, and we introduce this
flag simply because adding this to each 1-million will take years and
thousands LOC changes".

The new parameter is cheap to paper over the question being raised here,
but it definitely adds not only ambiguity (when it conflicts with -object)
and that we'll need to maintain its compatibility for all RAMs that we have
totally no idea what can be implied underneath for whatever QEMU cmdline
that can be specified.

IMHO that, OTOH, justifies that there may need some further study to
justify this parameter alone.  For example, if it's only the vRAM that is
missing, we may at least want to have a parameter nailing down to vRAM
behavior rather than affecting anything, so as to at least avoid collision
on -object parameters.

Thanks,

-- 
Peter Xu