linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/6] Deliver vGPU display refresh event to userspace
@ 2019-09-24  6:41 Tina Zhang
  2019-09-24  6:41 ` [PATCH v6 1/6] vfio: Define device specific irq type capability Tina Zhang
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Tina Zhang @ 2019-09-24  6:41 UTC (permalink / raw)
  To: intel-gvt-dev, kvm, linux-kernel
  Cc: Tina Zhang, kraxel, zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian,
	hang.yuan, alex.williamson, yi.l.liu

This series sends the vGPU display refresh events to user land. A vGPU
display IRQ is proposed to notify user space with all the display
updates events.  With this IRQ supported by vendor driver, user space
can stop the display update timer and fully depend on getting the
notification if an update is needed.

How does gvt-g provide the vGPU display refresh IRQ covering all the
display update events?

Instead of delivering page flip events only or vblank events only, we
choose to combine two of them, i.e. post display refresh event at
vblanks and skip some of them when no page flip happens. Vblanks as
upper bound are safe and skipping no-page-flip vblanks guarantees both
trivial performance impacts and good user experience without screen
tearing. Plus, we have the mask/unmask mechansim providing user space
flexibility to switch between event-notified refresh and classic
timer-based refresh.

In addition, there are some cases that guest app only uses one
framebuffer for both drawing and display. In such case, guest OS won't
do the plane page flip when the framebuffer is updated, thus the user
land won't be notified about the updated framebuffer. Hence, in single
framebuffer case, we apply a heuristic to determine whether it is the
case or not. If it is, notify user land when each vblank event
triggers.

v6:
- Add more description to VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ and
  VFIO_IRQ_INFO_CAP_DISPLAY. (Alex & Gerd)
- Irq capability index starts from 1
v5:
- Introduce a vGPU display irq cap which can notify user space with
  both primary and cursor plane update events through one eventfd. (Gerd & Alex)
v4:
- Deliver page flip event and single framebuffer refresh event bounded 
by display vblanks. (Kechen)
v3:
- Deliver display vblank event instead of page flip event. (Zhenyu)
v2:
- Use VFIO irq chain to get eventfds from userspace instead of adding
a new ABI. (Alex)
v1:
- https://patchwork.kernel.org/cover/10962341/

Kechen Lu (2):
  drm/i915/gvt: Deliver async primary plane page flip events at vblank
  drm/i915/gvt: Add cursor plane reg update trap emulation handler

Tina Zhang (4):
  vfio: Define device specific irq type capability
  vfio: Introduce vGPU display irq type
  drm/i915/gvt: Register vGPU display event irq
  drm/i915/gvt: Deliver vGPU refresh event to userspace

 drivers/gpu/drm/i915/gvt/cmd_parser.c |   6 +-
 drivers/gpu/drm/i915/gvt/display.c    |  47 +++++-
 drivers/gpu/drm/i915/gvt/display.h    |   3 +
 drivers/gpu/drm/i915/gvt/gvt.h        |   7 +
 drivers/gpu/drm/i915/gvt/handlers.c   |  32 +++-
 drivers/gpu/drm/i915/gvt/hypercall.h  |   1 +
 drivers/gpu/drm/i915/gvt/interrupt.c  |   7 +
 drivers/gpu/drm/i915/gvt/interrupt.h  |   3 +
 drivers/gpu/drm/i915/gvt/kvmgt.c      | 230 +++++++++++++++++++++++++-
 drivers/gpu/drm/i915/gvt/mpt.h        |  17 ++
 include/uapi/linux/vfio.h             |  57 ++++++-
 11 files changed, 391 insertions(+), 19 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 1/6] vfio: Define device specific irq type capability
  2019-09-24  6:41 [PATCH v6 0/6] Deliver vGPU display refresh event to userspace Tina Zhang
@ 2019-09-24  6:41 ` Tina Zhang
  2019-09-24  6:41 ` [PATCH v6 2/6] vfio: Introduce vGPU display irq type Tina Zhang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Tina Zhang @ 2019-09-24  6:41 UTC (permalink / raw)
  To: intel-gvt-dev, kvm, linux-kernel
  Cc: Tina Zhang, kraxel, zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian,
	hang.yuan, alex.williamson, yi.l.liu, Eric Auger

Cap the number of irqs with fixed indexes and use capability chains
to chain device specific irqs.

v2:
- Irq capability index starts from 1.

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/uapi/linux/vfio.h | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 9e843a147ead..aa6850f1daef 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -455,11 +455,27 @@ struct vfio_irq_info {
 #define VFIO_IRQ_INFO_MASKABLE		(1 << 1)
 #define VFIO_IRQ_INFO_AUTOMASKED	(1 << 2)
 #define VFIO_IRQ_INFO_NORESIZE		(1 << 3)
+#define VFIO_IRQ_INFO_FLAG_CAPS		(1 << 4) /* Info supports caps */
 	__u32	index;		/* IRQ index */
 	__u32	count;		/* Number of IRQs within this index */
+	__u32	cap_offset;	/* Offset within info struct of first cap */
 };
 #define VFIO_DEVICE_GET_IRQ_INFO	_IO(VFIO_TYPE, VFIO_BASE + 9)
 
+/*
+ * The irq type capability allows irqs unique to a specific device or
+ * class of devices to be exposed.
+ *
+ * The structures below define version 1 of this capability.
+ */
+#define VFIO_IRQ_INFO_CAP_TYPE      1
+
+struct vfio_irq_info_cap_type {
+	struct vfio_info_cap_header header;
+	__u32 type;     /* global per bus driver */
+	__u32 subtype;  /* type specific */
+};
+
 /**
  * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct vfio_irq_set)
  *
@@ -561,7 +577,8 @@ enum {
 	VFIO_PCI_MSIX_IRQ_INDEX,
 	VFIO_PCI_ERR_IRQ_INDEX,
 	VFIO_PCI_REQ_IRQ_INDEX,
-	VFIO_PCI_NUM_IRQS
+	VFIO_PCI_NUM_IRQS = 5	/* Fixed user ABI, IRQ indexes >=5 use   */
+				/* device specific cap to define content */
 };
 
 /*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 2/6] vfio: Introduce vGPU display irq type
  2019-09-24  6:41 [PATCH v6 0/6] Deliver vGPU display refresh event to userspace Tina Zhang
  2019-09-24  6:41 ` [PATCH v6 1/6] vfio: Define device specific irq type capability Tina Zhang
