[Intel-gfx] [PATCH v11 1/1] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> Behalf Of Zhenyu Wang
> Sent: Wednesday, July 12, 2017 5:49 PM
> To: Daniel Vetter <daniel at ffwll.ch>
> Cc: Tian, Kevin <kevin.tian at intel.com>; intel-gfx at lists.freedesktop.org;
> alex.williamson at redhat.com; zhenyuw at linux.intel.com; chris at chris-
> wilson.co.uk; kwankhede at nvidia.com; kraxel at redhat.com; Zhang, Tina
> <tina.zhang at intel.com>; intel-gvt-dev at lists.freedesktop.org; Wang, Zhi A
> <zhi.a.wang at intel.com>; Lv, Zhiyuan <zhiyuan.lv at intel.com>
> Subject: Re: [PATCH v11 1/1] vfio: ABI for mdev display dma-buf operation
> 
> On 2017.07.12 09:56:11 +0200, Daniel Vetter wrote:
> > On Wed, Jul 12, 2017 at 06:56:53AM +0000, Zhang, Tina wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Tina
> > > > Sent: Wednesday, July 12, 2017 2:11 PM
> > > > To: alex.williamson at redhat.com; kraxel at redhat.com;
> > > > chris at chris-wilson.co.uk; zhenyuw at linux.intel.com; Lv, Zhiyuan
> > > > <zhiyuan.lv at intel.com>; Wang, Zhi A <zhi.a.wang at intel.com>; Tian,
> > > > Kevin <kevin.tian at intel.com>; daniel at ffwll.ch;
> > > > kwankhede at nvidia.com
> > > > Cc: Zhang, Tina <tina.zhang at intel.com>;
> > > > intel-gfx at lists.freedesktop.org; intel-
> > > > gvt-dev at lists.freedesktop.org
> > > > Subject: [PATCH v11 1/1] vfio: ABI for mdev display dma-buf
> > > > operation
> > > >
> > > > Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user mode
> > > > query and get the plan and its related information.
> > > >
> > > > 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.
> > > >
> > > > Signed-off-by: Tina Zhang <tina.zhang at intel.com>
> > > >
> > > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > > > index
> > > > ae46105..c176cc8 100644
> > > > --- a/include/uapi/linux/vfio.h
> > > > +++ b/include/uapi/linux/vfio.h
> > > > @@ -502,6 +502,32 @@ struct vfio_pci_hot_reset {
> > > >
> > > >  #define VFIO_DEVICE_PCI_HOT_RESET	_IO(VFIO_TYPE, VFIO_BASE +
> 13)
> > > >
> > > > +/**
> > > > + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE +
> 14,
> > > > struct
> > > > +vfio_device_query_gfx_plane)
> > > > + *
> > > > + * Set the drm_plane_type and retrieve information about the gfx plane.
> > > > + *
> > > > + * Return: 0 on success, -errno on failure.
> > > > + */
> > > > +struct vfio_device_gfx_plane_info {
> > > > +	__u32 argsz;
> > > > +	__u32 flags;
> > > > +	/* in */
> > > > +	__u32 drm_plane_type;	/* type of plane: DRM_PLANE_TYPE_* */
> > > > +	/* out */
> > > > +	__u32 drm_format;	/* drm format of plane */
> > > > +	__u32 width;	/* width of plane */
> > > > +	__u32 height;	/* height of plane */
> > > > +	__u32 stride;	/* stride of plane */
> > > > +	__u32 size;	/* size of plane in bytes, align on page*/
> > > > +	__u32 x_pos;	/* horizontal position of cursor plane, upper left corner
> > > > in pixels */
> > > > +	__u32 y_pos;	/* vertical position of cursor plane, upper left corner in
> > > > lines*/
> > > > +	__s32 fd;	/* dma-buf fd */
> > > > +};
> > > > +
> > > I remove plane_id as I cannot find its meaning for dmabuf usage. Also, I
> don't know how it could be used by region usage.
> > > So, if someone can explain its usage, I can add it back. Thanks.
> > >
> > > Another open is, so far, our design is focused on dmabuf based gfx
> > > plane only. Can we go a step further to consider a general Buffer
> > > sharing interface? If we reference V4L2, we can see dmabuf can be
> > > considered as one kind of buffers, we can have  more kinds of
> > > buffers, like mmap buffer and so on. So, if we think about that, we may
> need another ioctl to ask the mdev device which kind of buffer it supports, and
> then use the VFIO_DEVICE_QUERY_* ioctl to get it back for user mode accessing.
> Different kinds of mdev devices can have different query ioctl name and
> structure. Is it reasonable?
> >
> > This is what dma-buf are for. v4l2 also supports dma-buf, and dma-buf
> > support mmap. I think dma-buf will give you everything you want.
> >
> 
> yep, I think Tina's point is to how to provide that dmabuf interface properly,
> either in this case for vgpu display specifically or any benefit to provide a generic
> buffer expose/share interface for vfio/mdev. But even for vgpu display interface,
> e.g gvt driver would go with dmabuf but nv driver will go with region based,
> then I don't think we could easily have a generic enough design for every mdev
> type or driver. So I believe we should stick with and hopefully get aligned for this
> interface now.
Thanks, Zhenyu. I'm just wondering to know if it is reasonable to support a general buffer expose/share interface for vfio/mdev device. After all, what we do here is to provide a way to expose/share the mdev buffer to host Apps, (not sure whether different mdev devices would also be interested in this), e.g. Qemu. Is it possible that we define a genera way to support different kinds of buffers? For example, could region be a kind of buffer existing with dma-buf type of buffer. Is there any other flavor of buffer we want to support, if we take some reference for V4L2 to buffer support
Enum v4l2_memory {
	V4L2_MEMORY_MMAP
	V4L2_MEMORY_USERPTR
	V4L2_MEMORY_OVERLAY
	V4L2_MEMORY_DMABUF
}
Thanks.

Tina
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827