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

[Peter Xu]

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.

PS to Steve: and I think I left tons of other comments in previous version
outside this patch too, but I don't think they're fully discussed when this
series was sent.  I can re-read the series again, but I don't think it'll
work out if we keep skipping discussions..

Thanks,

-- 
Peter Xu