On Mon, Apr 03, 2023 at 09:13:29AM +0200, David Hildenbrand wrote: > On 01.04.23 19:47, Stefan Hajnoczi wrote: > > On Sat, Apr 01, 2023 at 12:42:57PM +0000, Alexander Graf wrote: > > > Add an option for hostmem-file to start the memory object at an offset > > > into the target file. This is useful if multiple memory objects reside > > > inside the same target file, such as a device node. > > > > > > In particular, it's useful to map guest memory directly into /dev/mem > > > for experimentation. > > > > > > Signed-off-by: Alexander Graf <g...@amazon.com> > > > Reviewed-by: Stefan Hajnoczi <stefa...@gmail.com> > > > > > > --- > > > > > > v1 -> v2: > > > > > > - add qom documentation > > > - propagate offset into truncate, size and alignment checks > > > > > > v2 -> v3: > > > > > > - failed attempt at fixing typo > > > > > > v2 -> v4: > > > > > > - fix typo > > > --- > > > backends/hostmem-file.c | 40 +++++++++++++++++++++++++++++++++++++++- > > > include/exec/memory.h | 2 ++ > > > include/exec/ram_addr.h | 3 ++- > > > qapi/qom.json | 5 +++++ > > > qemu-options.hx | 6 +++++- > > > softmmu/memory.c | 3 ++- > > > softmmu/physmem.c | 14 ++++++++++---- > > > 7 files changed, 65 insertions(+), 8 deletions(-) > > > > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > > The change itself looks good to me, but I do think some other QEMU code that > ends up working on the RAMBlock is not prepared yet. Most probably, because > we never ended up using fd with an offset as guest RAM. > > We don't seem to be remembering that offset in the RAMBlock. First, I > thought block->offset would be used for that, but that's just the offset in > the ram_addr_t space. Maybe we need a new "block->fd_offset" to remember the > offset (unless I am missing something).
I think you're right. > > The real offset in the file would be required at least in two cases I can > see (whenever we essentially end up calling mmap() on the fd again): > > 1) qemu_ram_remap(): We'd have to add the file offset on top of the > calculated offset. > > 2) vhost-user: most probably whenever we set the mmap_offset. For example, > in vhost_user_fill_set_mem_table_msg() we'd similarly have to add the > file_offset on top of the calculated offset. vhost_user_get_mr_data() should > most probably do that. I had a patch to add that offset for the upcoming doublemap feature here: https://lore.kernel.org/all/20230117220914.2062125-8-pet...@redhat.com/ But that was because doublemap wants to map the guest mem twice for other purposes. I didn't yet notice that the code seem to be already broken if without offset==0. While, I _think_ we already have offset!=0 case for a ramblock, since: commit ed5d001916dd46ceed6d8850e453bcd7b5db2acb Author: Jagannathan Raman <jag.ra...@oracle.com> Date: Fri Jan 29 11:46:13 2021 -0500 multi-process: setup memory manager for remote device Where there's: memory_region_init_ram_from_fd(subregion, NULL, name, sysmem_info->sizes[region], RAM_SHARED, msg->fds[region], sysmem_info->offsets[region], errp); Thanks, -- Peter Xu