@ 2019-09-24  6:41 ` Tina Zhang
  2019-09-24 20:35   ` Alex Williamson
  2019-09-24  6:41 ` [PATCH v6 3/6] drm/i915/gvt: Register vGPU display event irq Tina Zhang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Tina Zhang @ 2019-09-24  6:41 UTC (permalink / raw)
  To: intel-gvt-dev, kvm, linux-kernel
  Cc: Tina Zhang, kraxel, zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian,
	hang.yuan, alex.williamson, yi.l.liu

Introduce vGPU specific irq type VFIO_IRQ_TYPE_GFX, and
VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ as the subtype for vGPU display.

Introduce vfio_irq_info_cap_display_plane_events capability to notify
user space with the vGPU's plane update events

v3:
- Add more description to VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ and
  VFIO_IRQ_INFO_CAP_DISPLAY. (Alex & Gerd)

v2:
- Add VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ description. (Alex & Kechen)
- Introduce vfio_irq_info_cap_display_plane_events. (Gerd & Alex)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 include/uapi/linux/vfio.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index aa6850f1daef..2946a028b0c3 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -476,6 +476,44 @@ struct vfio_irq_info_cap_type {
 	__u32 subtype;  /* type specific */
 };
 
+/* vGPU IRQ TYPE */
+#define VFIO_IRQ_TYPE_GFX			(1)
+
+/* sub-types for VFIO_IRQ_TYPE_GFX */
+/*
+ * vGPU device display refresh interrupt request. This irq asking for
+ * a user space display refresh, covers all display updates events,
+ * i.e. user space can stop the display update timer and fully depend
+ * on getting the notification if an update is needed.
+ */
+#define VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ	(1)
+
+/*
+ * Display capability of reporting display refresh interrupt events.
+ * This gives user space the capability to identify different display
+ * updates events of the display refresh interrupt request.
+ *
+ * When notified by VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ, user space can
+ * use the eventfd counter value to identify which plane has been
+ * updated.
+ *
+ * Note that there might be some cases like counter_value >
+ * (cur_event_val + pri_event_val), if notifications haven't been
+ * handled on time in user mode. These cases can be handled as all
+ * plane updated case or signle plane updated case depending on the
+ * value.
+ *
+ * cur_event_val: eventfd counter value for cursor plane change event.
+ * pri_event_val: eventfd counter value for primary plane change event.
+ */
+#define VFIO_IRQ_INFO_CAP_DISPLAY	2
+
+struct vfio_irq_info_cap_display_plane_events {
+	struct vfio_info_cap_header header;
+	__u64 cur_event_val;
+	__u64 pri_event_val;
+};
+
 /**
  * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct vfio_irq_set)
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 3/6] drm/i915/gvt: Register vGPU display event irq
  2019-09-24  6:41 [PATCH v6 0/6] Deliver vGPU display refresh event to userspace Tina Zhang
  2019-09-24  6:41 ` [PATCH v6 1/6] vfio: Define device specific irq type capability Tina Zhang
  2019-09-24  6:41 ` [PATCH v6 2/6] vfio: Introduce vGPU display irq type Tina Zhang
@ 2019-09-24  6:41 ` Tina Zhang
  2019-09-24  6:41 ` [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace Tina Zhang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Tina Zhang @ 2019-09-24  6:41 UTC (permalink / raw)
  To: intel-gvt-dev, kvm, linux-kernel
  Cc: Tina Zhang, kraxel, zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian,
	hang.yuan, alex.williamson, yi.l.liu

Gvt-g emulates and injects the vGPU's display interrupts in kernel
space. However the dma-buf based framebuffer consumer in the user
land (e.g. Qemu vfio/display) may also need to be notified by this
event.

Register the display irq as VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ to
each vGPU, so that the display interrupt event can be delivered to
userspace through eventfd.

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/display.c   | 10 +++-
 drivers/gpu/drm/i915/gvt/display.h   |  3 ++
 drivers/gpu/drm/i915/gvt/gvt.h       |  2 +
 drivers/gpu/drm/i915/gvt/hypercall.h |  1 +
 drivers/gpu/drm/i915/gvt/kvmgt.c     | 71 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/mpt.h       | 17 +++++++
 6 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index e1c313da6c00..1a0a4ae4826e 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -506,16 +506,22 @@ void intel_vgpu_clean_display(struct intel_vgpu *vgpu)
 int intel_vgpu_init_display(struct intel_vgpu *vgpu, u64 resolution)
 {
 	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	int ret;
 
 	intel_vgpu_init_i2c_edid(vgpu);
 
 	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ||
 	    IS_COFFEELAKE(dev_priv))
-		return setup_virtual_dp_monitor(vgpu, PORT_D, GVT_DP_D,
+		ret = setup_virtual_dp_monitor(vgpu, PORT_D, GVT_DP_D,
 						resolution);
 	else
-		return setup_virtual_dp_monitor(vgpu, PORT_B, GVT_DP_B,
+		ret = setup_virtual_dp_monitor(vgpu, PORT_B, GVT_DP_B,
 						resolution);
+
+	if (ret == 0)
+		intel_gvt_hypervisor_register_display_irq(vgpu);
+
+	return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gvt/display.h b/drivers/gpu/drm/i915/gvt/display.h
index a87f33e6a23c..ba07fbef9194 100644
--- a/drivers/gpu/drm/i915/gvt/display.h
+++ b/drivers/gpu/drm/i915/gvt/display.h
@@ -112,6 +112,9 @@
 #define SBI_ADDR_OFFSET_SHIFT           16
 #define SBI_ADDR_OFFSET_MASK            (0xffff << SBI_ADDR_OFFSET_SHIFT)
 
+#define DISPLAY_PRI_REFRESH_EVENT_VAL		(1UL << 56)
+#define DISPLAY_CUR_REFRESH_EVENT_VAL		(1UL << 48)
+
 struct intel_vgpu_sbi_register {
 	unsigned int offset;
 	u32 value;
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index b47c6acaf9c0..8008047d026c 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -201,6 +201,8 @@ struct intel_vgpu {
 		struct mdev_device *mdev;
 		struct vfio_region *region;
 		int num_regions;
+		struct vfio_irq *irq;
+		int num_irqs;
 		struct eventfd_ctx *intx_trigger;
 		struct eventfd_ctx *msi_trigger;
 
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index 4862fb12778e..be33f20f3bc1 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -68,6 +68,7 @@ struct intel_gvt_mpt {
 			     bool map);
 	int (*set_opregion)(void *vgpu);
 	int (*set_edid)(void *vgpu, int port_num);
+	int (*register_display_irq)(void *vgpu);
 	int (*get_vfio_device)(void *vgpu);
 	void (*put_vfio_device)(void *vgpu);
 	bool (*is_valid_gfn)(unsigned long handle, unsigned long gfn);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 343d79c1cb7e..269506300310 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -78,6 +78,19 @@ struct vfio_region {
 	void				*data;
 };
 
+struct intel_vgpu_irqops {
+	int (*add_capability)(struct intel_vgpu *vgpu,
+			   struct vfio_info_cap *caps);
+};
+
+struct vfio_irq {
+	u32	type;
+	u32	subtype;
+	u32	flags;
+	u32	count;
+	const struct intel_vgpu_irqops *ops;
+};
+
 struct vfio_edid_region {
 	struct vfio_region_gfx_edid vfio_edid_regs;
 	void *edid_blob;
@@ -635,6 +648,59 @@ static int kvmgt_set_edid(void *p_vgpu, int port_num)
 	return ret;
 }
 
+static int add_display_irq_capability(struct intel_vgpu *vgpu,
+			   struct vfio_info_cap *caps)
+{
+	struct vfio_irq_info_cap_display_plane_events cap = {
+		.header.id = VFIO_IRQ_INFO_CAP_DISPLAY,
+		.header.version = 1,
+		.cur_event_val = DISPLAY_CUR_REFRESH_EVENT_VAL,
+		.pri_event_val = DISPLAY_PRI_REFRESH_EVENT_VAL,
+	};
+
+	return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
+}
+
+
+static const struct intel_vgpu_irqops intel_vgpu_irqops_display = {
+	.add_capability = add_display_irq_capability,
+};
+
+static int intel_vgpu_register_irq(struct intel_vgpu *vgpu,
+				   unsigned int type, unsigned int subtype,
+				   u32 count, u32 flags,
+				   const struct intel_vgpu_irqops *ops)
+{
+	struct vfio_irq *irq;
+
+	irq = krealloc(vgpu->vdev.irq,
+			(vgpu->vdev.num_irqs + 1) * sizeof(*irq),
+			GFP_KERNEL);
+	if (!irq)
+		return -ENOMEM;
+
+	vgpu->vdev.irq = irq;
+	vgpu->vdev.irq[vgpu->vdev.num_irqs].type = type;
+	vgpu->vdev.irq[vgpu->vdev.num_irqs].subtype = subtype;
+	vgpu->vdev.irq[vgpu->vdev.num_irqs].count = count;
+	vgpu->vdev.irq[vgpu->vdev.num_irqs].flags = flags;
+	vgpu->vdev.irq[vgpu->vdev.num_irqs].ops = ops;
+	vgpu->vdev.num_irqs++;
+	return 0;
+}
+
+static int kvmgt_register_display_irq(void *p_vgpu)
+{
+	struct intel_vgpu *vgpu = (struct intel_vgpu *)p_vgpu;
+
+	intel_vgpu_register_irq(vgpu, VFIO_IRQ_TYPE_GFX,
+				VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ,
+				1,
+				VFIO_IRQ_INFO_MASKABLE | VFIO_IRQ_INFO_EVENTFD,
+				&intel_vgpu_irqops_display);
+	return 0;
+}
+
 static void kvmgt_put_vfio_device(void *vgpu)
 {
 	if (WARN_ON(!((struct intel_vgpu *)vgpu)->vdev.vfio_device))
@@ -1833,6 +1899,10 @@ static void kvmgt_detach_vgpu(void *p_vgpu)
 	vgpu->vdev.num_regions = 0;
 	kfree(vgpu->vdev.region);
 	vgpu->vdev.region = NULL;
+
+	vgpu->vdev.num_irqs = 0;
+	kfree(vgpu->vdev.irq);
+	vgpu->vdev.irq = NULL;
 }
 
 static int kvmgt_inject_msi(unsigned long handle, u32 addr, u16 data)
@@ -2046,6 +2116,7 @@ static struct intel_gvt_mpt kvmgt_mpt = {
 	.dma_unmap_guest_page = kvmgt_dma_unmap_guest_page,
 	.set_opregion = kvmgt_set_opregion,
 	.set_edid = kvmgt_set_edid,
+	.register_display_irq = kvmgt_register_display_irq,
 	.get_vfio_device = kvmgt_get_vfio_device,
 	.put_vfio_device = kvmgt_put_vfio_device,
 	.is_valid_gfn = kvmgt_is_valid_gfn,
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index 0f9440128123..abf4a69920d3 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -330,6 +330,23 @@ static inline int intel_gvt_hypervisor_set_edid(struct intel_vgpu *vgpu,
 	return intel_gvt_host.mpt->set_edid(vgpu, port_num);
 }
 
+/**
+ * intel_gvt_hypervisor_set_irq - register vgpu specific irq
+ * @vgpu: a vGPU
+ * @port_num: display port number
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ */
+static inline int intel_gvt_hypervisor_register_display_irq(
+						struct intel_vgpu *vgpu)
+{
+	if (!intel_gvt_host.mpt->register_display_irq)
+		return 0;
+
+	return intel_gvt_host.mpt->register_display_irq(vgpu);
+}
+
 /**
  * intel_gvt_hypervisor_get_vfio_device - increase vfio device ref count
  * @vgpu: a vGPU
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace
  2019-09-24  6:41 [PATCH v6 0/6] Deliver vGPU display refresh event to userspace Tina Zhang
                   ` (2 preceding siblings ...)
  2019-09-24  6:41 ` [PATCH v6 3/6] drm/i915/gvt: Register vGPU display event irq Tina Zhang
@ 2019-09-24  6:41 ` Tina Zhang
  2019-09-24  9:02   ` kbuild test robot
  2019-09-24 20:10   ` Alex Williamson
  2019-09-24  6:41 ` [PATCH v6 5/6] drm/i915/gvt: Deliver async primary plane page flip events at vblank Tina Zhang
  2019-09-24  6:41 ` [PATCH v6 6/6] drm/i915/gvt: Add cursor plane reg update trap emulation handler Tina Zhang
  5 siblings, 2 replies; 11+ messages in thread
