2019-05-27 08:50:45

by Zhang, Tina

[permalink] [raw]
Subject: [PATCH 0/2] Deliver vGPU page flip event to userspace

This series introduces VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to
set the eventfd based signaling mechanism in vfio display. vGPU can use
this eventfd to deliver the framebuffer page flip event to userspace.


Tina Zhang (2):
vfio: ABI for setting mdev display flip eventfd
drm/i915/gvt: Support delivering page flip event to userspace

drivers/gpu/drm/i915/gvt/dmabuf.c | 31 +++++++++++++++++++++++++++++
drivers/gpu/drm/i915/gvt/dmabuf.h | 1 +
drivers/gpu/drm/i915/gvt/gvt.c | 1 +
drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
drivers/gpu/drm/i915/gvt/handlers.c | 2 ++
drivers/gpu/drm/i915/gvt/kvmgt.c | 7 +++++++
include/uapi/linux/vfio.h | 12 +++++++++++
7 files changed, 56 insertions(+)

--
2.17.1


2019-05-27 08:51:46

by Zhang, Tina

[permalink] [raw]
Subject: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd

Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
based signaling mechanism to deliver vGPU framebuffer page flip
event to userspace.

Signed-off-by: Tina Zhang <[email protected]>
---
include/uapi/linux/vfio.h | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 02bb7ad6e986..27300597717f 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -696,6 +696,18 @@ struct vfio_device_ioeventfd {

#define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE + 16)

+/**
+ * VFIO_DEVICE_SET_GFX_FLIP_EVENTFD - _IOW(VFIO_TYPE, VFIO_BASE + 17, __s32)
+ *
+ * Set eventfd based signaling mechanism to deliver vGPU framebuffer page
+ * flip event to userspace. A value of -1 is used to stop the page flip
+ * delivering.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+
+#define VFIO_DEVICE_SET_GFX_FLIP_EVENTFD _IO(VFIO_TYPE, VFIO_BASE + 17)
+
/* -------- API for Type1 VFIO IOMMU -------- */

/**
--
2.17.1

2019-05-27 09:11:45

by Zhenyu Wang

[permalink] [raw]
Subject: Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd

On 2019.05.27 16:43:11 +0800, Tina Zhang wrote:
> Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
> based signaling mechanism to deliver vGPU framebuffer page flip
> event to userspace.
>

Should we add probe to see if driver can support gfx flip event?

> Signed-off-by: Tina Zhang <[email protected]>
> ---
> include/uapi/linux/vfio.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 02bb7ad6e986..27300597717f 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -696,6 +696,18 @@ struct vfio_device_ioeventfd {
>
> #define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE + 16)
>
> +/**
> + * VFIO_DEVICE_SET_GFX_FLIP_EVENTFD - _IOW(VFIO_TYPE, VFIO_BASE + 17, __s32)
> + *
> + * Set eventfd based signaling mechanism to deliver vGPU framebuffer page
> + * flip event to userspace. A value of -1 is used to stop the page flip
> + * delivering.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +
> +#define VFIO_DEVICE_SET_GFX_FLIP_EVENTFD _IO(VFIO_TYPE, VFIO_BASE + 17)
> +
> /* -------- API for Type1 VFIO IOMMU -------- */
>
> /**
> --
> 2.17.1
>

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


Attachments:
(No filename) (1.32 kB)
signature.asc (201.00 B)
Download all attachments

2019-05-27 12:25:01

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd

On Mon, May 27, 2019 at 05:07:41PM +0800, Zhenyu Wang wrote:
> On 2019.05.27 16:43:11 +0800, Tina Zhang wrote:
> > Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
> > based signaling mechanism to deliver vGPU framebuffer page flip
> > event to userspace.
>
> Should we add probe to see if driver can support gfx flip event?

Userspace can simply call VFIO_DEVICE_SET_GFX_FLIP_EVENTFD and see if it
worked. If so -> use the eventfd. Otherwise take the fallback path
(timer based polling). I can't see any advantage a separate feature
probe steps adds.

cheers,
Gerd

2019-05-27 14:06:50

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd

On Mon, 27 May 2019 16:43:11 +0800
Tina Zhang <[email protected]> wrote:

> Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
> based signaling mechanism to deliver vGPU framebuffer page flip
> event to userspace.
>
> Signed-off-by: Tina Zhang <[email protected]>
> ---
> include/uapi/linux/vfio.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 02bb7ad6e986..27300597717f 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -696,6 +696,18 @@ struct vfio_device_ioeventfd {
>
> #define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE + 16)
>
> +/**
> + * VFIO_DEVICE_SET_GFX_FLIP_EVENTFD - _IOW(VFIO_TYPE, VFIO_BASE + 17, __s32)
> + *
> + * Set eventfd based signaling mechanism to deliver vGPU framebuffer page
> + * flip event to userspace. A value of -1 is used to stop the page flip
> + * delivering.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +
> +#define VFIO_DEVICE_SET_GFX_FLIP_EVENTFD _IO(VFIO_TYPE, VFIO_BASE + 17)
> +
> /* -------- API for Type1 VFIO IOMMU -------- */
>
> /**

Why can't we use VFIO_DEVICE_SET_IRQS for this? We can add a
capability to vfio_irq_info in the same way that we did for regions to
describe device specific IRQ support. Thanks,

Alex

2019-05-28 01:45:20

by Zhang, Tina

[permalink] [raw]
Subject: RE: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd



> -----Original Message-----
> From: intel-gvt-dev [mailto:[email protected]] On
> Behalf Of Alex Williamson
> Sent: Monday, May 27, 2019 10:05 PM
> To: Zhang, Tina <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; Yuan, Hang <[email protected]>;
> [email protected]; [email protected]; Lv, Zhiyuan
> <[email protected]>
> Subject: Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd
>
> On Mon, 27 May 2019 16:43:11 +0800
> Tina Zhang <[email protected]> wrote:
>
> > Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
> > based signaling mechanism to deliver vGPU framebuffer page flip event
> > to userspace.
> >
> > Signed-off-by: Tina Zhang <[email protected]>
> > ---
> > include/uapi/linux/vfio.h | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 02bb7ad6e986..27300597717f 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -696,6 +696,18 @@ struct vfio_device_ioeventfd {
> >
> > #define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE +
> 16)
> >
> > +/**
> > + * VFIO_DEVICE_SET_GFX_FLIP_EVENTFD - _IOW(VFIO_TYPE, VFIO_BASE
> + 17,
> > +__s32)
> > + *
> > + * Set eventfd based signaling mechanism to deliver vGPU framebuffer
> > +page
> > + * flip event to userspace. A value of -1 is used to stop the page
> > +flip
> > + * delivering.
> > + *
> > + * Return: 0 on success, -errno on failure.
> > + */
> > +
> > +#define VFIO_DEVICE_SET_GFX_FLIP_EVENTFD _IO(VFIO_TYPE,
> VFIO_BASE +
> > +17)
> > +
> > /* -------- API for Type1 VFIO IOMMU -------- */
> >
> > /**
>
> Why can't we use VFIO_DEVICE_SET_IRQS for this? We can add a capability
> to vfio_irq_info in the same way that we did for regions to describe device
> specific IRQ support. Thanks,
Add a new kind of index, like this?
enum {
VFIO_PCI_INTX_IRQ_INDEX,
VFIO_PCI_MSI_IRQ_INDEX,
VFIO_PCI_MSIX_IRQ_INDEX,
VFIO_PCI_ERR_IRQ_INDEX,
VFIO_PCI_REQ_IRQ_INDEX,
+ VFIO_PCI_GFX_FLIP_EVENT_INDEX,
VFIO_PCI_NUM_IRQS
};
Perhaps this is what we don't want. This VFIO_PCI_GFX_FLIP_EVENT_INDEX is specific to graphics card and it's actually an event which is reported by INTX/MSI/ MSIX IRQ.
Thanks.

BR,
Tina
>
> Alex
> _______________________________________________
> intel-gvt-dev mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

2019-05-28 03:10:50

by Zhenyu Wang

[permalink] [raw]
Subject: Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd

On 2019.05.27 14:22:37 +0200, Gerd Hoffmann wrote:
> On Mon, May 27, 2019 at 05:07:41PM +0800, Zhenyu Wang wrote:
> > On 2019.05.27 16:43:11 +0800, Tina Zhang wrote:
> > > Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
> > > based signaling mechanism to deliver vGPU framebuffer page flip
> > > event to userspace.
> >
> > Should we add probe to see if driver can support gfx flip event?
>
> Userspace can simply call VFIO_DEVICE_SET_GFX_FLIP_EVENTFD and see if it
> worked. If so -> use the eventfd. Otherwise take the fallback path
> (timer based polling). I can't see any advantage a separate feature
> probe steps adds.
>

Then we need to define error return which means driver doesn't support
e.g -ENOTTY, and driver shouldn't return that for other possible
failure, so user space won't get confused.

I think if we can define this as generic display event notification?
Not necessarily just for flip, just a display change notification to
let user space query current state.

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


Attachments:
(No filename) (1.11 kB)
signature.asc (201.00 B)
Download all attachments

2019-05-28 04:19:54

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd

On Tue, 28 May 2019 01:42:57 +0000
"Zhang, Tina" <[email protected]> wrote:

> > -----Original Message-----
> > From: intel-gvt-dev [mailto:[email protected]] On
> > Behalf Of Alex Williamson
> > Sent: Monday, May 27, 2019 10:05 PM
> > To: Zhang, Tina <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; Yuan, Hang <[email protected]>;
> > [email protected]; [email protected]; Lv, Zhiyuan
> > <[email protected]>
> > Subject: Re: [PATCH 1/2] vfio: ABI for setting mdev display flip eventfd
> >
> > On Mon, 27 May 2019 16:43:11 +0800
> > Tina Zhang <[email protected]> wrote:
> >
> > > Add VFIO_DEVICE_SET_GFX_FLIP_EVENTFD ioctl command to set eventfd
> > > based signaling mechanism to deliver vGPU framebuffer page flip event
> > > to userspace.
> > >
> > > Signed-off-by: Tina Zhang <[email protected]>
> > > ---
> > > include/uapi/linux/vfio.h | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > > index 02bb7ad6e986..27300597717f 100644
> > > --- a/include/uapi/linux/vfio.h
> > > +++ b/include/uapi/linux/vfio.h
> > > @@ -696,6 +696,18 @@ struct vfio_device_ioeventfd {
> > >
> > > #define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE +
> > 16)
> > >
> > > +/**
> > > + * VFIO_DEVICE_SET_GFX_FLIP_EVENTFD - _IOW(VFIO_TYPE, VFIO_BASE
> > + 17,
> > > +__s32)
> > > + *
> > > + * Set eventfd based signaling mechanism to deliver vGPU framebuffer
> > > +page
> > > + * flip event to userspace. A value of -1 is used to stop the page
> > > +flip
> > > + * delivering.
> > > + *
> > > + * Return: 0 on success, -errno on failure.
> > > + */
> > > +
> > > +#define VFIO_DEVICE_SET_GFX_FLIP_EVENTFD _IO(VFIO_TYPE,
> > VFIO_BASE +
> > > +17)
> > > +
> > > /* -------- API for Type1 VFIO IOMMU -------- */
> > >
> > > /**
> >
> > Why can't we use VFIO_DEVICE_SET_IRQS for this? We can add a capability
> > to vfio_irq_info in the same way that we did for regions to describe device
> > specific IRQ support. Thanks,
> Add a new kind of index, like this?
> enum {
> VFIO_PCI_INTX_IRQ_INDEX,
> VFIO_PCI_MSI_IRQ_INDEX,
> VFIO_PCI_MSIX_IRQ_INDEX,
> VFIO_PCI_ERR_IRQ_INDEX,
> VFIO_PCI_REQ_IRQ_INDEX,
> + VFIO_PCI_GFX_FLIP_EVENT_INDEX,
> VFIO_PCI_NUM_IRQS
> };
> Perhaps this is what we don't want. This
> VFIO_PCI_GFX_FLIP_EVENT_INDEX is specific to graphics card and it's
> actually an event which is reported by INTX/MSI/ MSIX IRQ. Thanks.

Right, that is not what I'm suggesting. What I'm looking for is a
similar conversion to what we did for regions, where we extended the
data returned in GET_REGION_INFO to include capabilities
(c84982adb23b), capped the number of regions we define with fixed
indexes (c7bb4cb40f89), and added device specific regions, such as IGD
OpRegion (5846ff54e87d) and IGD host and LPC bridges (f572a960a15e).
The same thing should happen here, the current value of
VFIO_PCI_NUM_IRQS becomes fixed and part of the vfio-pci ABI,
vfio_irq_info is extended with capability support following the same
mechanism, headers, and helpers we use for regions, then the mdev device
simply exposes the extended (and backwards compatible) API without
requiring a device specific ioctl. Thanks,

Alex