RE: [PATCH v16 5/6] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@xxxxxxxxxxxxxxxxxxxxx] On
> Behalf Of Alex Williamson
> Sent: Monday, November 6, 2017 10:39 AM
> To: Zhang, Tina <tina.zhang@xxxxxxxxx>
> Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>; Daniel Vetter <daniel.vetter@xxxxxxxx>;
> intel-gfx@xxxxxxxxxxxxxxxxxxxxx; joonas.lahtinen@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; zhenyuw@xxxxxxxxxxxxxxx; chris@xxxxxxxxxxxxxxxxxx;
> kwankhede@xxxxxxxxxx; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx>; daniel@xxxxxxxx;
> intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>;
> kraxel@xxxxxxxxxx
> Subject: Re: [PATCH v16 5/6] vfio: ABI for mdev display dma-buf operation
> 
> On Mon,  6 Nov 2017 10:19:17 +0800
> Tina Zhang <tina.zhang@xxxxxxxxx> wrote:
> 
> > Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user query and
> > get a plane and its related information. So far, two types of buffers
> > are
> > supported: buffers based on dma-buf and buffers based on region.
> >
> > This ioctl can be invoked with:
> > 1) either DMABUF or REGION flag. Vendor driver returns a plane_info
> > successfully only when the specific kind of buffer is supported.
> > 2) flag PROBE. And at the same time either DMABUF or REGION must be
> > set, so that vendor driver can return success only when the specific
> > kind of buffer is supported.
> >
> > Add VFIO_DEVICE_GET_GFX_DMABUF ioctl command to let user get an
> > exposed dma-buf fd of a specific dmabuf_id which was returned in
> > VFIO_DEVICE_QUERY _GFX_PLANE ioctl command.
> >
> > The life cycle of an exposed MDEV buffer is handled by userspace and
> > tracked by kernel space. The returned dmabuf_id in struct vfio_device_
> > query_gfx_plane can be a new id of a new exposed buffer or an old id
> > of a re-exported buffer. Host user can check the value of dmabuf_id to
> > see if it needs to create new resources according to the new exposed
> > buffer or just re-use the existing resource related to the old buffer.
> >
> > v16:
> > - add x_hot and y_hot fields. (Gerd)
> > - add comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> > - rebase to 4.14.0-rc6.
> >
> > v15:
> > - add a ioctl to get a dmabuf for a given dmabuf id. (Gerd)
> >
> > v14:
> > - add PROBE, DMABUF and REGION flags. (Alex)
> >
> > v12:
> > - add drm_format_mod back. (Gerd and Zhenyu)
> > - add region_index. (Gerd)
> >
> > v11:
> > - rename plane_type to drm_plane_type. (Gerd)
> > - move fields of vfio_device_query_gfx_plane to vfio_device_gfx_plane_info.
> >   (Gerd)
> > - remove drm_format_mod, start fields. (Daniel)
> > - remove plane_id.
> >
> > v10:
> > - refine the ABI API VFIO_DEVICE_QUERY_GFX_PLANE. (Alex) (Gerd)
> >
> > v3:
> > - add a field gvt_plane_info in the drm_i915_gem_obj structure to save
> >   the decoded plane information to avoid look up while need the plane
> >   info. (Gerd)
> >
> > Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx>
> > Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> > Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > ---
> >  include/uapi/linux/vfio.h | 68
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index ae46105..6c55668 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -502,6 +502,74 @@ 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 flags, then retrieve the gfx plane info.
> > + *
> > + * flags supported:
> > + * - VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_DMABUF
> are set
> > + *   to ask if the mdev supports dma-buf. 0 on support, -EINVAL on no
> > + *   support for dma-buf.
> > + * - VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_REGION
> are set
> > + *   to ask if the mdev supports region. 0 on support, -EINVAL on no
> > + *   support for region.
> > + * - VFIO_GFX_PLANE_TYPE_DMABUF or VFIO_GFX_PLANE_TYPE_REGION
> is set
> > + *   with each call to query the plane info.
> > + * - Others are invalid and return -EINVAL.
> > + *
> > + * Return: 0 on success, -ENODEV with all out fields zero on mdev
> > + * device initialization, -errno on other failure.
> > + */
> > +struct vfio_device_gfx_plane_info {
> > +	__u32 argsz;
> > +	__u32 flags;
> > +#define VFIO_GFX_PLANE_TYPE_PROBE (1 << 0) #define
> > +VFIO_GFX_PLANE_TYPE_DMABUF (1 << 1) #define
> > +VFIO_GFX_PLANE_TYPE_REGION (1 << 2)
> > +	/* 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 */
> > +	__u32 y_pos;	/* vertical position of cursor plane*/
> > +	__u32 x_hot;    /* horizontal position of cursor hotspot */
> > +	__u32 y_hot;    /* vertical position of cursor hotspot */
> > +	union {
> > +		__u32 region_index;	/* region index */
> > +		__u32 dmabuf_id;	/* dma-buf id */
> > +	};
> > +};
> > +
> > +#define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)
> > +
> > +/**
> > + * VFIO_DEVICE_GET_GFX_DMABUF - _IOW(VFIO_TYPE, VFIO_BASE + 15,
> > + *				    struct vfio_device_gfx_dmabuf_fd)
> > + *
> > + * Retrieve dmabuf_fd of an exposed guest framebuffer referenced by
> > +dmabuf_id
> > + * which is returned from VFIO_DEVICE_QUERY_GFX_PLANE as a token of
> > +the
> > + * exposed guest framebuffer.
> > + *
> > + * Return: 0 on success, -errno on failure.
> > + */
> > +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)
> > +
> 
> I thought we had agreed to make this behave similar to
> VFIO_GROUP_GET_DEVICE_FD, the ioctl would take a __u32 dmabuf_id and
> return the file descriptor as the ioctl return value.  Thanks,
If we follow VFIO_GROUP_GET_DEVICE_FD, we would lose flags functionality.
Zhi and Zhenyu, how do you think about it?
Thanks.

BR,
Tina
> 
> Alex
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev