It's really tricky to handle the cached dmabuf_obj's life-cycle without touching other kernel modules (e.g. i915 or dmabuf).The proposed two ioctls will be helpful.
So, there is a problem about the releasing cached dmabuf_obj. We cannot rely on the drm_i915_gem_object_ops.release() to release the cached dmabuf_obj,
as this release operation is running in another thread, which leads to a racing condition and tricky to be solved without touching other modules.
So, in order to solve that kind of problem, I’d like to add one more ioctl, which is used for user mode to close the dmabuf_obj.
Including the proposed two ioctls, the incremental patch is like this:
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index bf40f7b..6aa6860 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -538,12 +538,33 @@ struct vfio_device_gfx_plane_info {
__u32 y_pos; /* vertical position of cursor plane, upper left corner in lines*/
union {
__u32 region_index; /* region index */
- __s32 fd; /* dma-buf fd */
+ __s32 dmabuf_id; /* dma-buf fd */
};
};
#define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)
+struct vfio_device_gfx_dmabuf_fd {
+ __u32 argsz;
+ __u32 flags;
+ /* in */
+ __u32 dmabuf_id;
+ /* out */
+ __s32 dmabuf_fd;
+};
+
+#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15)
+
+
+struct vfio_device_gfx_buffer {
+ __u32 argsz;
+ __u32 flags;
+ /* in */
+ __u32 id;
+};
+
+#define VFIO_DEVICE_CLOSE_BUF _IO(VFIO_TYPE, VFIO_BASE + 16)
And here are some details:
1. VFIO_DEVICE_QUERY_GFX_PLANE: is used for user mode to ask kernel part to create a buffer (in dmabuf case is dmabuf_obj in kernel) and return the buffer info.
2. VFIO_DEVICE_GET_GFX_DMABUF: is used for user mode to ask kernel part which interface it likes the buffer to be wrapped.
Actually, I think there could be a bunch of types, including DMBUF type.
So, maybe we can change the IOCTL's name to some generic name and use flags field to distinguish them.
In dmabuf case, a new dmabuf will be created each time with this ioctl invoked, and installed with a new fd.
The dmabuf is just a wrapper of kernel dmabuf_obj distinguished by dmabuf_id. So there could be more
than one dmabuf as the wrappers of the same dmabuf_obj, if this ioctl was invoked with the same dmabuf_id
many times. The dmabuf and its fd is fully controlled by user mode. And the VFIO_DEVICE_CLOSE_BUF can
guarantee the referenced dmabuf_obj will be released at last, after all the dmabufs are released.
3. VFIO_DEVICE_CLOSE_BUF: is used for user mode to tell kernel part to release that buffer.
In dmabuf case, this ioctl won't release the dmabuf_obj at once. It just decreases the reference count of the dmabuf_obj
and make sure this dmabuf_obj won't be reused through VFIO_DEVICE_QUERY_GFX_PLANE again. At last, after all the
referencing dmabuf are released by user mode, the dmabuf_obj will be released by kernel.
Thanks,
BR,
Tina
> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@xxxxxxxxxx]
> Sent: Tuesday, September 26, 2017 3:12 PM
> To: Zhang, Tina <tina.zhang@xxxxxxxxx>; zhenyuw@xxxxxxxxxxxxxxx
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Alex
> Williamson <alex.williamson@xxxxxxxxxx>; Daniel Vetter
> <daniel.vetter@xxxxxxxx>
> Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
>
> Hi,
>
> [ bringing a private discussion back to the list ]
>
> > The dma-buf's life cycle is handled by user mode and tracked by
> > kernel.
> > The returned fd in struct vfio_device_query_gfx_plane can be a new fd
> > or an old fd of a re-exported dma-buf. Host user mode can check the
> > value of fd and to see if it needs to create new resource according to
> > the new fd or just use the existed resource related to the old fd.
>
> Ok, this idea has a fundamental flaw: The life cycle of the dma-buf and the file
> handle are not identical. The dma-buf can exist longer than the file handle in
> case other references to the dma-buf exist. So when trying to use the file handle
> as identifier for the dma-buf you'll end up with all sorts of strange effects.
>
> So, I'd suggest to use a id instead, and add a ioctl to get a dmabuf for a given id
> (incremental patch):
>
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -538,12 +538,22 @@ struct vfio_device_gfx_plane_info {
> __u32 y_pos; /* vertical position of cursor plane, upper left corner in
> lines*/
> union {
> __u32 region_index; /* region index */
> - __s32 fd; /* dma-buf fd */
> + __u32 dmabuf_id; /* dma-buf id */
> };
> };
>
> #define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)
>
> +struct vfio_device_gfx_dmabuf_fd {
> + __u32 argsz;
> + __u32 flags;
> + /* in */
> + __u32 dmabuf_id;
> + /* out */
> + __s32 dmabuf_fd;
> +};
> +
> +#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15)
>
> /* -------- API for Type1 VFIO IOMMU -------- */
>
> [ no changes for a region-based display ]
>
> git branch, kernel, with updated dmabuf patch:
> https://www.kraxel.org/cgit/linux/log/?h=gvt-dmabuf-v14
>
> qemu branch:
> https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu
>
> cheers,
> Gerd
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
