On 24.02.25 09:54, Albert Esteve wrote:
On Mon, Feb 17, 2025 at 9:01 PM David Hildenbrand <da...@redhat.com> wrote:
On 17.02.25 17:40, Albert Esteve wrote:
Hi all,
Hi,
looks like our debugging session was successfu :)
One question below.
v3->v4
- Change mmap strategy to use RAM blocks
and subregions.
- Add new bitfield to qmp feature map
- Followed most review comments from
last iteration.
- Merged documentation patch again with
this one. Makes more sense to
review them together after all.
- Add documentation for MEM_READ/WRITE
messages.
The goal of this patch is to support
dynamic fd-backed memory maps initiated
from vhost-user backends.
There are many devices that could already
benefit of this feature, e.g.,
virtiofs or virtio-gpu.
After receiving the SHMEM_MAP/UNMAP request,
the frontend creates the RAMBlock form the
fd and maps it by adding it as a subregion
of the shared memory region container.
The VIRTIO Shared Memory Region list is
declared in the `VirtIODevice` struct
to make it generic.
TODO: There was a conversation on the
previous version around adding tests
to the patch (which I have acknowledged).
However, given the numerous changes
that the patch already has, I have
decided to send it early and collect
some feedback while I work on the
tests for the next iteration.
Given that I have been able to
test the implementation with
my local setup, I am more or less
confident that, at least, the code
is in a relatively sane state
so that no reviewing time is
wasted on broken patches.
This patch also includes:
- SHMEM_CONFIG frontend request that is
specifically meant to allow generic
vhost-user-device frontend to be able to
query VIRTIO Shared Memory settings from the
backend (as this device is generic and agnostic
of the actual backend configuration).
- MEM_READ/WRITE backend requests are
added to deal with a potential issue when having
multiple backends sharing a file descriptor.
When a backend calls SHMEM_MAP it makes
accessing to the region fail for other
backend as it is missing from their translation
table. So these requests are a fallback
for vhost-user memory translation fails.
Can you elaborate what the issue here is?
Why would SHMEM_MAP make accessing the region fail for other backends --
what makes this missing from their translation?
This issue was raised by Stefan Hajnoczi in one of the first
iterations of this patchset, based upon previous David Gilbert's work
on the virtiofs DAX Window.
Let me paste here some of his remarks:
"""
Other backends don't see these mappings. If the guest submits a vring
descriptor referencing a mapping to another backend, then that backend
won't be able to access this memory.
"""
[...]
"""
A bit more detail:
Device A has a VIRTIO Shared Memory Region. An application mmaps that
memory (examples: guest userspace driver using Linux VFIO, a guest
kernel driver that exposes the memory to userspace via mmap, or guest
kernel DAX). The application passes that memory as an I/O buffer to
device B (e.g. O_DIRECT disk I/O).
The result is that device B's vhost-user backend receives a vring
descriptor that points to a guest memory address in device A's VIRTIO
Shared Memory Region. Since device B does not have this memory in its
table, it cannot translate the address and the device breaks.
"""
I have not triggered the issue myself. So the idea is that the next
patch will *definitively* include some testing for the commits that I
cannot verify with my local setup.
Hah! But isn't that exact problem which is now solved by our rework?
Whatever is mapped in the VIRTIO Shared Memory Region will be
communicated to all other vhost-user devices. So they should have that
memory in their map and should be able to access it.
The only thing vhost-user devices cannot access are IIRC ram_device_ptr
memory regions (e.g., from vfio devices). But that is independent shared
memory regions.
--
Cheers,
David / dhildenb
