Re: [PATCH 2/3] memory: add memory_region_is_mapped_shared()

[Stefan Hajnoczi]

On Wed, Feb 24, 2021 at 02:33:32PM +0400, Marc-André Lureau wrote:
> On Mon, Feb 22, 2021 at 8:11 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > Add a function to query whether a memory region is mmap(MAP_SHARED).
> > This will be used to check that vhost-user memory regions can be shared
> > with the device backend process in the next patch.
> >
> > An inline function in "exec/memory.h" would have been nice but RAMBlock
> > fields are only accessible from memory.c (see "exec/ramblock.h").
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  include/exec/memory.h | 11 +++++++++++
> >  softmmu/memory.c      |  6 ++++++
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index c6fb714e49..7b7dbe9fd0 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -2457,6 +2457,17 @@ static inline bool
> > memory_access_is_direct(MemoryRegion *mr, bool is_write)
> >      }
> >  }
> >
> > +/**
> > + * memory_region_is_mapped_shared: check whether a memory region is
> > + * mmap(MAP_SHARED)
> > + *
> > + * Returns %true is a memory region is mmap(MAP_SHARED). This is always
> > false
> > + * on memory regions that do not support memory_region_get_ram_ptr().
> > + *
> > + * @mr: the memory region being queried
> > + */
> > +bool memory_region_is_mapped_shared(MemoryRegion *mr);
> > +
> >  /**
> >   * address_space_read: read from an address space.
> >   *
> > diff --git a/softmmu/memory.c b/softmmu/memory.c
> > index 874a8fccde..e6631e5d4c 100644
> > --- a/softmmu/memory.c
> > +++ b/softmmu/memory.c
> > @@ -1809,6 +1809,12 @@ bool memory_region_is_ram_device(MemoryRegion *mr)
> >      return mr->ram_device;
> >  }
> >
> > +bool memory_region_is_mapped_shared(MemoryRegion *mr)
> > +{
> > +    return memory_access_is_direct(mr, false) &&
> >
> 
> memory_access_is_direct is a bit special, it treats ram-device differently
> from rom-device since commit 4a2e242bbb306 ("memory: Don't use memcpy for
> ram_device regions").
> 
> I don't think it's an issue for now, but I wonder if in the future we will
> share ram-device as well.
> 
> Where did you get the idea to use that function? I suppose that's also the
> one we use somewhere else in the code path sharing the VM memory.

It's used right before qemu_map_ram_ptr() in address_space_read() and
seems like the most comprehensive way to check if a MemoryRegion is
accessed via memory loads/stores.

Do you prefer just memory_region_is_ram()? Or something more complicated
to also allow romd?

Stefan

signature.asc
Description: PGP signature