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 <graf@amazon.com>
Reviewed-by: Stefan Hajnoczi <stefanha@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 <stefanha@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-peterx@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.raman@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);