On 2025/11/18 14:22, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [PATCH v2 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs
associated with VFIO devices
On 2025/11/17 13:19, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [PATCH v2 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs
associated with VFIO devices
On 2025/11/13 12:17, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [PATCH v2 10/10] virtio-gpu-dmabuf: Create dmabuf for
blobs
associated with VFIO devices
On 2025/11/12 13:26, Kasireddy, Vivek wrote:
Hi Akihiko,
Subject: Re: [PATCH v2 10/10] virtio-gpu-dmabuf: Create dmabuf for
blobs
associated with VFIO devices
On 2025/11/09 14:33, Vivek Kasireddy wrote:
In addition to memfd, a blob resource can also have its backing
storage in a VFIO device region. Therefore, we first need to figure
out if the blob is backed by a VFIO device region or a memfd before
we can call the right API to get a dmabuf fd created.
So, once we have the ramblock and the associated mr, we rely on
memory_region_is_ram_device() to tell us where the backing storage
is located. If the blob resource is VFIO backed, we try to find the
right VFIO device that contains the blob and then invoke the API
vfio_device_create_dmabuf().
Note that in virtio_gpu_remap_udmabuf(), we first try to test if
the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
use the VFIO device fd directly to create the CPU mapping.
I have just remembered that mremap() will work for either of udmabuf
and
VFIO. That will avoid having two different methods and make
vfio_get_region_index_from_mr() and vfio_device_get_region_info()
unnecessary.
IIUC, the name virtio_gpu_remap_dmabuf() is misleading because we
are
not
actually doing remap but are simply calling mmap(). In other words, we
are not
expanding or shrinking existing mapping but are creating a new
mapping.
And, for dmabufs associated with VFIO devices, without having to call
vfio_get_region_index_from_mr() and vfio_device_get_region_info(), I
don't see
any other way to determine the region offset.
So, I guess I'll create a new patch to do s/remapped/map.
I mean calling mremap() with 0 as the old_size parameter. The man page
says:
> If the value of old_size is zero, and old_address refers to a
> shareable mapping (see the description of MAP_SHARED in
mmap(2)),
then
> mremap() will create a new mapping of the same pages.
It might be possible to use mremap() here but using mmap() seems very
straightforward given that we are actually not shrinking or expanding
an existing mapping but are instead creating a new mapping. Also, I am
wondering what benefit would mremap() bring as opposed to just using
mmap()?
As I noted earlier, mremap() removes the need of having two different
paths for udmabuf and VFIO, and make vfio_get_region_index_from_mr()
and
vfio_device_get_region_info() unnecessary, reducing code complexity.
Sorry, I should have researched thoroughly before but after looking at the
code
again, I don't see how mremap() removes the need for having two different
paths for udmabuf and VFIO and make vfio_get_region_index_from_mr()
and vfio_device_get_region_info() unnecessary. Could you please elaborate
how it can be done?
Not tested, but something like the following:
head = qemu_ram_mmap(-1, res->blob_size, qemu_real_host_page_size(),
QEMU_MAP_READONLY | QEMU_MAP_SHARED, 0);
if (head == MAP_FAILED) {
return NULL;
}
cursor = head;
for (i = 0; i < res->iov_cnt; i++) {
if (mremap(res->iov[i].iov_base, 0, res->iov[i].iov_len,
MREMAP_FIXED, cursor) == MAP_FAILED) {
This is very elegant and I can now see how it is expected to work. However,
I went ahead and tested it and it does not seem to work for VFIO backed
buffers. It works for buffers based out of System RAM though. Here is the
actual code I tested with that I am unconditionally calling for both VFIO
and udmabuf cases:
static void *vfio_dmabuf_mmap2(struct virtio_gpu_simple_resource *res,
VFIODevice *vdev)
{
void *head, *cursor;
int i;
head = qemu_ram_mmap(-1, res->blob_size, qemu_real_host_page_size(),
QEMU_MAP_READONLY |
QEMU_MAP_SHARED, 0);
By the way, please do:
head = mmap(NULL, res->blob_size, PROT_NONE, MAP_SHARED, -1, 0);
I forgot that we don't need to map a RAM but mmap() with PROT_NONE is
sufficient. It will catch a bug that fails to mmap() a real resource on
top of it.
if (head == MAP_FAILED) {
return head;
}
cursor = head;
for (i = 0; i < res->iov_cnt; i++) {
if (mremap(res->iov[i].iov_base, 0, res->iov[i].iov_len,
MREMAP_FIXED | MREMAP_MAYMOVE, cursor) == MAP_FAILED) {
goto err;
}
cursor += res->iov[i].iov_len;
}
return head;
err:
qemu_ram_munmap(-1, head, res->blob_size);
return MAP_FAILED;
}
It (mremap) initially errored with -EINVAL in all cases but adding
MREMAP_MAYMOVE
fixed it for buffers based out of RAM but for VFIO backed buffers, it seems to
be
throwing -EFAULT/Bad Address error. I did not yet check why or where the kernel
driver is returning this error from.
The man page says that EFAULT means:
> Some address in the range old_address to old_address+old_size is an
> invalid virtual memory address for this process. You can also get
> EFAULT even if there exist mappings that cover the whole address space
> requested, but those mappings are of different types.
None of this should be true so it should be a bug, though I'm not sure
if it is a bug of QEMU, Linux, or the man page (i.e., the man page
failed to mention another failure scenario). In any case it needs to be
debugged.
Regards,
Akihiko Odaki