From: Tina Zhang @ 2019-09-24  6:41 UTC (permalink / raw)
  To: intel-gvt-dev, kvm, linux-kernel
  Cc: Tina Zhang, kraxel, zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian,
	hang.yuan, alex.williamson, yi.l.liu, Kechen Lu

Deliver the display refresh events to the user land. Userspace can use
the irq mask/unmask mechanism to disable or enable the event delivery.

As we know, delivering refresh event at each vblank safely avoids
tearing and unexpected event overwhelming, but there are still spaces
to optimize.

For handling the normal case, deliver the page flip refresh
event at each vblank, in other words, bounded by vblanks. Skipping some
events bring performance enhancement while not hurting user experience.

For single framebuffer case, deliver the refresh events to userspace at
all vblanks. This heuristic at each vblank leverages pageflip_count
incresements to determine if there is no page flip happens after a certain
period and so that the case is regarded as single framebuffer one.
Although this heuristic makes incorrect decision sometimes and it depends
on guest behavior, for example, when no cursor movements happen, the
user experience does not harm and front buffer is still correctly acquired.
Meanwhile, in actual single framebuffer case, the user experience is
enhanced compared with page flip events only.

Addtionally, to mitigate the events delivering footprints, one eventfd and
8 byte eventfd counter partition are leveraged.

v3:
- make no_pageflip_count be per-vgpu instead of static. (Zhenyu)

v2:
- Support vfio_irq_info_cap_display_plane_events. (Tina)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Signed-off-by: Kechen Lu <kechen.lu@intel.com>
---
 drivers/gpu/drm/i915/gvt/display.c |  20 ++++
 drivers/gpu/drm/i915/gvt/gvt.h     |   3 +
 drivers/gpu/drm/i915/gvt/kvmgt.c   | 159 +++++++++++++++++++++++++++--
 3 files changed, 173 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index 1a0a4ae4826e..9f2c2cd10369 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -34,6 +34,8 @@
 
 #include "i915_drv.h"
 #include "gvt.h"
+#include <uapi/linux/vfio.h>
+#include <drm/drm_plane.h>
 
 static int get_edp_pipe(struct intel_vgpu *vgpu)
 {
@@ -387,6 +389,8 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt)
 	mutex_unlock(&gvt->lock);
 }
 
+#define PAGEFLIP_DELAY_THR 10
+
 static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
 {
 	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
@@ -396,7 +400,9 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
 		[PIPE_B] = PIPE_B_VBLANK,
 		[PIPE_C] = PIPE_C_VBLANK,
 	};
+	int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
 	int event;
+	u64 eventfd_signal_val = 0;
 
 	if (pipe < PIPE_A || pipe > PIPE_C)
 		return;
@@ -407,9 +413,23 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
 		if (!pipe_is_enabled(vgpu, pipe))
 			continue;
 
+		if (event == pri_flip_event)
+			eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
+
 		intel_vgpu_trigger_virtual_event(vgpu, event);
 	}
 
