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?
Thanks, Vivek > > mremap() is also sufficiently straightforward. The man page explicitly > states it is designed to create a new mapping. Using it for the purpose > (not shrinking or expanding an existing mapping) is not an abuse of the API. > > Regards, > Akihiko Odaki
