On Sat, 18 Nov 2017 23:29:31 +0530
Kirti Wankhede <kwankh...@nvidia.com> wrote:
> Extremely sorry for the delay.
> This works for VFIO_GFX_PLANE_TYPE_REGION. Tested with local changes.
>
> Reviewed-by: Kirti Wankhede <kwankh...@nvidia.com>
With that,
Acked-by: Alex Williamson <alex.william...@redhat.com>
> On 11/18/2017 9:00 PM, Alex Williamson wrote:
> >
> > Kirti?
> >
> > On Wed, 15 Nov 2017 21:11:42 -0700
> > Alex Williamson <alex.william...@redhat.com> wrote:
> >
> >> On Thu, 16 Nov 2017 11:21:56 +0800
> >> Zhenyu Wang <zhen...@linux.intel.com> wrote:
> >>
> >>> On 2017.11.15 11:48:42 -0700, Alex Williamson wrote:
> >>>> On Wed, 15 Nov 2017 17:11:54 +0800
> >>>> Tina Zhang <tina.zh...@intel.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.zh...@intel.com>
> >>>>> Cc: Gerd Hoffmann <kra...@redhat.com>
> >>>>> Cc: Alex Williamson <alex.william...@redhat.com>
> >>>>> Cc: Daniel Vetter <daniel.vet...@ffwll.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, do you mind by merging this just through gvt tree? Or you would
> >>> like to merge this then send pull request for us then we can apply left?
> >>>
> >>
> >> I'm fine with this going through the gvt tree, but I want some
> >> confirmation from nvidia that this meets their needs as well before I
> >> provide an ack. Thanks,
> >>
> >> Alex
