| Message ID | 1509934758-5003-6-git-send-email-tina.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 6 Nov 2017 10:19:17 +0800 Tina Zhang <tina.zhang@intel.com> 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@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Alex Williamson <alex.williamson@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > 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, Alex
> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > Behalf Of Alex Williamson > Sent: Monday, November 6, 2017 10:39 AM > To: Zhang, Tina <tina.zhang@intel.com> > Cc: Tian, Kevin <kevin.tian@intel.com>; Daniel Vetter <daniel.vetter@ffwll.ch>; > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com; linux- > kernel@vger.kernel.org; zhenyuw@linux.intel.com; chris@chris-wilson.co.uk; > kwankhede@nvidia.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; daniel@ffwll.ch; > intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>; > kraxel@redhat.com > 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@intel.com> 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@intel.com> > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Alex Williamson <alex.williamson@redhat.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > --- > > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
Hi, > +/** > + * 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. Should also not here that it is not an error if the guest has not defined a plane yet. The ioctl should return success in that case and zero-initialize plane info (drm_format + size + width + height fields). > + */ > +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_* */ Add a head field here? People asked @ kvm forum about multihead support. Even if the initial driver version doesn't support it we could add a field so it becomes easier to add it at some point in the future. Probing for available heads could be done with the PROBE flag, i.e. flags = PROBE | DMABUF, plane_type = PRIMARY, head = 0, 1, ... > + __u32 x_hot; /* horizontal position of cursor hotspot */ > + __u32 y_hot; /* vertical position of cursor hotspot */ Needs documentation how the driver signals "no hotspot information available" (using 0xffffffff for example). cheers, Gerd
Hi, > > 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? The ioctl is simple enough that not having flags should not be a problem I think. Also note that dmabuf_id is received using the PLANE_INFO ioctl, so should the need arise to negotiate something in the future chances are high that it can be done using the PLANE_INFO ioctl flags. cheers, Gerd
On Mon, 06 Nov 2017 10:05:34 +0100 Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > > > 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? > > The ioctl is simple enough that not having flags should not be a > problem I think. > > Also note that dmabuf_id is received using the PLANE_INFO ioctl, so > should the need arise to negotiate something in the future chances are > high that it can be done using the PLANE_INFO ioctl flags. Right, the ioctl is "get fd for thing" so we have control of "thing". I think we had this same discussion on v15. Thanks, Alex
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Tuesday, November 7, 2017 4:37 AM > To: Gerd Hoffmann <kraxel@redhat.com> > Cc: Zhang, Tina <tina.zhang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; > Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org; > joonas.lahtinen@linux.intel.com; linux-kernel@vger.kernel.org; > zhenyuw@linux.intel.com; chris@chris-wilson.co.uk; kwankhede@nvidia.com; > Lv, Zhiyuan <zhiyuan.lv@intel.com>; daniel@ffwll.ch; intel-gvt- > dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com> > Subject: Re: [PATCH v16 5/6] vfio: ABI for mdev display dma-buf operation > > On Mon, 06 Nov 2017 10:05:34 +0100 > Gerd Hoffmann <kraxel@redhat.com> wrote: > > > Hi, > > > > > > 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? > > > > The ioctl is simple enough that not having flags should not be a > > problem I think. > > > > Also note that dmabuf_id is received using the PLANE_INFO ioctl, so > > should the need arise to negotiate something in the future chances are > > high that it can be done using the PLANE_INFO ioctl flags. > > Right, the ioctl is "get fd for thing" so we have control of "thing". > I think we had this same discussion on v15. Thanks, Then, OK. Thanks. BR, Tina > > Alex
> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > Behalf Of Gerd Hoffmann > Sent: Monday, November 6, 2017 5:01 PM > To: Zhang, Tina <tina.zhang@intel.com>; alex.williamson@redhat.com; > chris@chris-wilson.co.uk; joonas.lahtinen@linux.intel.com; > zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; Wang, Zhi A > <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; daniel@ffwll.ch; > kwankhede@nvidia.com > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org; > intel-gvt-dev@lists.freedesktop.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v16 5/6] vfio: ABI for mdev display dma-buf operation > > Hi, > > > +/** > > + * 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. > > Should also not here that it is not an error if the guest has not defined a plane > yet. The ioctl should return success in that case and zero-initialize plane info > (drm_format + size + width + height fields). Indeed. I just need to update the comments, as this version is implemented with this in mind. Thanks. > > > + */ > > +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_* */ > > Add a head field here? People asked @ kvm forum about multihead support. > Even if the initial driver version doesn't support it we could add a field so it > becomes easier to add it at some point in the future. > > Probing for available heads could be done with the PROBE flag, i.e. > flags = PROBE | DMABUF, plane_type = PRIMARY, head = 0, 1, ... Does the multihead mean multiple display monitor? So each head can have its own one primary plane, one cursor plane and maybe some sprite planes if supported. And the flow could be like this: 1) flags = PROBE | DMABUF, then return the number of heads in vfio_device_gfx_plane_info.head. 2) flags = DMABUF with plane_type = PRIMARY, head = 0, 1, ..., then return the id of the exposed framebuffer of that head. Am I correct? Another question is if the sprite plane is supported, how we're going to identify them, as there could be several sprite planes for one display monitor. Add another field "num_plane" for it? Then, I guess the flow could be like this: 1) flags = PROBE | DMABUF, then return the number of heads in vfio_device_gfx_plane_info.head. 2) flags = PROBE | DMABUF and head = 0, 1, ..., and plane_type = PRIMARY/CURSOR/SPRITE, then return the num_plane. 3) flags = DMABUF with plane_type = PRIMARY, head = 0, 1, ..., num_plane =0, 1, ..., then return the id of the exposed framebuffer of that head. Does it make sense? Thanks. > > > + __u32 x_hot; /* horizontal position of cursor hotspot */ > > + __u32 y_hot; /* vertical position of cursor hotspot */ > > Needs documentation how the driver signals "no hotspot information available" > (using 0xffffffff for example). This version has implemented this. I will update the comments. Thanks. BR, Tina > > cheers, > Gerd > > _______________________________________________ > intel-gvt-dev mailing list > intel-gvt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
Hi, > > Add a head field here? People asked @ kvm forum about multihead > > support. > > Even if the initial driver version doesn't support it we could add > > a field so it > > becomes easier to add it at some point in the future. > > > > Probing for available heads could be done with the PROBE flag, i.e. > > flags = PROBE | DMABUF, plane_type = PRIMARY, head = 0, 1, ... > > Does the multihead mean multiple display monitor? So each head can > have its own one primary plane, one cursor plane and maybe some > sprite planes if supported. Yes and yes. > And the flow could be like this: > 1) flags = PROBE | DMABUF, then return the number of heads in > vfio_device_gfx_plane_info.head. I'd suggest to use { .flags = PROBE | DMABUF, .head = 0 } to check whenever dmabuf is supported for head 0, then { .flags = PROBE | DMABUF, .head = 1 } to check for head 1 support, ... Driver returns success if the head is supported and -EINVAL otherwise, simliar to the dmabuf vs. region probing. It is less confusing because .head is always an input field then. It is also a bit more flexible because different heads can support different modes (I don't expect drivers will actually use that though). > 2) flags = DMABUF with plane_type = PRIMARY, head = 0, 1, ..., then > return the id of the exposed framebuffer of that head. Yes. > Another question is if the sprite plane is supported, how we're going > to identify them, as there could be several sprite planes for one > display monitor. Do you mean DRM_PLANE_TYPE_OVERLAY? > Add another field "num_plane" for it? Then, I guess the flow could be > like this: > 1) flags = PROBE | DMABUF, then return the number of heads in > vfio_device_gfx_plane_info.head. > 2) flags = PROBE | DMABUF and head = 0, 1, ..., and plane_type = > PRIMARY/CURSOR/SPRITE, then return the num_plane. I'd suggest to do it simliar to the head probe outlined above, i.e. userspace calls { .flags = PROBE | DMABUF, .head = 0, .plane_type = OVERLAY, plane_num = 0, 1, 2, ... } and the driver returns -EINVAL or 0 depending on whenever it is supported or not. cheers, Gerd
On Tue, 07 Nov 2017 09:03:21 +0100 Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > > > Add a head field here? People asked @ kvm forum about multihead > > > support. > > > Even if the initial driver version doesn't support it we could add > > > a field so it > > > becomes easier to add it at some point in the future. > > > > > > Probing for available heads could be done with the PROBE flag, i.e. > > > flags = PROBE | DMABUF, plane_type = PRIMARY, head = 0, 1, ... > > > > Does the multihead mean multiple display monitor? So each head can > > have its own one primary plane, one cursor plane and maybe some > > sprite planes if supported. > > Yes and yes. > > > And the flow could be like this: > > 1) flags = PROBE | DMABUF, then return the number of heads in > > vfio_device_gfx_plane_info.head. > > I'd suggest to use { .flags = PROBE | DMABUF, .head = 0 } to check > whenever dmabuf is supported for head 0, then { .flags = PROBE | > DMABUF, .head = 1 } to check for head 1 support, ... > > Driver returns success if the head is supported and -EINVAL otherwise, > simliar to the dmabuf vs. region probing. > > It is less confusing because .head is always an input field then. > > It is also a bit more flexible because different heads can support > different modes (I don't expect drivers will actually use that though). Should we then specify the error code for "head doesn't exist" vs "head doesn't support dmabuf", with the former taking precedence? Perhaps -ENODEV vs -EINVAL. Are the heads guaranteed to be contiguous (the first -ENODEV is the end of possible heads)? Thanks, Alex
> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > Behalf Of Gerd Hoffmann > Sent: Tuesday, November 7, 2017 4:03 PM > To: Zhang, Tina <tina.zhang@intel.com>; alex.williamson@redhat.com; > chris@chris-wilson.co.uk; joonas.lahtinen@linux.intel.com; > zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; Wang, Zhi A > <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; daniel@ffwll.ch; > kwankhede@nvidia.com > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org; > intel-gvt-dev@lists.freedesktop.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v16 5/6] vfio: ABI for mdev display dma-buf operation > > Hi, > > > > Add a head field here? People asked @ kvm forum about multihead > > > support. > > > Even if the initial driver version doesn't support it we could add a > > > field so it becomes easier to add it at some point in the future. > > > > > > Probing for available heads could be done with the PROBE flag, i.e. > > > flags = PROBE | DMABUF, plane_type = PRIMARY, head = 0, 1, ... > > > > Does the multihead mean multiple display monitor? So each head can > > have its own one primary plane, one cursor plane and maybe some sprite > > planes if supported. > > Yes and yes. > > > And the flow could be like this: > > 1) flags = PROBE | DMABUF, then return the number of heads in > > vfio_device_gfx_plane_info.head. > > I'd suggest to use { .flags = PROBE | DMABUF, .head = 0 } to check whenever > dmabuf is supported for head 0, then { .flags = PROBE | DMABUF, .head = 1 } to > check for head 1 support, ... > > Driver returns success if the head is supported and -EINVAL otherwise, simliar to > the dmabuf vs. region probing. > > It is less confusing because .head is always an input field then. > > It is also a bit more flexible because different heads can support different modes > (I don't expect drivers will actually use that though). > > > 2) flags = DMABUF with plane_type = PRIMARY, head = 0, 1, ..., then > > return the id of the exposed framebuffer of that head. > > Yes. > > > Another question is if the sprite plane is supported, how we're going > > to identify them, as there could be several sprite planes for one > > display monitor. > > Do you mean DRM_PLANE_TYPE_OVERLAY? Yes. > > > Add another field "num_plane" for it? Then, I guess the flow could be > > like this: > > 1) flags = PROBE | DMABUF, then return the number of heads in > > vfio_device_gfx_plane_info.head. > > 2) flags = PROBE | DMABUF and head = 0, 1, ..., and plane_type = > > PRIMARY/CURSOR/SPRITE, then return the num_plane. > > I'd suggest to do it simliar to the head probe outlined above, i.e. > userspace calls { .flags = PROBE | DMABUF, .head = 0, .plane_type = OVERLAY, > plane_num = 0, 1, 2, ... } and the driver returns -EINVAL or 0 depending on > whenever it is supported or not. Agree. Thanks. BR, Tina > > cheers, > Gerd > > _______________________________________________ > intel-gvt-dev mailing list > intel-gvt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
Hi, > Should we then specify the error code for "head doesn't exist" vs > "head > doesn't support dmabuf", with the former taking precedence? Perhaps > -ENODEV vs -EINVAL. NODEV for "head doesn't exist" and INVAL for "head doesn't support dmabuf/region/..." ? > Are the heads guaranteed to be contiguous (the > first -ENODEV is the end of possible heads)? Yes, I think the valid heads should be contignous. The guest might still only use a non-contignous subset of the available heads though. cheers, Gerd
On Wed, 08 Nov 2017 09:25:49 +0100 Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > > Should we then specify the error code for "head doesn't exist" vs > > "head > > doesn't support dmabuf", with the former taking precedence? Perhaps > > -ENODEV vs -EINVAL. > > NODEV for "head doesn't exist" and INVAL for "head doesn't support > dmabuf/region/..." ? Yes, exactly. > > Are the heads guaranteed to be contiguous (the > > first -ENODEV is the end of possible heads)? > > Yes, I think the valid heads should be contignous. The guest might > still only use a non-contignous subset of the available heads though. Yep, I agree. Thanks, Alex
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) + /* -------- API for Type1 VFIO IOMMU -------- */ /**
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@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- include/uapi/linux/vfio.h | 68 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)