[Intel-gfx] [PATCH v13 5/7] vfio: ABI for mdev display dma-buf operation

[Alex Williamson]

On Thu, 3 Aug 2017 07:08:15 +0000
"Zhang, Tina" <tina.zhang at intel.com> wrote:

> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson at redhat.com]
> > Sent: Thursday, August 3, 2017 11:38 AM
> > To: Zhang, Tina <tina.zhang at intel.com>
> > Cc: Tian, Kevin <kevin.tian at intel.com>; intel-gfx at lists.freedesktop.org; dri-
> > devel at lists.freedesktop.org; kwankhede at nvidia.com; kraxel at redhat.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 v13 5/7] vfio: ABI for mdev display dma-buf operation
> > 
> > On Thu, 3 Aug 2017 03:17:09 +0000
> > "Zhang, Tina" <tina.zhang at intel.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On
> > > > Behalf Of Alex Williamson
> > > > Sent: Wednesday, August 2, 2017 5:18 AM
> > > > To: Zhang, Tina <tina.zhang at intel.com>
> > > > Cc: Tian, Kevin <kevin.tian at intel.com>; kraxel at redhat.com; intel-
> > > > gfx at lists.freedesktop.org; Wang, Zhi A <zhi.a.wang at intel.com>;
> > > > kwankhede at nvidia.com; dri-devel at lists.freedesktop.org; intel-gvt-
> > > > dev at lists.freedesktop.org; Lv, Zhiyuan <zhiyuan.lv at intel.com>
> > > > Subject: Re: [PATCH v13 5/7] vfio: ABI for mdev display dma-buf
> > > > operation
> > > >
> > > > On Tue, 25 Jul 2017 17:28:18 +0800
> > > > Tina Zhang <tina.zhang at intel.com> wrote:
> > > >  
> > > > > 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>
> > > > > ---
> > > > >  include/uapi/linux/vfio.h | 28 ++++++++++++++++++++++++++++
> > > > >  1 file changed, 28 insertions(+)
> > > > >
> > > > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > > > > index ae46105..827a230 100644
> > > > > --- a/include/uapi/linux/vfio.h
> > > > > +++ b/include/uapi/linux/vfio.h
> > > > > @@ -502,6 +502,34 @@ 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 */
> > > > > +	__u64 drm_format_mod;   /* tiled mode */
> > > > > +	__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*/  
> > > > > +	__u32 region_index;
> > > > > +	__s32 fd;	/* dma-buf fd */  
> > > >
> > > > How do I know which of these is valid, region_index or fd?  Can I
> > > > ask for one vs the other?  What are the errno values to
> > > > differentiate unsupported vs not initialized?  Is there a "probe"
> > > > flag that I can use to determine what the device supports without allocating  
> > those resources yet?  
> > > Dma-buf won't use region_index, which means region_index is zero all the  
> > time for dma-buf usage.  
> > > As we discussed, there won't be errno if not initialized, just keep all fields zero.
> > > I will add the comments about these in the next version. Thanks  
> > 
> > Zero is a valid region index.  
> If zero is valid, can we find out other invalid value? How about 0xffffffff?

Unlikely, but also valid.  Isn't this why we have a flags field in the
structure, so we don't need to rely on implicit values as invalid.
Also, all of the previously discussed usage models needs to be
documented, either here in the header or in a Documentation/ file.
Thanks,

Alex