* Peter Xu (pet...@redhat.com) wrote:
> On Tue, Jan 24, 2023 at 10:06:48AM +0000, Dr. David Alan Gilbert wrote:
> > * Peter Xu (pet...@redhat.com) wrote:
> > > Add a helper to do mmap() for a ramblock based on the cached informations.
> > >
> > > A trivial thing to mention is we need to move ramblock->fd setup to be
> > > earlier, before the ramblock_file_map() call, because it'll need to
> > > reference the fd being mapped. However that should not be a problem at
> > > all, majorly because the fd won't be freed if successful, and if it failed
> > > the fd will be freeed (or to be explicit, close()ed) by the caller.
> > >
> > > Export it - prepare to be used outside this file.
> > >
> > > Signed-off-by: Peter Xu <pet...@redhat.com>
> > > ---
> > > include/exec/ram_addr.h | 1 +
> > > softmmu/physmem.c | 25 +++++++++++++++++--------
> > > 2 files changed, 18 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> > > index 0bf9cfc659..56db25009a 100644
> > > --- a/include/exec/ram_addr.h
> > > +++ b/include/exec/ram_addr.h
> > > @@ -98,6 +98,7 @@ bool ramblock_is_pmem(RAMBlock *rb);
> > >
> > > long qemu_minrampagesize(void);
> > > long qemu_maxrampagesize(void);
> > > +void *ramblock_file_map(RAMBlock *block);
> > >
> > > /**
> > > * qemu_ram_alloc_from_file,
> > > diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> > > index 6096eac286..cdda7eaea5 100644
> > > --- a/softmmu/physmem.c
> > > +++ b/softmmu/physmem.c
> > > @@ -1532,17 +1532,31 @@ static int file_ram_open(const char *path,
> > > return fd;
> > > }
> > >
> > > +/* Do the mmap() for a ramblock based on information already setup */
> > > +void *ramblock_file_map(RAMBlock *block)
> > > +{
> > > + uint32_t qemu_map_flags;
> > > +
> > > + qemu_map_flags = (block->flags & RAM_READONLY) ? QEMU_MAP_READONLY :
> > > 0;
> > > + qemu_map_flags |= (block->flags & RAM_SHARED) ? QEMU_MAP_SHARED : 0;
> > > + qemu_map_flags |= (block->flags & RAM_PMEM) ? QEMU_MAP_SYNC : 0;
> > > + qemu_map_flags |= (block->flags & RAM_NORESERVE) ?
> > > QEMU_MAP_NORESERVE : 0;
> > > +
> > > + return qemu_ram_mmap(block->fd, block->mmap_length, block->mr->align,
> > > + qemu_map_flags, block->file_offset);
> > > +}
> > > +
> > > static void *file_ram_alloc(RAMBlock *block,
> > > int fd,
> > > bool truncate,
> > > off_t offset,
> > > Error **errp)
> > > {
> > > - uint32_t qemu_map_flags;
> > > void *area;
> > >
> > > /* Remember the offset just in case we'll need to map the range
> > > again */
> >
> > Note that this comment is now wrong; you need to always set that for the
> > map call.
>
> This line is added in patch 7. After this patch, a ramblock should always
> be mapped with ramblock_file_map(), so it keeps being true?
With ramblock_file_map() it's not a 'just in case' any more though is
it? This value always goes through the block-> now?
Dave
> >
> > Other than that,
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>
> Thanks,
>
> --
> Peter Xu
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
