Re: [PATCH V8 04/39] memory: RAM_ANON flag

[David Hildenbrand]

On 15.06.22 16:51, Steve Sistare wrote:
> A memory-backend-ram or a memory-backend-memfd block with the RAM_SHARED
> flag set is not migrated when migrate_ignore_shared() is true, but this
> is wrong, because it has no named backing store, and its contents will be
> lost.  Define a new flag RAM_ANON to distinguish this case.  Cpr will also
> test this flag, for similar reasons.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  backends/hostmem-epc.c   |  2 +-
>  backends/hostmem-memfd.c |  1 +
>  backends/hostmem-ram.c   |  1 +
>  include/exec/memory.h    |  3 +++
>  include/exec/ram_addr.h  |  1 +
>  migration/ram.c          |  3 ++-
>  softmmu/physmem.c        | 12 +++++++++---
>  7 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/backends/hostmem-epc.c b/backends/hostmem-epc.c
> index 037292d..cb06255 100644
> --- a/backends/hostmem-epc.c
> +++ b/backends/hostmem-epc.c
> @@ -37,7 +37,7 @@ sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, 
> Error **errp)
>      }
>  
>      name = object_get_canonical_path(OBJECT(backend));
> -    ram_flags = (backend->share ? RAM_SHARED : 0) | RAM_PROTECTED;
> +    ram_flags = (backend->share ? RAM_SHARED : 0) | RAM_PROTECTED | MAP_ANON;

I'm pretty sure that doesn't compile. -> RAM_ANON

>      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
>                                     name, backend->size, ram_flags,
>                                     fd, 0, errp);
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index 3fc85c3..c9d8001 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, 
> Error **errp)
>      name = host_memory_backend_get_name(backend);
>      ram_flags = backend->share ? RAM_SHARED : 0;
>      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
> +    ram_flags |= RAM_ANON;
>      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
>                                     backend->size, ram_flags, fd, 0, errp);
>      g_free(name);
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index b8e55cd..5e80149 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -30,6 +30,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error 
> **errp)
>      name = host_memory_backend_get_name(backend);
>      ram_flags = backend->share ? RAM_SHARED : 0;
>      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
> +    ram_flags |= RAM_ANON;
>      memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), 
> name,
>                                             backend->size, ram_flags, errp);
>      g_free(name);
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index f1c1945..0daddd7 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -203,6 +203,9 @@ typedef struct IOMMUTLBEvent {
>  /* RAM that isn't accessible through normal means. */
>  #define RAM_PROTECTED (1 << 8)
>  
> +/* RAM has no name outside the qemu process. */
> +#define RAM_ANON (1 << 9)

That name is a bit misleading because it mangles anonymous memory with
an anonymous file, which doesn't provide anonymous memory in "kernel
speak". Please find a better name, some idea below ...

I think what you actual want to know is: is this from a real file,
instead of from an anonymous file or anonymous memory. A real file can
be re-opened and remapped after closing QEMU. Further, you need
MAP_SHARED semantics.


/* RAM maps a real file instead of an anonymous file or no file/fd. */
#define RAM_REAL_FILE (1 << 9)

bool ramblock_maps_real_file(RAMBlock *rb)
{
    return rb->flags & RAM_REAL_FILE;
}


Maybe we can come up with a better name for "real file".


Set the flag from applicable callsites. When setting the flag
internally, assert that we don't have a fd -- that cannot possibly make
sense.

At applicable callsites check for ramblock_maps_real_file() and that
it's actually a shared mapping. If not, it cannot be preserved by
restarting QEMU (easily, there might be ways for memfd involving other
processes).


Make sense?

-- 
Thanks,

David / dhildenb