linux-kernel - Re: [PATCH v18 5/6] vfio: ABI for mdev display dma-buf operation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171115114842.30a0f72e@t450s.home>
Date:   Wed, 15 Nov 2017 11:48:42 -0700
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Tina Zhang <tina.zhang@...el.com>
Cc:     kraxel@...hat.com, chris@...is-wilson.co.uk,
        joonas.lahtinen@...ux.intel.com, zhenyuw@...ux.intel.com,
        zhiyuan.lv@...el.com, zhi.a.wang@...el.com, kevin.tian@...el.com,
        daniel@...ll.ch, kwankhede@...dia.com, hang.yuan@...el.com,
        intel-gfx@...ts.freedesktop.org,
        intel-gvt-dev@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Daniel Vetter <daniel.vetter@...ll.ch>
Subject: Re: [PATCH v18 5/6] vfio: ABI for mdev display dma-buf operation

On Wed, 15 Nov 2017 17:11:54 +0800
Tina Zhang <tina.zhang@...el.com> wrote:

> Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user query and get
> a plane and its 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 returns success only when the specific kind of
> buffer is supported.
> 
> Add VFIO_DEVICE_GET_GFX_DMABUF ioctl command to let user get a specific
> dma-buf fd of an exposed MDEV buffer provided by 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.
> 
> v18:
> - update comments for VFIO_DEVICE_GET_GFX_DMABUF. (Alex)
> 
> v17:
> - modify VFIO_DEVICE_GET_GFX_DMABUF interface. (Alex)
> 
> 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@...el.com>
> Cc: Gerd Hoffmann <kraxel@...hat.com>
> Cc: Alex Williamson <alex.williamson@...hat.com>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> ---
>  include/uapi/linux/vfio.h | 62 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index ae46105..5c1cca2 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -502,6 +502,68 @@ 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.
> + *
> + * Note:
> + * 1. Plane could be disabled by guest. In that case, success will be
> + *    returned with zero-initialized drm_format, size, width and height
> + *    fields.
> + * 2. x_hot/y_hot is set to 0xFFFFFFFF if no hotspot information available
> + *
> + * Return: 0 on success, -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, __u32)
> + *
> + * Return a new dma-buf file descriptor for an exposed guest framebuffer
> + * described by the provided dmabuf_id. The dmabuf_id is returned from VFIO_
> + * DEVICE_QUERY_GFX_PLANE as a token of the exposed guest framebuffer.
> + */
> +
> +#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15)
> +
>  /* -------- API for Type1 VFIO IOMMU -------- */
>  
>  /**

Looks good to me.  Kirti, have you had a chance yet to verify this is
sufficient for your region-based approach?  It would be great to get
confirmation of that ASAP so we can get this in.  Thanks,

Alex