+	if (eventfd_signal_val)
+		vgpu->no_pageflip_count = 0;
+	else if (!eventfd_signal_val && vgpu->no_pageflip_count > PAGEFLIP_DELAY_THR)
+		eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
+	else
+		vgpu->no_pageflip_count++;
+
+	if (vgpu->vdev.vblank_trigger && !vgpu->vdev.display_event_mask &&
+		eventfd_signal_val)
+		eventfd_signal(vgpu->vdev.vblank_trigger, eventfd_signal_val);
+
 	if (pipe_is_enabled(vgpu, pipe)) {
 		vgpu_vreg_t(vgpu, PIPE_FRMCOUNT_G4X(pipe))++;
 		intel_vgpu_trigger_virtual_event(vgpu, vblank_event[pipe]);
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 8008047d026c..cc39b449b061 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -205,6 +205,8 @@ struct intel_vgpu {
 		int num_irqs;
 		struct eventfd_ctx *intx_trigger;
 		struct eventfd_ctx *msi_trigger;
+		struct eventfd_ctx *vblank_trigger;
+		bool display_event_mask;
 
 		/*
 		 * Two caches are used to avoid mapping duplicated pages (eg.
@@ -229,6 +231,7 @@ struct intel_vgpu {
 	struct idr object_idr;
 
 	struct completion vblank_done;
+	int no_pageflip_count;
 
 	u32 scan_nonprivbb;
 };
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 269506300310..f30b7a5272e8 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1250,6 +1250,8 @@ static int intel_vgpu_get_irq_count(struct intel_vgpu *vgpu, int type)
 {
 	if (type == VFIO_PCI_INTX_IRQ_INDEX || type == VFIO_PCI_MSI_IRQ_INDEX)
 		return 1;
+	else if (type < VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs)
+		return vgpu->vdev.irq[type - VFIO_PCI_NUM_IRQS].count;
 
 	return 0;
 }
@@ -1297,7 +1299,60 @@ static int intel_vgpu_set_msi_trigger(struct intel_vgpu *vgpu,
 	return 0;
 }
 
-static int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
+static int intel_vgu_set_display_irq_mask(struct intel_vgpu *vgpu,
+		unsigned int index, unsigned int start, unsigned int count,
+		u32 flags, void *data)
+{
+	if (start != 0 || count > 2)
+		return -EINVAL;
+
+	if (flags & VFIO_IRQ_SET_DATA_NONE)
+		vgpu->vdev.display_event_mask = true;
+
+	return 0;
+}
+
+static int intel_vgu_set_display_irq_unmask(struct intel_vgpu *vgpu,
+		unsigned int index, unsigned int start, unsigned int count,
+		u32 flags, void *data)
+{
+	if (start != 0 || count > 2)
+		return -EINVAL;
+
+	if (flags & VFIO_IRQ_SET_DATA_NONE)
+		vgpu->vdev.display_event_mask = false;
+
+	return 0;
+}
+
+static int intel_vgpu_set_display_event_trigger(struct intel_vgpu *vgpu,
+		unsigned int index, unsigned int start, unsigned int count,
+		u32 flags, void *data)
+{
+	struct eventfd_ctx *trigger;
+
+	if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
+		int fd = *(int *)data;
+
+		trigger = eventfd_ctx_fdget(fd);
+		if (IS_ERR(trigger)) {
+			gvt_vgpu_err("eventfd_ctx_fdget failed\n");
+			return PTR_ERR(trigger);
+		}
+		vgpu->vdev.vblank_trigger = trigger;
+		vgpu->vdev.display_event_mask = false;
+	} else if ((flags & VFIO_IRQ_SET_DATA_NONE) && !count) {
+		trigger = vgpu->vdev.vblank_trigger;
+		if (trigger) {
+			eventfd_ctx_put(trigger);
+			vgpu->vdev.vblank_trigger = NULL;
+		}
+	}
+
+	return 0;
+}
+
+int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
 		unsigned int index, unsigned int start, unsigned int count,
 		void *data)
 {
@@ -1330,6 +1385,35 @@ static int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
 			break;
 		}
 		break;
+	default:
+	{
+		int i;
+
+		if (index >= VFIO_PCI_NUM_IRQS +
+					vgpu->vdev.num_irqs)
+			return -EINVAL;
+		index =
+			array_index_nospec(index,
+						VFIO_PCI_NUM_IRQS +
+						vgpu->vdev.num_irqs);
+
+		i = index - VFIO_PCI_NUM_IRQS;
+		if (vgpu->vdev.irq[i].type == VFIO_IRQ_TYPE_GFX &&
+		    vgpu->vdev.irq[i].subtype ==
+		    VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ) {
+			switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
+			case VFIO_IRQ_SET_ACTION_MASK:
+				func = intel_vgu_set_display_irq_mask;
+				break;
+			case VFIO_IRQ_SET_ACTION_UNMASK:
+				func = intel_vgu_set_display_irq_unmask;
+				break;
+			case VFIO_IRQ_SET_ACTION_TRIGGER:
+				func = intel_vgpu_set_display_event_trigger;
+				break;
+			}
+		}
+	}
 	}
 
 	if (!func)
@@ -1361,7 +1445,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 		info.flags |= VFIO_DEVICE_FLAGS_RESET;
 		info.num_regions = VFIO_PCI_NUM_REGIONS +
 				vgpu->vdev.num_regions;
-		info.num_irqs = VFIO_PCI_NUM_IRQS;
+		info.num_irqs = VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs;
 
 		return copy_to_user((void __user *)arg, &info, minsz) ?
 			-EFAULT : 0;
@@ -1519,32 +1603,88 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 			-EFAULT : 0;
 	} else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
 		struct vfio_irq_info info;
+		struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
+		unsigned int i;
+		int ret;
 
 		minsz = offsetofend(struct vfio_irq_info, count);
 
 		if (copy_from_user(&info, (void __user *)arg, minsz))
 			return -EFAULT;
 
-		if (info.argsz < minsz || info.index >= VFIO_PCI_NUM_IRQS)
+		if (info.argsz < minsz)
 			return -EINVAL;
 
 		switch (info.index) {
 		case VFIO_PCI_INTX_IRQ_INDEX:
 		case VFIO_PCI_MSI_IRQ_INDEX:
+			info.flags = VFIO_IRQ_INFO_EVENTFD;
 			break;
-		default:
+		case VFIO_PCI_MSIX_IRQ_INDEX:
+		case VFIO_PCI_ERR_IRQ_INDEX:
+		case VFIO_PCI_REQ_IRQ_INDEX:
 			return -EINVAL;
-		}
+		default:
+		{
+			struct vfio_irq_info_cap_type cap_type = {
+				.header.id = VFIO_IRQ_INFO_CAP_TYPE,
+				.header.version = 1 };
 
-		info.flags = VFIO_IRQ_INFO_EVENTFD;
+			if (info.index >= VFIO_PCI_NUM_IRQS +
+					vgpu->vdev.num_irqs)
+				return -EINVAL;
+			info.index =
+				array_index_nospec(info.index,
+						VFIO_PCI_NUM_IRQS +
+						vgpu->vdev.num_irqs);
+
+			i = info.index - VFIO_PCI_NUM_IRQS;
+
+			info.flags = vgpu->vdev.irq[i].flags;
+			cap_type.type = vgpu->vdev.irq[i].type;
+			cap_type.subtype = vgpu->vdev.irq[i].subtype;
+
+			ret = vfio_info_add_capability(&caps,
+						&cap_type.header,
+						sizeof(cap_type));
+			if (ret)
+				return ret;
+
+			if (vgpu->vdev.irq[i].ops->add_capability) {
+				ret = vgpu->vdev.irq[i].ops->add_capability(vgpu,
+									    &caps);
+				if (ret)
+					return ret;
+			}
+		}
+		}
 
 		info.count = intel_vgpu_get_irq_count(vgpu, info.index);
 
 		if (info.index == VFIO_PCI_INTX_IRQ_INDEX)
 			info.flags |= (VFIO_IRQ_INFO_MASKABLE |
 				       VFIO_IRQ_INFO_AUTOMASKED);
-		else
-			info.flags |= VFIO_IRQ_INFO_NORESIZE;
+
+		if (caps.size) {
+			info.flags |= VFIO_IRQ_INFO_FLAG_CAPS;
+			if (info.argsz < sizeof(info) + caps.size) {
+				info.argsz = sizeof(info) + caps.size;
+				info.cap_offset = 0;
+			} else {
+				vfio_info_cap_shift(&caps, sizeof(info));
+				if (copy_to_user((void __user *)arg +
+						  sizeof(info), caps.buf,
+						  caps.size)) {
+					kfree(caps.buf);
+					return -EFAULT;
+				}
+				info.cap_offset = sizeof(info);
+				if (offsetofend(struct vfio_irq_info, cap_offset) > minsz)
+					minsz = offsetofend(struct vfio_irq_info, cap_offset);
+			}
+
+			kfree(caps.buf);
+		}
 
 		return copy_to_user((void __user *)arg, &info, minsz) ?
 			-EFAULT : 0;
@@ -1563,7 +1703,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 			int max = intel_vgpu_get_irq_count(vgpu, hdr.index);
 
 			ret = vfio_set_irqs_validate_and_prepare(&hdr, max,
-						VFIO_PCI_NUM_IRQS, &data_size);
+					VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs,
+								 &data_size);
 			if (ret) {
 				gvt_vgpu_err("intel:vfio_set_irqs_validate_and_prepare failed\n");
 				return -EINVAL;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 5/6] drm/i915/gvt: Deliver async primary plane page flip events at vblank
  2019-09-24  6:41 [PATCH v6 0/6] Deliver vGPU display refresh event to userspace Tina Zhang
                   ` (3 preceding siblings ...)
  2019-09-24  6:41 ` [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace Tina Zhang
@ 2019-09-24  6:41 ` Tina Zhang
  2019-09-24 20:14   ` Alex Williamson
  2019-09-24  6:41 ` [PATCH v6 6/6] drm/i915/gvt: Add cursor plane reg update trap emulation handler Tina Zhang
  5 siblings, 1 reply; 11+ messages in thread
From: Tina Zhang @ 2019-09-24  6:41 UTC (permalink / raw)
  To: intel-gvt-dev, kvm, linux-kernel
  Cc: Kechen Lu, kraxel, zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian,
	hang.yuan, alex.williamson, yi.l.liu, Tina Zhang

From: Kechen Lu <kechen.lu@intel.com>

Only sync primary plane page flip events are checked and delivered
as the display refresh events before, this patch tries to deliver async
primary page flip events bounded by vblanks.

To deliver correct async page flip, the new async flip bitmap is
introduced and in vblank emulation handler to check bitset.

Signed-off-by: Kechen Lu <kechen.lu@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/cmd_parser.c |  6 ++++--
 drivers/gpu/drm/i915/gvt/display.c    | 10 ++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h        |  2 ++
 drivers/gpu/drm/i915/gvt/handlers.c   |  5 +++--
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index e753b1e706e2..1abb05431177 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -1365,9 +1365,11 @@ static int gen8_update_plane_mmio_from_mi_display_flip(
 	if (info->plane == PLANE_PRIMARY)
 		vgpu_vreg_t(vgpu, PIPE_FLIPCOUNT_G4X(info->pipe))++;
 
-	if (info->async_flip)
+	if (info->async_flip) {
 		intel_vgpu_trigger_virtual_event(vgpu, info->event);
-	else
+		set_bit(info->plane,
+			vgpu->display.async_flip_event[info->pipe]);
+	} else
 		set_bit(info->event, vgpu->irq.flip_done_event[info->pipe]);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index 9f2c2cd10369..9acde0bdd5f4 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -419,6 +419,16 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
 		intel_vgpu_trigger_virtual_event(vgpu, event);
 	}
 
+	for_each_set_bit(event, vgpu->display.async_flip_event[pipe],
+			I915_MAX_PLANES) {
+		clear_bit(event, vgpu->display.async_flip_event[pipe]);
+		if (!pipe_is_enabled(vgpu, pipe))
+			continue;
+
+		if (event == PLANE_PRIMARY)
+			eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
+	}
+
 	if (eventfd_signal_val)
 		vgpu->no_pageflip_count = 0;
 	else if (!eventfd_signal_val && vgpu->no_pageflip_count > PAGEFLIP_DELAY_THR)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index cc39b449b061..73769a87b407 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -128,6 +128,8 @@ struct intel_vgpu_display {
 	struct intel_vgpu_i2c_edid i2c_edid;
 	struct intel_vgpu_port ports[I915_MAX_PORTS];
 	struct intel_vgpu_sbi sbi;
+	DECLARE_BITMAP(async_flip_event[I915_MAX_PIPES],
+		       I915_MAX_PLANES);
 };
 
 struct vgpu_sched_ctl {
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index 45a9124e53b6..e5a022c2e7bb 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -760,9 +760,10 @@ static int pri_surf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
 
 	vgpu_vreg_t(vgpu, PIPE_FLIPCOUNT_G4X(pipe))++;
 
-	if (vgpu_vreg_t(vgpu, DSPCNTR(pipe)) & PLANE_CTL_ASYNC_FLIP)
+	if (vgpu_vreg_t(vgpu, DSPCNTR(pipe)) & PLANE_CTL_ASYNC_FLIP) {
 		intel_vgpu_trigger_virtual_event(vgpu, event);
-	else
+		set_bit(PLANE_PRIMARY, vgpu->display.async_flip_event[pipe]);
+	} else
 		set_bit(event, vgpu->irq.flip_done_event[pipe]);
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 6/6] drm/i915/gvt: Add cursor plane reg update trap emulation handler
  2019-09-24  6:41 [PATCH v6 0/6] Deliver vGPU display refresh event to userspace Tina Zhang
                   ` (4 preceding siblings ...)
  2019-09-24  6:41 ` [PATCH v6 5/6] drm/i915/gvt: Deliver async primary plane page flip events at vblank Tina Zhang
@ 2019-09-24  6:41 ` Tina Zhang
  5 siblings, 0 replies; 11+ messages in thread
From: Tina Zhang @ 2019-09-24  6:41 UTC (permalink / raw)
  To: intel-gvt-dev, kvm, linux-kernel
  Cc: Kechen Lu, kraxel, zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian,
	hang.yuan, alex.williamson, yi.l.liu, Tina Zhang

From: Kechen Lu <kechen.lu@intel.com>

This patch adds the cursor plane CURBASE reg update trap handler
in order to:

- Deliver the cursor refresh event at each vblank emulation,
the flip_done_event bit check is supposed to do here. If cursor
plane updates happen, deliver the cursor refresh events.

- Support the sync and async cursor plane updates and
corresponding cursor plane flip interrupts reporting.

v2:
- As the suggestion from Zhenyu, the experiments show that
Windows driver programs the CURBASE and CURPOS at one time as
well as the Linux i915 driver. So only track the CURBASE is
enough.

Signed-off-by: Kechen Lu <kechen.lu@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/display.c   |  7 +++++++
 drivers/gpu/drm/i915/gvt/handlers.c  | 27 ++++++++++++++++++++++++---
 drivers/gpu/drm/i915/gvt/interrupt.c |  7 +++++++
 drivers/gpu/drm/i915/gvt/interrupt.h |  3 +++
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index 9acde0bdd5f4..12a03c3b9051 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -401,6 +401,7 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
 		[PIPE_C] = PIPE_C_VBLANK,
 	};
 	int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
+	int cur_flip_event = CURSOR_A_FLIP_DONE + pipe;
 	int event;
 	u64 eventfd_signal_val = 0;
 
@@ -416,6 +417,9 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
 		if (event == pri_flip_event)
 			eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
 
+		if (event == cur_flip_event)
+			eventfd_signal_val |= DISPLAY_CUR_REFRESH_EVENT_VAL;
+
 		intel_vgpu_trigger_virtual_event(vgpu, event);
 	}
 
@@ -427,6 +431,9 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
 
 		if (event == PLANE_PRIMARY)
 			eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
+
+		if (event == PLANE_CURSOR)
+			eventfd_signal_val |= DISPLAY_CUR_REFRESH_EVENT_VAL;
 	}
 
 	if (eventfd_signal_val)
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index e5a022c2e7bb..deb1bf5ba028 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -769,6 +769,27 @@ static int pri_surf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
 	return 0;
 }
 
+#define CURBASE_TO_PIPE(offset) \
+	calc_index(offset, _CURABASE, _CURBBASE, 0, CURBASE(PIPE_C))
+
+static int cur_surf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
+		void *p_data, unsigned int bytes)
+{
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	u32 pipe = CURBASE_TO_PIPE(offset);
+	int event = CURSOR_A_FLIP_DONE + pipe;
+
+	write_vreg(vgpu, offset, p_data, bytes);
+
+	if (vgpu_vreg_t(vgpu, CURCNTR(pipe)) & PLANE_CTL_ASYNC_FLIP) {
+		intel_vgpu_trigger_virtual_event(vgpu, event);
+		set_bit(PLANE_CURSOR, vgpu->display.async_flip_event[pipe]);
+	} else
+		set_bit(event, vgpu->irq.flip_done_event[pipe]);
+
+	return 0;
+}
+
 #define SPRSURF_TO_PIPE(offset) \
 	calc_index(offset, _SPRA_SURF, _SPRB_SURF, 0, SPRSURF(PIPE_C))
 
@@ -1990,9 +2011,9 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
 	MMIO_D(CURPOS(PIPE_B), D_ALL);
 	MMIO_D(CURPOS(PIPE_C), D_ALL);
 
-	MMIO_D(CURBASE(PIPE_A), D_ALL);
-	MMIO_D(CURBASE(PIPE_B), D_ALL);
-	MMIO_D(CURBASE(PIPE_C), D_ALL);
+	MMIO_DH(CURBASE(PIPE_A), D_ALL, NULL, cur_surf_mmio_write);
+	MMIO_DH(CURBASE(PIPE_B), D_ALL, NULL, cur_surf_mmio_write);
+	MMIO_DH(CURBASE(PIPE_C), D_ALL, NULL, cur_surf_mmio_write);
 
 	MMIO_D(CUR_FBC_CTL(PIPE_A), D_ALL);
 	MMIO_D(CUR_FBC_CTL(PIPE_B), D_ALL);
diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c
index 11accd3e1023..8e43490c88d6 100644
--- a/drivers/gpu/drm/i915/gvt/interrupt.c
+++ b/drivers/gpu/drm/i915/gvt/interrupt.c
@@ -113,6 +113,9 @@ static const char * const irq_name[INTEL_GVT_EVENT_MAX] = {
 	[SPRITE_A_FLIP_DONE] = "Sprite Plane A flip done",
 	[SPRITE_B_FLIP_DONE] = "Sprite Plane B flip done",
 	[SPRITE_C_FLIP_DONE] = "Sprite Plane C flip done",
+	[CURSOR_A_FLIP_DONE] = "Cursor Plane A flip done",
+	[CURSOR_B_FLIP_DONE] = "Cursor Plane B flip done",
+	[CURSOR_C_FLIP_DONE] = "Cursor Plane C flip done",
 
 	[PCU_THERMAL] = "PCU Thermal Event",
 	[PCU_PCODE2DRIVER_MAILBOX] = "PCU pcode2driver mailbox event",
@@ -593,6 +596,10 @@ static void gen8_init_irq(
 		SET_BIT_INFO(irq, 4, SPRITE_A_FLIP_DONE, INTEL_GVT_IRQ_INFO_DE_PIPE_A);
 		SET_BIT_INFO(irq, 4, SPRITE_B_FLIP_DONE, INTEL_GVT_IRQ_INFO_DE_PIPE_B);
 		SET_BIT_INFO(irq, 4, SPRITE_C_FLIP_DONE, INTEL_GVT_IRQ_INFO_DE_PIPE_C);
+
+		SET_BIT_INFO(irq, 6, CURSOR_A_FLIP_DONE, INTEL_GVT_IRQ_INFO_DE_PIPE_A);
+		SET_BIT_INFO(irq, 6, CURSOR_B_FLIP_DONE, INTEL_GVT_IRQ_INFO_DE_PIPE_B);
+		SET_BIT_INFO(irq, 6, CURSOR_C_FLIP_DONE, INTEL_GVT_IRQ_INFO_DE_PIPE_C);
 	}
 
 	/* GEN8 interrupt PCU events */
diff --git a/drivers/gpu/drm/i915/gvt/interrupt.h b/drivers/gpu/drm/i915/gvt/interrupt.h
index 5313fb1b33e1..158f1c7a23f2 100644
--- a/drivers/gpu/drm/i915/gvt/interrupt.h
+++ b/drivers/gpu/drm/i915/gvt/interrupt.h
@@ -92,6 +92,9 @@ enum intel_gvt_event_type {
 	SPRITE_A_FLIP_DONE,
 	SPRITE_B_FLIP_DONE,
 	SPRITE_C_FLIP_DONE,
+	CURSOR_A_FLIP_DONE,
+	CURSOR_B_FLIP_DONE,
+	CURSOR_C_FLIP_DONE,
 
 	PCU_THERMAL,
 	PCU_PCODE2DRIVER_MAILBOX,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace
  2019-09-24  6:41 ` [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace Tina Zhang
@ 2019-09-24  9:02   ` kbuild test robot
  2019-09-24 20:10   ` Alex Williamson
  1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2019-09-24  9:02 UTC (permalink / raw)
  To: Tina Zhang
  Cc: kbuild-all, intel-gvt-dev, kvm, linux-kernel, Tina Zhang, kraxel,
	zhenyuw, zhiyuan.lv, zhi.a.wang, kevin.tian, hang.yuan,
	alex.williamson, yi.l.liu, Kechen Lu

[-- Attachment #1: Type: text/plain, Size: 3387 bytes --]

Hi Tina,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[cannot apply to v5.3 next-20190920]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tina-Zhang/Deliver-vGPU-display-refresh-event-to-userspace/20190924-145111
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/gvt/display.c: In function 'emulate_vblank_on_pipe':
>> drivers/gpu/drm/i915/gvt/display.c:429:10: error: 'struct intel_vgpu' has no member named 'vdev'
     if (vgpu->vdev.vblank_trigger && !vgpu->vdev.display_event_mask &&
             ^~
   drivers/gpu/drm/i915/gvt/display.c:429:40: error: 'struct intel_vgpu' has no member named 'vdev'
     if (vgpu->vdev.vblank_trigger && !vgpu->vdev.display_event_mask &&
                                           ^~
   drivers/gpu/drm/i915/gvt/display.c:431:22: error: 'struct intel_vgpu' has no member named 'vdev'
      eventfd_signal(vgpu->vdev.vblank_trigger, eventfd_signal_val);
                         ^~

vim +429 drivers/gpu/drm/i915/gvt/display.c

   393	
   394	static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
   395	{
   396		struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
   397		struct intel_vgpu_irq *irq = &vgpu->irq;
   398		int vblank_event[] = {
   399			[PIPE_A] = PIPE_A_VBLANK,
   400			[PIPE_B] = PIPE_B_VBLANK,
   401			[PIPE_C] = PIPE_C_VBLANK,
   402		};
   403		int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
   404		int event;
   405		u64 eventfd_signal_val = 0;
   406	
   407		if (pipe < PIPE_A || pipe > PIPE_C)
   408			return;
   409	
   410		for_each_set_bit(event, irq->flip_done_event[pipe],
   411				INTEL_GVT_EVENT_MAX) {
   412			clear_bit(event, irq->flip_done_event[pipe]);
   413			if (!pipe_is_enabled(vgpu, pipe))
   414				continue;
   415	
   416			if (event == pri_flip_event)
   417				eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
   418	
   419			intel_vgpu_trigger_virtual_event(vgpu, event);
   420		}
   421	
   422		if (eventfd_signal_val)
   423			vgpu->no_pageflip_count = 0;
   424		else if (!eventfd_signal_val && vgpu->no_pageflip_count > PAGEFLIP_DELAY_THR)
   425			eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
   426		else
   427			vgpu->no_pageflip_count++;
   428	
 > 429		if (vgpu->vdev.vblank_trigger && !vgpu->vdev.display_event_mask &&
   430			eventfd_signal_val)
   431			eventfd_signal(vgpu->vdev.vblank_trigger, eventfd_signal_val);
   432	
   433		if (pipe_is_enabled(vgpu, pipe)) {
   434			vgpu_vreg_t(vgpu, PIPE_FRMCOUNT_G4X(pipe))++;
   435			intel_vgpu_trigger_virtual_event(vgpu, vblank_event[pipe]);
   436		}
   437	}
   438	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50927 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace
  2019-09-24  6:41 ` [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace Tina Zhang
  2019-09-24  9:02   ` kbuild test robot
@ 2019-09-24 20:10   ` Alex Williamson
  1 sibling, 0 replies; 11+ messages in thread
From: Alex Williamson @ 2019-09-24 20:10 UTC (permalink / raw)
  To: Tina Zhang
  Cc: intel-gvt-dev, kvm, linux-kernel, kraxel, zhenyuw, zhiyuan.lv,
	zhi.a.wang, kevin.tian, hang.yuan, yi.l.liu, Kechen Lu

On Tue, 24 Sep 2019 14:41:41 +0800
Tina Zhang <tina.zhang@intel.com> wrote:

> Deliver the display refresh events to the user land. Userspace can use
> the irq mask/unmask mechanism to disable or enable the event delivery.
> 
> As we know, delivering refresh event at each vblank safely avoids
> tearing and unexpected event overwhelming, but there are still spaces
> to optimize.
> 
> For handling the normal case, deliver the page flip refresh
> event at each vblank, in other words, bounded by vblanks. Skipping some
> events bring performance enhancement while not hurting user experience.
> 
> For single framebuffer case, deliver the refresh events to userspace at
> all vblanks. This heuristic at each vblank leverages pageflip_count
> incresements to determine if there is no page flip happens after a certain
> period and so that the case is regarded as single framebuffer one.
> Although this heuristic makes incorrect decision sometimes and it depends
> on guest behavior, for example, when no cursor movements happen, the
> user experience does not harm and front buffer is still correctly acquired.
> Meanwhile, in actual single framebuffer case, the user experience is
> enhanced compared with page flip events only.
> 
> Addtionally, to mitigate the events delivering footprints, one eventfd and
> 8 byte eventfd counter partition are leveraged.
> 
> v3:
> - make no_pageflip_count be per-vgpu instead of static. (Zhenyu)
> 
> v2:
> - Support vfio_irq_info_cap_display_plane_events. (Tina)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> Signed-off-by: Kechen Lu <kechen.lu@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/display.c |  20 ++++
>  drivers/gpu/drm/i915/gvt/gvt.h     |   3 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c   | 159 +++++++++++++++++++++++++++--
>  3 files changed, 173 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
> index 1a0a4ae4826e..9f2c2cd10369 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -34,6 +34,8 @@
>  
>  #include "i915_drv.h"
>  #include "gvt.h"
> +#include <uapi/linux/vfio.h>
> +#include <drm/drm_plane.h>
>  
>  static int get_edp_pipe(struct intel_vgpu *vgpu)
>  {
> @@ -387,6 +389,8 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt)
>  	mutex_unlock(&gvt->lock);
>  }
>  
> +#define PAGEFLIP_DELAY_THR 10
> +
>  static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> @@ -396,7 +400,9 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
>  		[PIPE_B] = PIPE_B_VBLANK,
>  		[PIPE_C] = PIPE_C_VBLANK,
>  	};
> +	int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
>  	int event;
> +	u64 eventfd_signal_val = 0;
>  
>  	if (pipe < PIPE_A || pipe > PIPE_C)
>  		return;
> @@ -407,9 +413,23 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
>  		if (!pipe_is_enabled(vgpu, pipe))
>  			continue;
>  
> +		if (event == pri_flip_event)
> +			eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
> +
>  		intel_vgpu_trigger_virtual_event(vgpu, event);
>  	}
>  
> +	if (eventfd_signal_val)
> +		vgpu->no_pageflip_count = 0;
> +	else if (!eventfd_signal_val && vgpu->no_pageflip_count > PAGEFLIP_DELAY_THR)

The !eventfd_signal_val test is redundant since this is and else branch
of a test for eventfd_signal_val.

> +		eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
> +	else
> +		vgpu->no_pageflip_count++;
> +
> +	if (vgpu->vdev.vblank_trigger && !vgpu->vdev.display_event_mask &&
> +		eventfd_signal_val)
> +		eventfd_signal(vgpu->vdev.vblank_trigger, eventfd_signal_val);
> +
>  	if (pipe_is_enabled(vgpu, pipe)) {
>  		vgpu_vreg_t(vgpu, PIPE_FRMCOUNT_G4X(pipe))++;
>  		intel_vgpu_trigger_virtual_event(vgpu, vblank_event[pipe]);
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 8008047d026c..cc39b449b061 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -205,6 +205,8 @@ struct intel_vgpu {
>  		int num_irqs;
>  		struct eventfd_ctx *intx_trigger;
>  		struct eventfd_ctx *msi_trigger;
> +		struct eventfd_ctx *vblank_trigger;
> +		bool display_event_mask;
>  
>  		/*
>  		 * Two caches are used to avoid mapping duplicated pages (eg.
> @@ -229,6 +231,7 @@ struct intel_vgpu {
>  	struct idr object_idr;
>  
>  	struct completion vblank_done;
> +	int no_pageflip_count;
>  
>  	u32 scan_nonprivbb;
>  };
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 269506300310..f30b7a5272e8 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1250,6 +1250,8 @@ static int intel_vgpu_get_irq_count(struct intel_vgpu *vgpu, int type)
>  {
>  	if (type == VFIO_PCI_INTX_IRQ_INDEX || type == VFIO_PCI_MSI_IRQ_INDEX)
>  		return 1;
> +	else if (type < VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs)
> +		return vgpu->vdev.irq[type - VFIO_PCI_NUM_IRQS].count;
>  
>  	return 0;
>  }
> @@ -1297,7 +1299,60 @@ static int intel_vgpu_set_msi_trigger(struct intel_vgpu *vgpu,
>  	return 0;
>  }
>  
> -static int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
> +static int intel_vgu_set_display_irq_mask(struct intel_vgpu *vgpu,
> +		unsigned int index, unsigned int start, unsigned int count,
> +		u32 flags, void *data)
> +{
> +	if (start != 0 || count > 2)
> +		return -EINVAL;

Was this meant to be >= 2?  Or just > 1?  I think we're only
advertising an index with a count of 1.

> +
> +	if (flags & VFIO_IRQ_SET_DATA_NONE)
> +		vgpu->vdev.display_event_mask = true;

It's also valid for a user to set this with DATA_BOOL.

Both comments above apply to instances below as well.

> +
> +	return 0;
> +}
> +
> +static int intel_vgu_set_display_irq_unmask(struct intel_vgpu *vgpu,
> +		unsigned int index, unsigned int start, unsigned int count,
> +		u32 flags, void *data)
> +{
> +	if (start != 0 || count > 2)
> +		return -EINVAL;
> +
> +	if (flags & VFIO_IRQ_SET_DATA_NONE)
> +		vgpu->vdev.display_event_mask = false;
> +
> +	return 0;
> +}
> +
> +static int intel_vgpu_set_display_event_trigger(struct intel_vgpu *vgpu,
> +		unsigned int index, unsigned int start, unsigned int count,
> +		u32 flags, void *data)
> +{
> +	struct eventfd_ctx *trigger;

start and count are ignored here, the user could pass anything.

> +	if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
> +		int fd = *(int *)data;
> +
> +		trigger = eventfd_ctx_fdget(fd);
> +		if (IS_ERR(trigger)) {
> +			gvt_vgpu_err("eventfd_ctx_fdget failed\n");
> +			return PTR_ERR(trigger);
> +		}
> +		vgpu->vdev.vblank_trigger = trigger;
> +		vgpu->vdev.display_event_mask = false;
> +	} else if ((flags & VFIO_IRQ_SET_DATA_NONE) && !count) {
> +		trigger = vgpu->vdev.vblank_trigger;
> +		if (trigger) {
> +			eventfd_ctx_put(trigger);
> +			vgpu->vdev.vblank_trigger = NULL;
> +		}
> +	}

I don't think this supports all user interactions either, notably
providing an eventfd of -1 to disable the IRQ as defined in the uapi.
Thanks,

Alex

> +
> +	return 0;
> +}
> +
> +int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
>  		unsigned int index, unsigned int start, unsigned int count,
>  		void *data)
>  {
> @@ -1330,6 +1385,35 @@ static int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
>  			break;
>  		}
>  		break;
> +	default:
> +	{
> +		int i;
> +
> +		if (index >= VFIO_PCI_NUM_IRQS +
> +					vgpu->vdev.num_irqs)
> +			return -EINVAL;
> +		index =
> +			array_index_nospec(index,
> +						VFIO_PCI_NUM_IRQS +
> +						vgpu->vdev.num_irqs);
> +
> +		i = index - VFIO_PCI_NUM_IRQS;
> +		if (vgpu->vdev.irq[i].type == VFIO_IRQ_TYPE_GFX &&
> +		    vgpu->vdev.irq[i].subtype ==
> +		    VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ) {
> +			switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
> +			case VFIO_IRQ_SET_ACTION_MASK:
> +				func = intel_vgu_set_display_irq_mask;
> +				break;
> +			case VFIO_IRQ_SET_ACTION_UNMASK:
> +				func = intel_vgu_set_display_irq_unmask;
> +				break;
> +			case VFIO_IRQ_SET_ACTION_TRIGGER:
> +				func = intel_vgpu_set_display_event_trigger;
> +				break;
> +			}
> +		}
> +	}
>  	}
>  
>  	if (!func)
> @@ -1361,7 +1445,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
>  		info.flags |= VFIO_DEVICE_FLAGS_RESET;
>  		info.num_regions = VFIO_PCI_NUM_REGIONS +
>  				vgpu->vdev.num_regions;
> -		info.num_irqs = VFIO_PCI_NUM_IRQS;
> +		info.num_irqs = VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs;
>  
>  		return copy_to_user((void __user *)arg, &info, minsz) ?
>  			-EFAULT : 0;
> @@ -1519,32 +1603,88 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
>  			-EFAULT : 0;
>  	} else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
>  		struct vfio_irq_info info;
> +		struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> +		unsigned int i;
> +		int ret;
>  
>  		minsz = offsetofend(struct vfio_irq_info, count);
>  
>  		if (copy_from_user(&info, (void __user *)arg, minsz))
>  			return -EFAULT;
>  
> -		if (info.argsz < minsz || info.index >= VFIO_PCI_NUM_IRQS)
> +		if (info.argsz < minsz)
>  			return -EINVAL;
>  
>  		switch (info.index) {
>  		case VFIO_PCI_INTX_IRQ_INDEX:
>  		case VFIO_PCI_MSI_IRQ_INDEX:
> +			info.flags = VFIO_IRQ_INFO_EVENTFD;
>  			break;
> -		default:
> +		case VFIO_PCI_MSIX_IRQ_INDEX:
> +		case VFIO_PCI_ERR_IRQ_INDEX:
> +		case VFIO_PCI_REQ_IRQ_INDEX:
>  			return -EINVAL;
> -		}
> +		default:
> +		{
> +			struct vfio_irq_info_cap_type cap_type = {
> +				.header.id = VFIO_IRQ_INFO_CAP_TYPE,
> +				.header.version = 1 };
>  
> -		info.flags = VFIO_IRQ_INFO_EVENTFD;
> +			if (info.index >= VFIO_PCI_NUM_IRQS +
> +					vgpu->vdev.num_irqs)
> +				return -EINVAL;
> +			info.index =
> +				array_index_nospec(info.index,
> +						VFIO_PCI_NUM_IRQS +
> +						vgpu->vdev.num_irqs);
> +
> +			i = info.index - VFIO_PCI_NUM_IRQS;
> +
> +			info.flags = vgpu->vdev.irq[i].flags;
> +			cap_type.type = vgpu->vdev.irq[i].type;
> +			cap_type.subtype = vgpu->vdev.irq[i].subtype;
> +
> +			ret = vfio_info_add_capability(&caps,
> +						&cap_type.header,
> +						sizeof(cap_type));
> +			if (ret)
> +				return ret;
> +
> +			if (vgpu->vdev.irq[i].ops->add_capability) {
> +				ret = vgpu->vdev.irq[i].ops->add_capability(vgpu,
> +									    &caps);
> +				if (ret)
> +					return ret;
> +			}
> +		}
> +		}
>  
>  		info.count = intel_vgpu_get_irq_count(vgpu, info.index);
>  
>  		if (info.index == VFIO_PCI_INTX_IRQ_INDEX)
>  			info.flags |= (VFIO_IRQ_INFO_MASKABLE |
>  				       VFIO_IRQ_INFO_AUTOMASKED);
> -		else
> -			info.flags |= VFIO_IRQ_INFO_NORESIZE;
> +
> +		if (caps.size) {
> +			info.flags |= VFIO_IRQ_INFO_FLAG_CAPS;
> +			if (info.argsz < sizeof(info) + caps.size) {
> +				info.argsz = sizeof(info) + caps.size;
> +				info.cap_offset = 0;
> +			} else {
> +				vfio_info_cap_shift(&caps, sizeof(info));
> +				if (copy_to_user((void __user *)arg +
> +						  sizeof(info), caps.buf,
> +						  caps.size)) {
> +					kfree(caps.buf);
> +					return -EFAULT;
> +				}
> +				info.cap_offset = sizeof(info);
> +				if (offsetofend(struct vfio_irq_info, cap_offset) > minsz)
> +					minsz = offsetofend(struct vfio_irq_info, cap_offset);
> +			}
> +
> +			kfree(caps.buf);
> +		}
>  
>  		return copy_to_user((void __user *)arg, &info, minsz) ?
>  			-EFAULT : 0;
> @@ -1563,7 +1703,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
>  			int max = intel_vgpu_get_irq_count(vgpu, hdr.index);
>  
>  			ret = vfio_set_irqs_validate_and_prepare(&hdr, max,
> -						VFIO_PCI_NUM_IRQS, &data_size);
> +					VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs,
> +								 &data_size);
>  			if (ret) {
>  				gvt_vgpu_err("intel:vfio_set_irqs_validate_and_prepare failed\n");
>  				return -EINVAL;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 5/6] drm/i915/gvt: Deliver async primary plane page flip events at vblank
  2019-09-24  6:41 ` [PATCH v6 5/6] drm/i915/gvt: Deliver async primary plane page flip events at vblank Tina Zhang
@ 2019-09-24 20:14   ` Alex Williamson
  0 siblings, 0 replies; 11+ messages in thread
From: Alex Williamson @ 2019-09-24 20:14 UTC (permalink / raw)
  To: Tina Zhang
  Cc: intel-gvt-dev, kvm, linux-kernel, Kechen Lu, kraxel, zhenyuw,
	zhiyuan.lv, zhi.a.wang, kevin.tian, hang.yuan, yi.l.liu

On Tue, 24 Sep 2019 14:41:42 +0800
Tina Zhang <tina.zhang@intel.com> wrote:

> From: Kechen Lu <kechen.lu@intel.com>
> 
> Only sync primary plane page flip events are checked and delivered
> as the display refresh events before, this patch tries to deliver async
> primary page flip events bounded by vblanks.
> 
> To deliver correct async page flip, the new async flip bitmap is
> introduced and in vblank emulation handler to check bitset.
> 
> Signed-off-by: Kechen Lu <kechen.lu@intel.com>
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/cmd_parser.c |  6 ++++--
>  drivers/gpu/drm/i915/gvt/display.c    | 10 ++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.h        |  2 ++
>  drivers/gpu/drm/i915/gvt/handlers.c   |  5 +++--
>  4 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index e753b1e706e2..1abb05431177 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -1365,9 +1365,11 @@ static int gen8_update_plane_mmio_from_mi_display_flip(
>  	if (info->plane == PLANE_PRIMARY)
>  		vgpu_vreg_t(vgpu, PIPE_FLIPCOUNT_G4X(info->pipe))++;
>  
> -	if (info->async_flip)
> +	if (info->async_flip) {
>  		intel_vgpu_trigger_virtual_event(vgpu, info->event);
> -	else
> +		set_bit(info->plane,
> +			vgpu->display.async_flip_event[info->pipe]);
> +	} else
>  		set_bit(info->event, vgpu->irq.flip_done_event[info->pipe]);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
> index 9f2c2cd10369..9acde0bdd5f4 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -419,6 +419,16 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
>  		intel_vgpu_trigger_virtual_event(vgpu, event);
>  	}
>  
> +	for_each_set_bit(event, vgpu->display.async_flip_event[pipe],
> +			I915_MAX_PLANES) {
> +		clear_bit(event, vgpu->display.async_flip_event[pipe]);
> +		if (!pipe_is_enabled(vgpu, pipe))
> +			continue;
> +
> +		if (event == PLANE_PRIMARY)
> +			eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;

Is it worthwhile to continue the for_each_set_bit here, or should we
clear the remaining bits and break from the loop?  Thanks,

Alex

> +	}
> +
>  	if (eventfd_signal_val)
>  		vgpu->no_pageflip_count = 0;
>  	else if (!eventfd_signal_val && vgpu->no_pageflip_count > PAGEFLIP_DELAY_THR)
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index cc39b449b061..73769a87b407 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -128,6 +128,8 @@ struct intel_vgpu_display {
>  	struct intel_vgpu_i2c_edid i2c_edid;
>  	struct intel_vgpu_port ports[I915_MAX_PORTS];
>  	struct intel_vgpu_sbi sbi;
> +	DECLARE_BITMAP(async_flip_event[I915_MAX_PIPES],
> +		       I915_MAX_PLANES);
>  };
>  
>  struct vgpu_sched_ctl {
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> index 45a9124e53b6..e5a022c2e7bb 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -760,9 +760,10 @@ static int pri_surf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
>  
>  	vgpu_vreg_t(vgpu, PIPE_FLIPCOUNT_G4X(pipe))++;
>  
> -	if (vgpu_vreg_t(vgpu, DSPCNTR(pipe)) & PLANE_CTL_ASYNC_FLIP)
> +	if (vgpu_vreg_t(vgpu, DSPCNTR(pipe)) & PLANE_CTL_ASYNC_FLIP) {
>  		intel_vgpu_trigger_virtual_event(vgpu, event);
> -	else
> +		set_bit(PLANE_PRIMARY, vgpu->display.async_flip_event[pipe]);
> +	} else
>  		set_bit(event, vgpu->irq.flip_done_event[pipe]);
>  
>  	return 0;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 2/6] vfio: Introduce vGPU display irq type
  2019-09-24  6:41 ` [PATCH v6 2/6] vfio: Introduce vGPU display irq type Tina Zhang
@ 2019-09-24 20:35   ` Alex Williamson
  0 siblings, 0 replies; 11+ messages in thread
From: Alex Williamson @ 2019-09-24 20:35 UTC (permalink / raw)
  To: Tina Zhang
  Cc: intel-gvt-dev, kvm, linux-kernel, kraxel, zhenyuw, zhiyuan.lv,
	zhi.a.wang, kevin.tian, hang.yuan, yi.l.liu

On Tue, 24 Sep 2019 14:41:39 +0800
Tina Zhang <tina.zhang@intel.com> wrote:

> Introduce vGPU specific irq type VFIO_IRQ_TYPE_GFX, and
> VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ as the subtype for vGPU display.
> 
> Introduce vfio_irq_info_cap_display_plane_events capability to notify
> user space with the vGPU's plane update events
> 
> v3:
> - Add more description to VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ and
>   VFIO_IRQ_INFO_CAP_DISPLAY. (Alex & Gerd)
> 
> v2:
> - Add VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ description. (Alex & Kechen)
> - Introduce vfio_irq_info_cap_display_plane_events. (Gerd & Alex)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> ---
>  include/uapi/linux/vfio.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index aa6850f1daef..2946a028b0c3 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -476,6 +476,44 @@ struct vfio_irq_info_cap_type {
>  	__u32 subtype;  /* type specific */
>  };
>  
> +/* vGPU IRQ TYPE */
> +#define VFIO_IRQ_TYPE_GFX			(1)
> +
> +/* sub-types for VFIO_IRQ_TYPE_GFX */
> +/*
> + * vGPU device display refresh interrupt request. This irq asking for
> + * a user space display refresh, covers all display updates events,
> + * i.e. user space can stop the display update timer and fully depend
> + * on getting the notification if an update is needed.
> + */
> +#define VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ	(1)
> +
> +/*
> + * Display capability of reporting display refresh interrupt events.

Perhaps, "Capability for interpreting GFX_DISPLAY_IRQ eventfd value"

> + * This gives user space the capability to identify different display
> + * updates events of the display refresh interrupt request.
> + *
> + * When notified by VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ, user space can
> + * use the eventfd counter value to identify which plane has been
> + * updated.
> + *
> + * Note that there might be some cases like counter_value >
> + * (cur_event_val + pri_event_val), if notifications haven't been
> + * handled on time in user mode. These cases can be handled as all
> + * plane updated case or signle plane updated case depending on the
> + * value.

Seems like in the GVT-g implementation such a value is not possible.
In fact, when this capability is provided, doesn't userspace interpret
the eventfd value more as a bitmask of events rather than a counter?
If so, (cur_event_val + pri_event_val) may be mathematically accurate,
but maybe obfuscates the logical interpretation... or maybe that's just
me.

> + *
> + * cur_event_val: eventfd counter value for cursor plane change event.
> + * pri_event_val: eventfd counter value for primary plane change event.

I think there should be a note that this capability is optional and
lacking this feature, userspace should refresh all display events on
notification.

> + */
> +#define VFIO_IRQ_INFO_CAP_DISPLAY	2
> +
> +struct vfio_irq_info_cap_display_plane_events {
> +	struct vfio_info_cap_header header;
> +	__u64 cur_event_val;
> +	__u64 pri_event_val;

AIUI, the GVT-g implementation sets a single bit and userspace expects
one or both of those bits to be set on notification.  Should we
simplify this a bit and just define these as cur_event_bit,
pri_event_bit and use a __u8 for each to define the bit position?
Thanks,

Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-09-24 20:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-24  6:41 [PATCH v6 0/6] Deliver vGPU display refresh event to userspace Tina Zhang
2019-09-24  6:41 ` [PATCH v6 1/6] vfio: Define device specific irq type capability Tina Zhang
2019-09-24  6:41 ` [PATCH v6 2/6] vfio: Introduce vGPU display irq type Tina Zhang
2019-09-24 20:35   ` Alex Williamson
2019-09-24  6:41 ` [PATCH v6 3/6] drm/i915/gvt: Register vGPU display event irq Tina Zhang
2019-09-24  6:41 ` [PATCH v6 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace Tina Zhang
2019-09-24  9:02   ` kbuild test robot
2019-09-24 20:10   ` Alex Williamson
2019-09-24  6:41 ` [PATCH v6 5/6] drm/i915/gvt: Deliver async primary plane page flip events at vblank Tina Zhang
2019-09-24 20:14   ` Alex Williamson
2019-09-24  6:41 ` [PATCH v6 6/6] drm/i915/gvt: Add cursor plane reg update trap emulation handler Tina Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).