On Fri, 05 May 2017 08:55:31 +0200 Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote: > Hi, > > > > >>Hmm, that looks like a rather strange way to return a file descriptor. > > > >> > > > >>What is the reason to not use ioctls on the vfio file handle, like > > > >>older version of these patches did? > > > >If I understood correctly that Alex prefer not to change the ioctls on the vfio file > > > >handle like the old version. > > > >So I used this way the smallest change to general vfio framework only adding a > > > >subregion definition. > > > > I think I was hoping we could avoid a separate file descriptor > > altogether and use a vfio region instead. > > What exactly did you have in mind? Put the framebuffer information > (struct intel_vgpu_dmabuf) into the vfio region, then access it using > read/write/mmap? Yeah, that was my hope. Adding a new file descriptor means we have one more reference floating around complicating the life cycle of the device, group, and container. Furthermore this one is really only visible to the mdev vendor driver, so we can't rely on vfio-core, the vendor driver will need to consider the reference when releasing the device. > > However, it was explained > > previously why this really needs to be a separate fd and I agree that > > using a region to expose an fd is really awkward. > > Now with this patchset we have *two* kinds of separate file handles. > First the anon-fd created by reading from the region. This is then used > to run the intel ioctls on, which in turn create the other kind of file > handle (dma-buf-fd). > > The dma-buf-fd really needs to be a separate fd, because it gets passed > around as handle and because this is the way dma-bufs work (guess this > is the discussion you are referring to). Yep, we're going to need to trust the vendor driver to manage it, we have lots of places where we need to trust the vendor driver for an mdev device, unfortunately. > I can't see a compelling reason for the anon-fd though. I suspect this > was done due to a misunderstanding ... Yeah, vfio-core passes device ioctls to the vendor driver, so the vendor driver should be able to implement a VFIO_DEVICE_GVT_GET_DMABUF_FD ioctl direclty. Ideally maybe this isn't even GVT specific, and we'd s/GVT_//. Thanks, Alex _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
