All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/9] Introduce the implementation of GVT context
@ 2016-06-02 16:36 Zhi Wang
  2016-06-02 16:36 ` [PATCH v6 1/9] drm/i915: Factor out i915_pvinfo.h Zhi Wang
                   ` (9 more replies)
  0 siblings, 10 replies; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

Mostly this patchset introduces the implementation of GVT context. GVT
context is a special GEM context used by GVT-g. GVT-g uses it as the shadow
context.It doesn't have a drm client nor a PPGTT. And it requires a larger
ring buffer with several special features need by GVT-g workload scheduler
like context status change notification, context single submission...

v6:

- Take Chris comments.

v5:

- Drop PPGTT related patches.
- Let most functions take struct drm_i915_private *
- Fixed some misspelled words in Kconfig
- Only complied some feature when CONFIG_DRM_I915_GVT=y
- Drop the fecne related changes, will send it after this series.

v4:

- Based on the latest drm-intel-nightly branch.
- Drop PPGTT refactor patches. (GVT-g will use LRI to load PDPs)
- Drop i915_gem_context() refactor patches, reuse kernel context functions.
  (Dave Gordon)
- Drop context allocation params and refactor as the lrc deferred
  allocation function has been refactored in another styles.
- Re-wrtie GVT context creation function

Difference from community release
---------------------------------

This patchset is different from regular iGVT-g code release[4], which
is still based on old host-mediated architecture. Furthermore, this
patchset only supports BDW whereas code release supports HSW/BDW/SKL.
We will add SKL support later based on this RFC code and HSW support
will be dropped.

Internally we tested this RFC patchset with both linux and windows VM
and the architecture changes work fine.

Acknowledgment
---------------

iGVT-g implementation is several years effort and many people
contributed to the code. There names are not here yet. In later formal
patchset we will reflect individual's contribution.

Meanwhile, in the previous iGVT-g related discussion, Daniel, Chris
and Joonas ever gave very good inputs. We appreciate them and look
forward to more comments/suggestions from community.

We are trying to get more familiar with i915 but may still have gaps.
We are willing to adopt suggestions to keep improving. We hope to work
with community together to make iGVT-g a great component in i915 to
support graphics virtualization. Thanks!

Reference
---------

[1] https://01.org/igvt-g
[2] http://lists.freedesktop.org/archives/intel-gfx/2014-September/053098.html
[3] http://lists.freedesktop.org/archives/intel-gfx/2015-September/075397.html

Bing Niu (1):
  drm/i915: Introduce host graphics memory partition for GVT-g

Zhi Wang (8):
  drm/i915: Factor out i915_pvinfo.h
  drm/i915: Fold vGPU active check into inner functions
  drm/i915: gvt: Introduce the basic architecture of GVT-g
  drm/i915: Make ring buffer size of a LRC context configurable
  drm/i915: Make addressing mode bits in context descriptor configurable
  drm/i915: Introduce execlist context status change notification
  drm/i915: Support LRC context single submission
  drm/i915: Introduce GVT context creation API

 drivers/gpu/drm/i915/Kconfig            |  19 ++++
 drivers/gpu/drm/i915/Makefile           |   5 +
 drivers/gpu/drm/i915/gvt/Makefile       |   5 +
 drivers/gpu/drm/i915/gvt/debug.h        |  34 ++++++
 drivers/gpu/drm/i915/gvt/gvt.c          | 181 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h          |  75 +++++++++++++
 drivers/gpu/drm/i915/gvt/hypercall.h    |  38 +++++++
 drivers/gpu/drm/i915/gvt/mpt.h          |  49 +++++++++
 drivers/gpu/drm/i915/i915_dma.c         |  17 ++-
 drivers/gpu/drm/i915/i915_drv.h         |  15 +++
 drivers/gpu/drm/i915/i915_gem_context.c |  35 ++++++
 drivers/gpu/drm/i915/i915_gem_gtt.c     |  11 +-
 drivers/gpu/drm/i915/i915_params.c      |   5 +
 drivers/gpu/drm/i915/i915_params.h      |   1 +
 drivers/gpu/drm/i915/i915_pvinfo.h      | 116 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_vgpu.c        |  32 ++++--
 drivers/gpu/drm/i915/i915_vgpu.h        |  90 +---------------
 drivers/gpu/drm/i915/intel_gvt.c        |  90 ++++++++++++++++
 drivers/gpu/drm/i915/intel_gvt.h        |  71 +++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        |  55 +++++++---
 drivers/gpu/drm/i915/intel_lrc.h        |  16 +++
 21 files changed, 841 insertions(+), 119 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
 create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
 create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
 create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
 create mode 100644 drivers/gpu/drm/i915/i915_pvinfo.h
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.h

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v6 1/9] drm/i915: Factor out i915_pvinfo.h
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  8:45   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 2/9] drm/i915: Fold vGPU active check into inner functions Zhi Wang
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

As the PVINFO page definition is used by both GVT-g guest (vGPU) and GVT-g
host (GVT-g kernel device model), factor it out for better code structure.

v3:

Take Joonas's comments:
- Use offsetof to calculate the member offset of PVINFO structure

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_pvinfo.h | 116 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_vgpu.h   |  86 +--------------------------
 2 files changed, 117 insertions(+), 85 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_pvinfo.h

diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
new file mode 100644
index 0000000..eb45afb
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -0,0 +1,116 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _I915_PVINFO_H_
+#define _I915_PVINFO_H_
+
+/* The MMIO offset of the shared info between guest and host emulator */
+#define VGT_PVINFO_PAGE	0x78000
+#define VGT_PVINFO_SIZE	0x1000
+
+/*
+ * The following structure pages are defined in GEN MMIO space
+ * for virtualization. (One page for now)
+ */
+#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
+#define VGT_VERSION_MAJOR 1
+#define VGT_VERSION_MINOR 0
+
+#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
+#define INTEL_VGT_IF_VERSION \
+	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
+
+/*
+ * notifications from guest to vgpu device model
+ */
+enum vgt_g2v_type {
+	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
+	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
+	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
+	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
+	VGT_G2V_EXECLIST_CONTEXT_CREATE,
+	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
+	VGT_G2V_MAX,
+};
+
+struct vgt_if {
+	uint64_t magic;		/* VGT_MAGIC */
+	uint16_t version_major;
+	uint16_t version_minor;
+	uint32_t vgt_id;	/* ID of vGT instance */
+	uint32_t rsv1[12];	/* pad to offset 0x40 */
+	/*
+	 *  Data structure to describe the balooning info of resources.
+	 *  Each VM can only have one portion of continuous area for now.
+	 *  (May support scattered resource in future)
+	 *  (starting from offset 0x40)
+	 */
+	struct {
+		/* Aperture register balooning */
+		struct {
+			uint32_t base;
+			uint32_t size;
+		} mappable_gmadr;	/* aperture */
+		/* GMADR register balooning */
+		struct {
+			uint32_t base;
+			uint32_t size;
+		} nonmappable_gmadr;	/* non aperture */
+		/* allowed fence registers */
+		uint32_t fence_num;
+		uint32_t rsv2[3];
+	} avail_rs;		/* available/assigned resource */
+	uint32_t rsv3[0x200 - 24];	/* pad to half page */
+	/*
+	 * The bottom half page is for response from Gfx driver to hypervisor.
+	 */
+	uint32_t rsv4;
+	uint32_t display_ready;	/* ready for display owner switch */
+
+	uint32_t rsv5[4];
+
+	uint32_t g2v_notify;
+	uint32_t rsv6[7];
+
+	struct {
+		uint32_t lo;
+		uint32_t hi;
+	} pdp[4];
+
+	uint32_t execlist_context_descriptor_lo;
+	uint32_t execlist_context_descriptor_hi;
+
+	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
+} __packed;
+
+#define _vgtif_reg(x) \
+	(VGT_PVINFO_PAGE + offsetof(struct vgt_if, x))
+
+#define vgtif_reg(x) \
+	_MMIO(_vgtif_reg(x))
+
+/* vGPU display status to be used by the host side */
+#define VGT_DRV_DISPLAY_NOT_READY 0
+#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
+
+#endif /* _I915_PVINFO_H_ */
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 21ffcfe..07e67d5 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -24,91 +24,7 @@
 #ifndef _I915_VGPU_H_
 #define _I915_VGPU_H_
 
-/* The MMIO offset of the shared info between guest and host emulator */
-#define VGT_PVINFO_PAGE	0x78000
-#define VGT_PVINFO_SIZE	0x1000
-
-/*
- * The following structure pages are defined in GEN MMIO space
- * for virtualization. (One page for now)
- */
-#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
-#define VGT_VERSION_MAJOR 1
-#define VGT_VERSION_MINOR 0
-
-#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
-#define INTEL_VGT_IF_VERSION \
-	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
-
-/*
- * notifications from guest to vgpu device model
- */
-enum vgt_g2v_type {
-	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
-	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
-	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
-	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
-	VGT_G2V_EXECLIST_CONTEXT_CREATE,
-	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
-	VGT_G2V_MAX,
-};
-
-struct vgt_if {
-	uint64_t magic;		/* VGT_MAGIC */
-	uint16_t version_major;
-	uint16_t version_minor;
-	uint32_t vgt_id;	/* ID of vGT instance */
-	uint32_t rsv1[12];	/* pad to offset 0x40 */
-	/*
-	 *  Data structure to describe the balooning info of resources.
-	 *  Each VM can only have one portion of continuous area for now.
-	 *  (May support scattered resource in future)
-	 *  (starting from offset 0x40)
-	 */
-	struct {
-		/* Aperture register balooning */
-		struct {
-			uint32_t base;
-			uint32_t size;
-		} mappable_gmadr;	/* aperture */
-		/* GMADR register balooning */
-		struct {
-			uint32_t base;
-			uint32_t size;
-		} nonmappable_gmadr;	/* non aperture */
-		/* allowed fence registers */
-		uint32_t fence_num;
-		uint32_t rsv2[3];
-	} avail_rs;		/* available/assigned resource */
-	uint32_t rsv3[0x200 - 24];	/* pad to half page */
-	/*
-	 * The bottom half page is for response from Gfx driver to hypervisor.
-	 */
-	uint32_t rsv4;
-	uint32_t display_ready;	/* ready for display owner switch */
-
-	uint32_t rsv5[4];
-
-	uint32_t g2v_notify;
-	uint32_t rsv6[7];
-
-	struct {
-		uint32_t lo;
-		uint32_t hi;
-	} pdp[4];
-
-	uint32_t execlist_context_descriptor_lo;
-	uint32_t execlist_context_descriptor_hi;
-
-	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
-} __packed;
-
-#define vgtif_reg(x) \
-	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
-
-/* vGPU display status to be used by the host side */
-#define VGT_DRV_DISPLAY_NOT_READY 0
-#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
+#include "i915_pvinfo.h"
 
 extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
 extern int intel_vgt_balloon(struct drm_device *dev);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 2/9] drm/i915: Fold vGPU active check into inner functions
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
  2016-06-02 16:36 ` [PATCH v6 1/9] drm/i915: Factor out i915_pvinfo.h Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  8:49   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

v5:
- Let functions take struct drm_i915_private *. (Tvrtko)

- Fold vGPU related active check into the inner functions. (Kevin)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++-------
 drivers/gpu/drm/i915/i915_vgpu.c    | 13 +++++++++----
 drivers/gpu/drm/i915/i915_vgpu.h    |  4 ++--
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4668477..6f203fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2732,11 +2732,9 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
 	i915_address_space_init(&ggtt->base, dev_priv);
 	ggtt->base.total += PAGE_SIZE;
 
-	if (intel_vgpu_active(dev_priv)) {
-		ret = intel_vgt_balloon(dev);
-		if (ret)
-			return ret;
-	}
+	ret = intel_vgt_balloon(dev_priv);
+	if (ret)
+		return ret;
 
 	if (!HAS_LLC(dev))
 		ggtt->base.mm.color_adjust = i915_gtt_color_adjust;
@@ -2836,8 +2834,7 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev)
 	i915_gem_cleanup_stolen(dev);
 
 	if (drm_mm_initialized(&ggtt->base.mm)) {
-		if (intel_vgpu_active(dev_priv))
-			intel_vgt_deballoon();
+		intel_vgt_deballoon(dev_priv);
 
 		drm_mm_takedown(&ggtt->base.mm);
 		list_del(&ggtt->base.global_link);
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index d5a7a5e..5312816 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -101,10 +101,13 @@ static struct _balloon_info_ bl_info;
  * This function is called to deallocate the ballooned-out graphic memory, when
  * driver is unloaded or when ballooning fails.
  */
-void intel_vgt_deballoon(void)
+void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
 {
 	int i;
 
+	if (!intel_vgpu_active(dev_priv))
+		return;
+
 	DRM_DEBUG("VGT deballoon.\n");
 
 	for (i = 0; i < 4; i++) {
@@ -177,9 +180,8 @@ static int vgt_balloon_space(struct drm_mm *mm,
  * Returns:
  * zero on success, non-zero if configuration invalid or ballooning failed
  */
-int intel_vgt_balloon(struct drm_device *dev)
+int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	unsigned long ggtt_end = ggtt->base.start + ggtt->base.total;
 
@@ -187,6 +189,9 @@ int intel_vgt_balloon(struct drm_device *dev)
 	unsigned long unmappable_base, unmappable_size, unmappable_end;
 	int ret;
 
+	if (!intel_vgpu_active(dev_priv))
+		return 0;
+
 	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
 	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
 	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
@@ -258,6 +263,6 @@ int intel_vgt_balloon(struct drm_device *dev)
 
 err:
 	DRM_ERROR("VGT balloon fail\n");
-	intel_vgt_deballoon();
+	intel_vgt_deballoon(dev_priv);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 07e67d5..f8917c6 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -27,7 +27,7 @@
 #include "i915_pvinfo.h"
 
 extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
-extern int intel_vgt_balloon(struct drm_device *dev);
-extern void intel_vgt_deballoon(void);
+extern int intel_vgt_balloon(struct drm_i915_private *dev_priv);
+extern void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
 
 #endif /* _I915_VGPU_H_ */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
  2016-06-02 16:36 ` [PATCH v6 1/9] drm/i915: Factor out i915_pvinfo.h Zhi Wang
  2016-06-02 16:36 ` [PATCH v6 2/9] drm/i915: Fold vGPU active check into inner functions Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  9:14   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 4/9] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

This patch introduces the very basic framework of GVT-g device model,
includes basic prototypes, definitions, initialization.

v6:
- Refine introduction in Kconfig. (Chris)
- The exposed API functions will take struct intel_gvt * instead of
void *. (Chris/Tvrtko)
- Remove most memebers of strct intel_gvt_device_info. Will add them
in the device model patches.(Chris)
- Remove gvt_info() and gvt_err() in debug.h. (Chris)
- Move GVT kernel parameter into i915_params. (Chris)
- Remove include/drm/i915_gvt.h, as GVT-g will be built within i915.
- Remove the redundant struct i915_gvt *, as the functions in i915
will directly take struct intel_gvt *.
- Add more comments for reviewer.

v5:
Take Tvrtko's comments:
- Fix the misspelled words in Kconfig
- Let functions take drm_i915_private * instead of struct drm_device *
- Remove redundant prints/local varible initialization

v3:
Take Joonas' comments:
- Change file name i915_gvt.* to intel_gvt.*
- Move GVT kernel parameter into intel_gvt.c
- Remove redundant debug macros
- Change error handling style
- Add introductions for some stub functions
- Introduce drm/i915_gvt.h.

Take Kevin's comments:
- Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c

v2:
- Introduce i915_gvt.c.
It's necessary to introduce the stubs between i915 driver and GVT-g host,
as GVT-g components is configurable in kernel config. When disabled, the
stubs here do nothing.

Take Joonas' comments:
- Replace boolean return value with int.
- Replace customized info/warn/debug macros with DRM macros.
- Document all non-static functions like i915.
- Remove empty and unused functions.
- Replace magic number with marcos.
- Set GVT-g in kernel config to "n" by default.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/Kconfig         |  19 ++++
 drivers/gpu/drm/i915/Makefile        |   5 +
 drivers/gpu/drm/i915/gvt/Makefile    |   5 +
 drivers/gpu/drm/i915/gvt/debug.h     |  34 +++++++
 drivers/gpu/drm/i915/gvt/gvt.c       | 181 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h       |  75 +++++++++++++++
 drivers/gpu/drm/i915/gvt/hypercall.h |  38 ++++++++
 drivers/gpu/drm/i915/gvt/mpt.h       |  49 ++++++++++
 drivers/gpu/drm/i915/i915_dma.c      |  17 +++-
 drivers/gpu/drm/i915/i915_drv.h      |  10 ++
 drivers/gpu/drm/i915/i915_params.c   |   5 +
 drivers/gpu/drm/i915/i915_params.h   |   1 +
 drivers/gpu/drm/i915/intel_gvt.c     |  90 +++++++++++++++++
 drivers/gpu/drm/i915/intel_gvt.h     |  46 +++++++++
 14 files changed, 571 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
 create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
 create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
 create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.h

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 29a32b1..e4fd9da 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -57,6 +57,25 @@ config DRM_I915_USERPTR
 
 	  If in doubt, say "Y".
 
+config DRM_I915_GVT
+        bool "Enable Intel GVT-g graphics virtualization host support"
+        depends on DRM_I915
+        default n
+        help
+	  Choose this option if you want to enable Intel GVT-g graphics
+	  virtualization technology host support with integrated graphics.
+	  With GVT-g, it's possible to have one integrated graphics
+	  device shared by multiple VMs under different hypervisors.
+
+	  Note that at least one hypervisor like Xen or KVM is required for
+	  this driver to work, and it only supports newer device from
+	  Broadwell+. For further information and setup guide, you can
+	  visit: http://01.org/zh/igvt-g.
+
+	  The first version is still preliminary. Use it under you own risk.
+
+	  If in doubt, say "N".
+
 menu "drm/i915 Debugging"
 depends on DRM_I915
 depends on EXPERT
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 7e29444..276abf1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -104,6 +104,11 @@ i915-y += i915_vgpu.o
 # legacy horrors
 i915-y += i915_dma.o
 
+ifeq ($(CONFIG_DRM_I915_GVT),y)
+i915-y += intel_gvt.o
+include $(src)/gvt/Makefile
+endif
+
 obj-$(CONFIG_DRM_I915)  += i915.o
 
 CFLAGS_i915_trace_points.o := -I$(src)
diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
new file mode 100644
index 0000000..d0f21a6
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -0,0 +1,5 @@
+GVT_DIR := gvt
+GVT_SOURCE := gvt.o
+
+ccflags-y                      += -I$(src) -I$(src)/$(GVT_DIR) -Wall
+i915-y			       += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
new file mode 100644
index 0000000..7ef412b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/debug.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef __GVT_DEBUG_H__
+#define __GVT_DEBUG_H__
+
+#define gvt_dbg_core(fmt, args...) \
+	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
+
+/*
+ * Other GVT debug stuff will be introduced in the GVT device model patches.
+ */
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
new file mode 100644
index 0000000..9f1d3e7
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -0,0 +1,181 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/types.h>
+#include <xen/xen.h>
+#include <linux/kthread.h>
+
+#include "gvt.h"
+
+struct intel_gvt_host intel_gvt_host;
+
+static const char * const supported_hypervisors[] = {
+	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
+	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
+};
+
+#define MB(x) (x * 1024ULL * 1024ULL)
+#define GB(x) (x * MB(1024))
+
+/* Load MPT modules and detect if we're running in host */
+static int init_gvt_host(void)
+{
+	if (intel_gvt_host.initialized)
+		return 0;
+
+	/* Xen DOM U */
+	if (xen_domain() && !xen_initial_domain())
+		return -ENODEV;
+
+	/* Try to load MPT modules for hypervisors */
+	if (xen_initial_domain()) {
+		/* In Xen dom0 */
+		intel_gvt_host.mpt = try_then_request_module(
+				symbol_get(xengt_mpt), "xengt");
+		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_XEN;
+	} else {
+		/* not in Xen. Try KVMGT */
+		intel_gvt_host.mpt = try_then_request_module(
+				symbol_get(kvmgt_mpt), "kvm");
+		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_KVM;
+	}
+
+	/* Fail to load MPT modules - bail out */
+	if (!intel_gvt_host.mpt)
+		return -EINVAL;
+
+	/* Try to detect if we're running in host instead of VM. */
+	if (!intel_gvt_hypervisor_detect_host())
+		return -ENODEV;
+
+	gvt_dbg_core("Running with hypervisor %s in host mode\n",
+			supported_hypervisors[intel_gvt_host.hypervisor_type]);
+
+	idr_init(&intel_gvt_host.gvt_idr);
+	mutex_init(&intel_gvt_host.gvt_idr_lock);
+	intel_gvt_host.initialized = true;
+	return 0;
+}
+
+static void init_device_info(struct intel_gvt *gvt)
+{
+	if (IS_BROADWELL(gvt->dev_priv))
+		gvt->device_info.max_support_vgpus = 8;
+	/* This function will grow large in GVT device model patches. */
+}
+
+static void free_gvt_device(struct intel_gvt *gvt)
+{
+	mutex_lock(&intel_gvt_host.gvt_idr_lock);
+	idr_remove(&intel_gvt_host.gvt_idr, gvt->id);
+	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
+
+	vfree(gvt);
+}
+
+static struct intel_gvt *alloc_gvt_device(struct drm_i915_private *dev_priv)
+{
+	struct intel_gvt *gvt;
+	int ret;
+
+	/*
+	 * This data structure will grow large in future, use vzalloc() at
+	 * the beginning.
+	 */
+	gvt = vzalloc(sizeof(*gvt));
+	if (!gvt)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_lock(&intel_gvt_host.gvt_idr_lock);
+	ret = idr_alloc(&intel_gvt_host.gvt_idr, gvt, 0, 0, GFP_KERNEL);
+	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
+
+	if (ret < 0)
+		goto err;
+
+	gvt->id = ret;
+	mutex_init(&gvt->lock);
+	gvt->dev_priv = dev_priv;
+	idr_init(&gvt->vgpu_idr);
+
+	return gvt;
+err:
+	free_gvt_device(gvt);
+	return ERR_PTR(ret);
+}
+
+/**
+ * intel_gvt_destroy_device - destroy a GVT device
+ * @gvt: intel gvt device
+ *
+ * This function is called at the driver unloading stage, to destroy a
+ * GVT device and free the related resources.
+ *
+ */
+void intel_gvt_destroy_device(struct intel_gvt *gvt)
+{
+	/* Another de-initialization of GVT components will be introduced. */
+	free_gvt_device(gvt);
+}
+
+/**
+ * intel_gvt_create_device - create a GVT device
+ * @dev_priv: drm i915 private data
+ *
+ * This function is called at the initialization stage, to create a
+ * GVT device and initialize necessary GVT components for it.
+ *
+ * Returns:
+ * pointer to the intel gvt device structure, error pointer if failed.
+ *
+ */
+struct intel_gvt *intel_gvt_create_device(struct drm_i915_private *dev_priv)
+{
+	struct intel_gvt *gvt;
+	int ret;
+
+	ret = init_gvt_host();
+	if (ret)
+		return ERR_PTR(ret);
+
+	gvt_dbg_core("create new gvt device\n");
+
+	gvt = alloc_gvt_device(dev_priv);
+	if (IS_ERR(gvt)) {
+		ret = PTR_ERR(gvt);
+		goto out_err;
+	}
+
+	gvt_dbg_core("init gvt device, id %d\n", gvt->id);
+
+	init_device_info(gvt);
+	/*
+	 * Other initialization of GVT components will be called here.
+	 */
+	gvt_dbg_core("gvt device creation is done, id %d\n", gvt->id);
+
+	return gvt;
+
+out_err:
+	return ERR_PTR(ret);
+}
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
new file mode 100644
index 0000000..f021487
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -0,0 +1,75 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _GVT_H_
+#define _GVT_H_
+
+#include "i915_drv.h"
+#include "i915_pvinfo.h"
+
+#include "debug.h"
+#include "hypercall.h"
+
+#define GVT_MAX_VGPU 8
+#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) - 1)))
+
+enum {
+	INTEL_GVT_HYPERVISOR_XEN = 0,
+	INTEL_GVT_HYPERVISOR_KVM,
+};
+
+struct intel_gvt_host {
+	bool initialized;
+	int hypervisor_type;
+	struct mutex gvt_idr_lock;
+	struct idr gvt_idr;
+	struct intel_gvt_mpt *mpt;
+};
+
+extern struct intel_gvt_host intel_gvt_host;
+
+/* Describe per-platform limitations. */
+struct intel_gvt_device_info {
+	u32 max_support_vgpus;
+	/* This data structure will grow bigger in GVT device model patches */
+};
+
+struct intel_vgpu {
+	struct intel_gvt *gvt;
+	int id;
+	unsigned long handle; /* vGPU handle used by hypervisor MPT modules */
+};
+
+struct intel_gvt {
+	struct mutex lock;
+	int id;
+
+	struct drm_i915_private *dev_priv;
+	struct idr vgpu_idr;	/* vGPU IDR pool */
+
+	struct intel_gvt_device_info device_info;
+};
+
+#include "mpt.h"
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
new file mode 100644
index 0000000..254df8b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _GVT_HYPERCALL_H_
+#define _GVT_HYPERCALL_H_
+
+/*
+ * Specific GVT-g MPT modules function collections. Currently GVT-g supports
+ * both Xen and KVM by providing dedicated hypervisor-related MPT modules.
+ */
+struct intel_gvt_mpt {
+	int (*detect_host)(void);
+};
+
+extern struct intel_gvt_mpt xengt_mpt;
+extern struct intel_gvt_mpt kvmgt_mpt;
+
+#endif /* _GVT_HYPERCALL_H_ */
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
new file mode 100644
index 0000000..783f4f8
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _GVT_MPT_H_
+#define _GVT_MPT_H_
+
+/**
+ * DOC: Hypervisor Service APIs for GVT-g Core Logic
+ *
+ * This is the glue layer between specific hypervisor MPT modules and GVT-g core
+ * logic. Each kind of hypervisor MPT module provides a collection of function
+ * callbacks via gvt_kernel_dm and will be attached to GVT host when driver
+ * loading. GVT-g core logic will call these APIs to request specific services
+ * from hypervisor.
+ */
+
+/**
+ * intel_gvt_hypervisor_detect_host - check if GVT-g is running within
+ * hypervisor host/privilged domain
+ *
+ * Returns:
+ * Zero on success, -ENODEV if current kernel is running inside a VM
+ */
+static inline int intel_gvt_hypervisor_detect_host(void)
+{
+	return intel_gvt_host.mpt->detect_host();
+}
+
+#endif /* _GVT_MPT_H_ */
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 07edaed..0baf64a 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -35,6 +35,7 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "intel_gvt.h"
 #include "i915_vgpu.h"
 #include "i915_trace.h"
 #include <linux/pci.h>
@@ -1244,18 +1245,22 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		goto out_ggtt;
 	}
 
+	ret = intel_gvt_init(dev_priv);
+	if (ret)
+		goto out_ggtt;
+
 	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
 	 * otherwise the vga fbdev driver falls over. */
 	ret = i915_kick_out_firmware_fb(dev_priv);
 	if (ret) {
 		DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	ret = i915_kick_out_vgacon(dev_priv);
 	if (ret) {
 		DRM_ERROR("failed to remove conflicting VGA console\n");
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	pci_set_master(dev->pdev);
@@ -1266,7 +1271,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		if (ret) {
 			DRM_ERROR("failed to set DMA mask\n");
 
-			goto out_ggtt;
+			goto out_gvt;
 		}
 	}
 
@@ -1296,7 +1301,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 				     aperture_size);
 	if (!ggtt->mappable) {
 		ret = -EIO;
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	ggtt->mtrr = arch_phys_wc_add(ggtt->mappable_base,
@@ -1329,6 +1334,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 
 	return 0;
 
+out_gvt:
+	intel_gvt_cleanup(dev_priv);
 out_ggtt:
 	i915_ggtt_cleanup_hw(dev);
 
@@ -1487,6 +1494,8 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_fbdev_fini(dev);
 
+	intel_gvt_cleanup(dev_priv);
+
 	ret = i915_gem_suspend(dev);
 	if (ret) {
 		DRM_ERROR("failed to idle hardware: %d\n", ret);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e4c8e34..bdf499f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -61,6 +61,8 @@
 #include "i915_gem_gtt.h"
 #include "i915_gem_render_state.h"
 
+#include "intel_gvt.h"
+
 /* General customization:
  */
 
@@ -1737,6 +1739,8 @@ struct drm_i915_private {
 
 	struct i915_virtual_gpu vgpu;
 
+	struct intel_gvt *gvt;
+
 	struct intel_guc guc;
 
 	struct intel_csr csr;
@@ -2938,6 +2942,12 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
 u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
 
 void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
+
+static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
+{
+	return dev_priv->gvt ? true : false;
+}
+
 static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
 {
 	return dev_priv->vgpu.active;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 5e18cf9..c9615ce 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -60,6 +60,7 @@ struct i915_params i915 __read_mostly = {
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
 	.enable_dpcd_backlight = false,
+	.enable_gvt = false,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -222,3 +223,7 @@ MODULE_PARM_DESC(inject_load_failure,
 module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
 MODULE_PARM_DESC(enable_dpcd_backlight,
 	"Enable support for DPCD backlight control (default:false)");
+
+module_param_named(enable_gvt, i915.enable_gvt, bool, 0600);
+MODULE_PARM_DESC(enable_gvt,
+	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 1323261..0ad020b 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -63,6 +63,7 @@ struct i915_params {
 	bool nuclear_pageflip;
 	bool enable_dp_mst;
 	bool enable_dpcd_backlight;
+	bool enable_gvt;
 };
 
 extern struct i915_params i915 __read_mostly;
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
new file mode 100644
index 0000000..8556f58
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "i915_drv.h"
+#include "intel_gvt.h"
+
+/**
+ * DOC: Intel GVT-g host support
+ *
+ * Intel GVT-g is a graphics virtualization technology which shares the
+ * GPU among multiple virtual machines on a time-sharing basis. Each
+ * virtual machine is presented a virtual GPU (vGPU), which has equivalent
+ * features as the underlying physical GPU (pGPU), so i915 driver can run
+ * seamlessly in a virtual machine. This file provides the englightments
+ * of GVT and the necessary components used by GVT in i915 driver.
+ */
+
+static bool is_supported_device(struct drm_i915_private *dev_priv)
+{
+	if (IS_BROADWELL(dev_priv))
+		return true;
+	return false;
+}
+
+/**
+ * intel_gvt_init - initialize GVT components
+ * @dev_priv: drm i915 private data
+ *
+ * This function is called at the initialization stage to create a GVT device.
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ *
+ */
+int intel_gvt_init(struct drm_i915_private *dev_priv)
+{
+	if (!i915.enable_gvt) {
+		DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
+		return 0;
+	}
+
+	if (!is_supported_device(dev_priv)) {
+		DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
+		return 0;
+	}
+
+	dev_priv->gvt = intel_gvt_create_device(dev_priv);
+	if (IS_ERR(dev_priv->gvt)) {
+		DRM_DEBUG_DRIVER("GVT-g is disabled\n");
+		dev_priv->gvt = NULL;
+		return 0;
+	}
+	return 0;
+}
+
+/**
+ * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
+ * @dev_priv: drm i915 private *
+ *
+ * This function is called at the i915 driver unloading stage, to shutdown
+ * GVT components and release the related resources.
+ */
+void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
+{
+	if (!intel_gvt_active(dev_priv))
+		return;
+
+	intel_gvt_destroy_device(dev_priv->gvt);
+	dev_priv->gvt = NULL;
+}
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
new file mode 100644
index 0000000..b9b361b
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _INTEL_GVT_H_
+#define _INTEL_GVT_H_
+
+#ifdef CONFIG_DRM_I915_GVT
+#include "gvt/gvt.h"
+extern int intel_gvt_init(struct drm_i915_private *dev_priv);
+extern void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
+extern struct intel_gvt *intel_gvt_create_device(
+		struct drm_i915_private *dev_priv);
+extern void intel_gvt_destroy_device(struct intel_gvt *gvt);
+#else
+struct intel_gvt {
+};
+static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+static inline void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
+{
+}
+#endif
+
+#endif /* _INTEL_GVT_H_ */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 4/9] drm/i915: Introduce host graphics memory partition for GVT-g
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
                   ` (2 preceding siblings ...)
  2016-06-02 16:36 ` [PATCH v6 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  9:17   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 5/9] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

From: Bing Niu <bing.niu@intel.com>

This patch introduces host graphics memory partition when GVT-g
is enabled.

Under GVT-g, i915 host driver only owned limited graphics resources,
others are managed by GVT-g resource allocator and kept for other vGPUs.

v6:

- Remove kernel parameters used to configure GGTT owned by host. (Chris)
- Other coding style comments from Chris.
- Add more comments for reviewer.

v3:

- Remove fence partition, will use i915 fence stealing in future.(Kevin)
- Santinize GVT host gm kernel parameters. (Joonas)

v2:
- Address all coding-style comments from Joonas previously.
- Fix errors and warnning reported by checkpatch.pl. (Joonas)
- Move the graphs into the header files. (Daniel)

Signed-off-by: Bing Niu <bing.niu@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_vgpu.c | 23 +++++++++++++++++------
 drivers/gpu/drm/i915/intel_gvt.h | 25 +++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 5312816..0d891a3 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -189,14 +189,25 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 	unsigned long unmappable_base, unmappable_size, unmappable_end;
 	int ret;
 
-	if (!intel_vgpu_active(dev_priv))
+	if (intel_gvt_active(dev_priv)) {
+		/* Retrieve GGTT partition information from macros */
+		mappable_base = 0;
+		mappable_size = INTEL_GVT_HOST_LOW_GM_SIZE;
+		unmappable_base = dev_priv->ggtt.mappable_end;
+		unmappable_size = INTEL_GVT_HOST_HIGH_GM_SIZE;
+	} else if (intel_vgpu_active(dev_priv)) {
+		/* Retrieve GGTT partition information from PVINFO */
+		mappable_base = I915_READ(
+				vgtif_reg(avail_rs.mappable_gmadr.base));
+		mappable_size = I915_READ(
+				vgtif_reg(avail_rs.mappable_gmadr.size));
+		unmappable_base = I915_READ(
+				vgtif_reg(avail_rs.nonmappable_gmadr.base));
+		unmappable_size = I915_READ(
+				vgtif_reg(avail_rs.nonmappable_gmadr.size));
+	} else
 		return 0;
 
-	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
-	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
-	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
-	unmappable_size = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size));
-
 	mappable_end = mappable_base + mappable_size;
 	unmappable_end = unmappable_base + unmappable_size;
 
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index b9b361b..f0b9aac 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -24,6 +24,31 @@
 #ifndef _INTEL_GVT_H_
 #define _INTEL_GVT_H_
 
+/*
+ * Under GVT-g, i915 host driver only owned limited graphics resources,
+ * others are managed by GVT-g resource allocator and kept for other vGPUs.
+ *
+ * For graphics memory space partition, a typical layout looks like:
+ *
+ * +-------+-----------------------+------+-----------------------+
+ * |* Host |   *GVT-g Resource     |* Host|   *GVT-g Resource     |
+ * | Owned |   Allocator Managed   | Owned|   Allocator Managed   |
+ * |       |                       |      |                       |
+ * +---------------+-------+----------------------+-------+-------+
+ * |       |       |       |       |      |       |       |       |
+ * | i915  | vm 1  | vm 2  | vm 3  | i915 | vm 1  | vm 2  | vm 3  |
+ * |       |       |       |       |      |       |       |       |
+ * +-------+-------+-------+--------------+-------+-------+-------+
+ * |           Aperture            |            Hidden            |
+ * +-------------------------------+------------------------------+
+ * |                       GGTT memory space                      |
+ * +--------------------------------------------------------------+
+ */
+
+/* GGTT memory space owned by host */
+#define INTEL_GVT_HOST_LOW_GM_SIZE (96 * 1024 * 1024)
+#define INTEL_GVT_HOST_HIGH_GM_SIZE (384 * 1024 * 1024)
+
 #ifdef CONFIG_DRM_I915_GVT
 #include "gvt/gvt.h"
 extern int intel_gvt_init(struct drm_i915_private *dev_priv);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 5/9] drm/i915: Make ring buffer size of a LRC context configurable
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
                   ` (3 preceding siblings ...)
  2016-06-02 16:36 ` [PATCH v6 4/9] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  9:20   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 6/9] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

This patch introduces an option for configuring the ring buffer size
of a LRC context after the context creation.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 1 +
 drivers/gpu/drm/i915/i915_gem_context.c | 1 +
 drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdf499f..a4af035 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -878,6 +878,7 @@ struct i915_gem_context {
 		int pin_count;
 		bool initialised;
 	} engine[I915_NUM_ENGINES];
+	u32 lrc_ring_buffer_size;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a3b11aa..1663981 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -295,6 +295,7 @@ __create_hw_context(struct drm_device *dev,
 	ctx->remap_slice = ALL_L3_SLICES(dev_priv);
 
 	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
+	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
 
 	return ctx;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5c191a1..72a0cca 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2484,7 +2484,8 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 		return PTR_ERR(ctx_obj);
 	}
 
-	ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE);
+	ringbuf = intel_engine_create_ringbuffer(engine,
+			ctx->lrc_ring_buffer_size);
 	if (IS_ERR(ringbuf)) {
 		ret = PTR_ERR(ringbuf);
 		goto error_deref_obj;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 6/9] drm/i915: Make addressing mode bits in context descriptor configurable
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
                   ` (4 preceding siblings ...)
  2016-06-02 16:36 ` [PATCH v6 5/9] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  9:25   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification Zhi Wang
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

Currently the addressing mode bit in context descriptor is statically
generated from the configuration of system-wide PPGTT usage model.

GVT-g will load the PPGTT shadow page table by itself and probably one
guest is using a different addressing mode with i915 host. The addressing
mode bits of a LRC context should be configurable under this case.

v6:

- Directly save the addressing mode bits inside i915_gem_context. (Chris)
- Move the LRC context addressing mode bits into intel_lrc.h. (Chris)

v5:

- Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_gem_context.c |  2 ++
 drivers/gpu/drm/i915/intel_lrc.c        | 13 +------------
 drivers/gpu/drm/i915/intel_lrc.h        | 11 +++++++++++
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a4af035..f6cb60a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -879,6 +879,7 @@ struct i915_gem_context {
 		bool initialised;
 	} engine[I915_NUM_ENGINES];
 	u32 lrc_ring_buffer_size;
+	u32 lrc_addressing_mode_bits;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 1663981..d9d7779 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -296,6 +296,8 @@ __create_hw_context(struct drm_device *dev,
 
 	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
 	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
+	ctx->lrc_addressing_mode_bits = GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
+		GEN8_CTX_ADDRESSING_MODE_SHIFT;
 
 	return ctx;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 72a0cca..ffb436c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -208,16 +208,6 @@
 } while (0)
 
 enum {
-	ADVANCED_CONTEXT = 0,
-	LEGACY_32B_CONTEXT,
-	ADVANCED_AD_CONTEXT,
-	LEGACY_64B_CONTEXT
-};
-#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
-#define GEN8_CTX_ADDRESSING_MODE(dev)  (USES_FULL_48BIT_PPGTT(dev) ?\
-		LEGACY_64B_CONTEXT :\
-		LEGACY_32B_CONTEXT)
-enum {
 	FAULT_AND_HANG = 0,
 	FAULT_AND_HALT, /* Debug only */
 	FAULT_AND_STREAM,
@@ -281,8 +271,6 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
 					(engine->id == VCS || engine->id == VCS2);
 
 	engine->ctx_desc_template = GEN8_CTX_VALID;
-	engine->ctx_desc_template |= GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
-				   GEN8_CTX_ADDRESSING_MODE_SHIFT;
 	if (IS_GEN8(dev_priv))
 		engine->ctx_desc_template |= GEN8_CTX_L3LLC_COHERENT;
 	engine->ctx_desc_template |= GEN8_CTX_PRIVILEGE;
@@ -326,6 +314,7 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
 	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
 
 	desc = engine->ctx_desc_template;			/* bits  0-11 */
+	desc |= ctx->lrc_addressing_mode_bits;			/* bits  3-4  */
 	desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
 								/* bits 12-31 */
 	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;		/* bits 32-52 */
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index a8db42a..e133c33 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -28,6 +28,17 @@
 
 #define GEN8_LR_CONTEXT_ALIGN 4096
 
+enum {
+	ADVANCED_CONTEXT = 0,
+	LEGACY_32B_CONTEXT,
+	ADVANCED_AD_CONTEXT,
+	LEGACY_64B_CONTEXT
+};
+#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
+#define GEN8_CTX_ADDRESSING_MODE(dev_priv) (USES_FULL_48BIT_PPGTT(dev_priv) ?\
+		LEGACY_64B_CONTEXT : \
+		LEGACY_32B_CONTEXT)
+
 /* Execlists regs */
 #define RING_ELSP(ring)				_MMIO((ring)->mmio_base + 0x230)
 #define RING_EXECLIST_STATUS_LO(ring)		_MMIO((ring)->mmio_base + 0x234)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
                   ` (5 preceding siblings ...)
  2016-06-02 16:36 ` [PATCH v6 6/9] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  9:40   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 8/9] drm/i915: Support LRC context single submission Zhi Wang
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

This patch introduces an approach to track the execlist context status
change.

GVT-g uses GVT context as the "shadow context". The content inside GVT
context will be copied back to guest after the context is idle. So GVT-g
has to know the status of the execlist context.

This function is configurable in the context creation service. Currently,
Only GVT-g will create the "status-change-notification" enabled GEM
context.

v6:

- When !CONFIG_DRM_I915_GVT, make GVT code as dead code then compiler
could automatically eliminate them for us. (Chris)
- Always initialize the notifier header, so it could be switched on/off
at runtime. (Chris)

v5:

- Only compile this feature when CONFIG_DRM_I915_GVT is enabled.(Tvrtko)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/intel_lrc.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.h |  5 +++++
 3 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f6cb60a..dee72d3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -877,9 +877,11 @@ struct i915_gem_context {
 		u64 lrc_desc;
 		int pin_count;
 		bool initialised;
+		struct atomic_notifier_head status_notifier;
 	} engine[I915_NUM_ENGINES];
 	u32 lrc_ring_buffer_size;
 	u32 lrc_addressing_mode_bits;
+	bool enable_lrc_status_change_notification;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ffb436c..96d20c8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -404,6 +404,22 @@ static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
 	spin_unlock_irq(&dev_priv->uncore.lock);
 }
 
+static inline void execlists_context_status_change(
+		struct drm_i915_gem_request *rq,
+		unsigned long status)
+{
+	/* The compiler should be fine with the dead-code elimination */
+	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
+		return;
+
+	if (!rq->ctx->enable_lrc_status_change_notification)
+		return;
+
+	atomic_notifier_call_chain(
+			&rq->ctx->engine[rq->engine->id].status_notifier,
+			status, rq);
+}
+
 static void execlists_context_unqueue(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
@@ -439,6 +455,11 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
 	if (unlikely(!req0))
 		return;
 
+	execlists_context_status_change(req0, CONTEXT_SCHEDULE_IN);
+
+	if (req1)
+		execlists_context_status_change(req1, CONTEXT_SCHEDULE_IN);
+
 	if (req0->elsp_submitted & engine->idle_lite_restore_wa) {
 		/*
 		 * WaIdleLiteRestore: make sure we never cause a lite restore
@@ -477,6 +498,8 @@ execlists_check_remove_request(struct intel_engine_cs *engine, u32 ctx_id)
 	if (--head_req->elsp_submitted > 0)
 		return 0;
 
+	execlists_context_status_change(head_req, CONTEXT_SCHEDULE_OUT);
+
 	list_del(&head_req->execlist_link);
 	i915_gem_request_unreference(head_req);
 
@@ -2489,6 +2512,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 	ce->ringbuf = ringbuf;
 	ce->state = ctx_obj;
 	ce->initialised = engine->init_context == NULL;
+	ATOMIC_INIT_NOTIFIER_HEAD(&ce->status_notifier);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index e133c33..7a7ae8d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -68,6 +68,11 @@ enum {
 #define GEN8_CSB_READ_PTR(csb_status) \
 	(((csb_status) & GEN8_CSB_READ_PTR_MASK) >> 8)
 
+enum {
+	CONTEXT_SCHEDULE_IN = 0,
+	CONTEXT_SCHEDULE_OUT,
+};
+
 /* Logical Rings */
 int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
 int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 8/9] drm/i915: Support LRC context single submission
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
                   ` (6 preceding siblings ...)
  2016-06-02 16:36 ` [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  9:47   ` Joonas Lahtinen
  2016-06-02 16:36 ` [PATCH v6 9/9] drm/i915: Introduce GVT context creation API Zhi Wang
  2016-06-03  6:43 ` ✗ Ro.CI.BAT: warning for Introduce the implementation of GVT context (rev4) Patchwork
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

This patch introduces the support of LRC context signle submission.
As GVT context may come from different guests, which requires different
configuration of render registers. It can't be combined into a dual ELSP
submission combo.

Only GVT-g will create this kinds of GEM context currently.

v6:
- Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)

v5:

- Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dee72d3..92d01e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -882,6 +882,7 @@ struct i915_gem_context {
 	u32 lrc_ring_buffer_size;
 	u32 lrc_addressing_mode_bits;
 	bool enable_lrc_status_change_notification;
+	bool enable_lrc_single_submission;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 96d20c8..ce707ea 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -446,6 +446,21 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
 			i915_gem_request_unreference(req0);
 			req0 = cursor;
 		} else {
+			/* Compiler will do the dead-code elimination */
+			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
+				/*
+				 * req0 (after merged) ctx requires single
+				 * submission, stop picking
+				 */
+				if (req0->ctx->enable_lrc_single_submission)
+					break;
+				/*
+				 * req0 ctx doesn't require single submission,
+				 * but next req ctx requires, stop picking
+				 */
+				if (cursor->ctx->enable_lrc_single_submission)
+					break;
+			}
 			req1 = cursor;
 			WARN_ON(req1->elsp_submitted);
 			break;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v6 9/9] drm/i915: Introduce GVT context creation API
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
                   ` (7 preceding siblings ...)
  2016-06-02 16:36 ` [PATCH v6 8/9] drm/i915: Support LRC context single submission Zhi Wang
@ 2016-06-02 16:36 ` Zhi Wang
  2016-06-03  9:59   ` Joonas Lahtinen
  2016-06-03  6:43 ` ✗ Ro.CI.BAT: warning for Introduce the implementation of GVT context (rev4) Patchwork
  9 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2016-06-02 16:36 UTC (permalink / raw)
  To: intel-gfx, tvrtko.ursulin, joonas.lahtinen, kevin.tian,
	zhiyuan.lv, chris

GVT workload scheduler needs special host LRC contexts, the so called
"shadow LRC context" to submit guest workload to host i915. During the
guest workload submission, GVT fills the shadow LRC context with the
content of guest LRC context: engine context is copied without changes,
ring context is mostly owned by host i915.

The GVT-g workload scheduler flow:

         +-----------+                   +-----------+
         | GVT Guest |                   | GVT Guest |
         +-+-----^---+                   +-+-----^---+
           |     |                         |     |
           |     | GVT-g                   |     | GVT-g
vELSP write|     | emulates     vELSP write|     | emulates
           |     | Execlist/CSB            |     | Execlist/CSB
           |     | Status                  |     | Status
           |     |                         |     |
    +------v-----+-------------------------v-----+---------+
    |           GVT Virtual Execlist Submission            |
    +------+-------------------------------+---------------+
           |                               |
           | Per-VM/Ring Workoad Q         | Per-VM/Ring Workload Q
   +---------------------+--+      +------------------------+
       +---v--------+    ^             +---v--------+
       |GVT Workload|... |             |GVT Workload|...
       +------------+    |             +------------+
                         |
                         | Pick Workload from Q
    +--------------------+---------------------------------+
    |                GVT Workload Scheduler                |
    +--------------------+---------------------------------+
                         |         * Shadow guest LRC context
                  +------v------+  * Shadow guest ring buffer
                  | GVT Context |  * Scan/Patch guest RB instructions
                  +------+------+
                         |
                         v
              Host i915 GEM Submission

v6:

- Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)

v5:
- Only compile this feature when CONFIG_DRM_I915_GVT is enabled. (Tvrtko)
- Rebase the code into new repo.
- Add a comment about the ring buffer size. (Joonas)

v2:

Mostly based on Daniel's idea. Call the refactored core logic of GEM
context creation service and LRC context creation service to create the GVT
context.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index d9d7779..c0259d7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -342,6 +342,38 @@ i915_gem_create_context(struct drm_device *dev,
 	return ctx;
 }
 
+/**
+ * i915_gem_create_gvt_context - create a GVT GEM context
+ * @dev: drm device *
+ *
+ * This function is used to create a GVT specific GEM context.
+ *
+ * Returns:
+ * pointer to i915_gem_context on success, error pointer if failed
+ *
+ */
+struct i915_gem_context *
+i915_gem_create_gvt_context(struct drm_device *dev)
+{
+	struct i915_gem_context *ctx;
+
+	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
+		return ERR_PTR(-ENODEV);
+
+	mutex_lock(&dev->struct_mutex);
+
+	ctx = i915_gem_create_context(dev, NULL);
+	if (IS_ERR(ctx))
+		goto out;
+
+	ctx->enable_lrc_status_change_notification = true;
+	ctx->enable_lrc_single_submission = true;
+	ctx->lrc_ring_buffer_size = 512 * PAGE_SIZE; /* Max ring buffer size */
+out:
+	mutex_unlock(&dev->struct_mutex);
+	return ctx;
+}
+
 static void i915_gem_context_unpin(struct i915_gem_context *ctx,
 				   struct intel_engine_cs *engine)
 {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* ✗ Ro.CI.BAT: warning for Introduce the implementation of GVT context (rev4)
  2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
                   ` (8 preceding siblings ...)
  2016-06-02 16:36 ` [PATCH v6 9/9] drm/i915: Introduce GVT context creation API Zhi Wang
@ 2016-06-03  6:43 ` Patchwork
  9 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2016-06-03  6:43 UTC (permalink / raw)
  To: Wang, Zhi A; +Cc: intel-gfx

== Series Details ==

Series: Introduce the implementation of GVT context (rev4)
URL   : https://patchwork.freedesktop.org/series/7208/
State : warning

== Summary ==

Series 7208v4 Introduce the implementation of GVT context
http://patchwork.freedesktop.org/api/1.0/series/7208/revisions/4/mbox

Test drv_module_reload_basic:
                pass       -> SKIP       (ro-skl-i7-6700hq)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-wb-pro-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-wb-ro-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_mmap_gtt:
        Subgroup basic-write-cpu-read-gtt:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-write-gtt:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_storedw_loop:
        Subgroup basic-bsd:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-render:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-vebox:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test kms_addfb_basic:
        Subgroup bad-pitch-0:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup bo-too-small:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup tile-pitch-mismatch:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)

fi-hsw-i7-4770k  total:209  pass:187  dwarn:0   dfail:0   fail:3   skip:19 
fi-snb-i7-2600   total:209  pass:167  dwarn:0   dfail:0   fail:3   skip:39 
ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26 
ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0   skip:19 
ro-ilk-i7-620lm  total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26 
ro-ivb2-i7-3770  total:102  pass:79   dwarn:0   dfail:0   fail:0   skip:22 
ro-skl-i7-6700hq total:204  pass:181  dwarn:1   dfail:0   fail:0   skip:22 
ro-snb-i7-2620M  total:102  pass:72   dwarn:0   dfail:0   fail:0   skip:29 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1085/

cbc3c4a drm-intel-nightly: 2016y-06m-02d-22h-03m-59s UTC integration manifest
9413fff drm/i915: Introduce GVT context creation API
b4f4f3d drm/i915: Support LRC context single submission
a476548 drm/i915: Introduce execlist context status change notification
28b78a8 drm/i915: Make addressing mode bits in context descriptor configurable
b810d7a drm/i915: Make ring buffer size of a LRC context configurable
a1d8abe drm/i915: Introduce host graphics memory partition for GVT-g
abb5464 drm/i915: gvt: Introduce the basic architecture of GVT-g
9d04099 drm/i915: Fold vGPU active check into inner functions
fcf109b drm/i915: Factor out i915_pvinfo.h

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 1/9] drm/i915: Factor out i915_pvinfo.h
  2016-06-02 16:36 ` [PATCH v6 1/9] drm/i915: Factor out i915_pvinfo.h Zhi Wang
@ 2016-06-03  8:45   ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  8:45 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> As the PVINFO page definition is used by both GVT-g guest (vGPU) and GVT-g
> host (GVT-g kernel device model), factor it out for better code structure.
> 
> v3:
> 
> Take Joonas's comments:
> - Use offsetof to calculate the member offset of PVINFO structure
> 

Split this patch in two; one to do the move, then other to convert to
offsetof and with that, both patches;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pvinfo.h | 116 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_vgpu.h   |  86 +--------------------------
>  2 files changed, 117 insertions(+), 85 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_pvinfo.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
> new file mode 100644
> index 0000000..eb45afb
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -0,0 +1,116 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _I915_PVINFO_H_
> +#define _I915_PVINFO_H_
> +
> +/* The MMIO offset of the shared info between guest and host emulator */
> +#define VGT_PVINFO_PAGE	0x78000
> +#define VGT_PVINFO_SIZE	0x1000
> +
> +/*
> + * The following structure pages are defined in GEN MMIO space
> + * for virtualization. (One page for now)
> + */
> +#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
> +#define VGT_VERSION_MAJOR 1
> +#define VGT_VERSION_MINOR 0
> +
> +#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
> +#define INTEL_VGT_IF_VERSION \
> +	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
> +
> +/*
> + * notifications from guest to vgpu device model
> + */
> +enum vgt_g2v_type {
> +	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> +	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> +	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> +	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> +	VGT_G2V_EXECLIST_CONTEXT_CREATE,
> +	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> +	VGT_G2V_MAX,
> +};
> +
> +struct vgt_if {
> +	uint64_t magic;		/* VGT_MAGIC */
> +	uint16_t version_major;
> +	uint16_t version_minor;
> +	uint32_t vgt_id;	/* ID of vGT instance */
> +	uint32_t rsv1[12];	/* pad to offset 0x40 */
> +	/*
> +	 *  Data structure to describe the balooning info of resources.
> +	 *  Each VM can only have one portion of continuous area for now.
> +	 *  (May support scattered resource in future)
> +	 *  (starting from offset 0x40)
> +	 */
> +	struct {
> +		/* Aperture register balooning */
> +		struct {
> +			uint32_t base;
> +			uint32_t size;
> +		} mappable_gmadr;	/* aperture */
> +		/* GMADR register balooning */
> +		struct {
> +			uint32_t base;
> +			uint32_t size;
> +		} nonmappable_gmadr;	/* non aperture */
> +		/* allowed fence registers */
> +		uint32_t fence_num;
> +		uint32_t rsv2[3];
> +	} avail_rs;		/* available/assigned resource */
> +	uint32_t rsv3[0x200 - 24];	/* pad to half page */
> +	/*
> +	 * The bottom half page is for response from Gfx driver to hypervisor.
> +	 */
> +	uint32_t rsv4;
> +	uint32_t display_ready;	/* ready for display owner switch */
> +
> +	uint32_t rsv5[4];
> +
> +	uint32_t g2v_notify;
> +	uint32_t rsv6[7];
> +
> +	struct {
> +		uint32_t lo;
> +		uint32_t hi;
> +	} pdp[4];
> +
> +	uint32_t execlist_context_descriptor_lo;
> +	uint32_t execlist_context_descriptor_hi;
> +
> +	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
> +} __packed;
> +
> +#define _vgtif_reg(x) \
> +	(VGT_PVINFO_PAGE + offsetof(struct vgt_if, x))
> +
> +#define vgtif_reg(x) \
> +	_MMIO(_vgtif_reg(x))
> +
> +/* vGPU display status to be used by the host side */
> +#define VGT_DRV_DISPLAY_NOT_READY 0
> +#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
> +
> +#endif /* _I915_PVINFO_H_ */
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index 21ffcfe..07e67d5 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -24,91 +24,7 @@
>  #ifndef _I915_VGPU_H_
>  #define _I915_VGPU_H_
>  
> -/* The MMIO offset of the shared info between guest and host emulator */
> -#define VGT_PVINFO_PAGE	0x78000
> -#define VGT_PVINFO_SIZE	0x1000
> -
> -/*
> - * The following structure pages are defined in GEN MMIO space
> - * for virtualization. (One page for now)
> - */
> -#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
> -#define VGT_VERSION_MAJOR 1
> -#define VGT_VERSION_MINOR 0
> -
> -#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
> -#define INTEL_VGT_IF_VERSION \
> -	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
> -
> -/*
> - * notifications from guest to vgpu device model
> - */
> -enum vgt_g2v_type {
> -	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> -	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> -	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> -	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> -	VGT_G2V_EXECLIST_CONTEXT_CREATE,
> -	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> -	VGT_G2V_MAX,
> -};
> -
> -struct vgt_if {
> -	uint64_t magic;		/* VGT_MAGIC */
> -	uint16_t version_major;
> -	uint16_t version_minor;
> -	uint32_t vgt_id;	/* ID of vGT instance */
> -	uint32_t rsv1[12];	/* pad to offset 0x40 */
> -	/*
> -	 *  Data structure to describe the balooning info of resources.
> -	 *  Each VM can only have one portion of continuous area for now.
> -	 *  (May support scattered resource in future)
> -	 *  (starting from offset 0x40)
> -	 */
> -	struct {
> -		/* Aperture register balooning */
> -		struct {
> -			uint32_t base;
> -			uint32_t size;
> -		} mappable_gmadr;	/* aperture */
> -		/* GMADR register balooning */
> -		struct {
> -			uint32_t base;
> -			uint32_t size;
> -		} nonmappable_gmadr;	/* non aperture */
> -		/* allowed fence registers */
> -		uint32_t fence_num;
> -		uint32_t rsv2[3];
> -	} avail_rs;		/* available/assigned resource */
> -	uint32_t rsv3[0x200 - 24];	/* pad to half page */
> -	/*
> -	 * The bottom half page is for response from Gfx driver to hypervisor.
> -	 */
> -	uint32_t rsv4;
> -	uint32_t display_ready;	/* ready for display owner switch */
> -
> -	uint32_t rsv5[4];
> -
> -	uint32_t g2v_notify;
> -	uint32_t rsv6[7];
> -
> -	struct {
> -		uint32_t lo;
> -		uint32_t hi;
> -	} pdp[4];
> -
> -	uint32_t execlist_context_descriptor_lo;
> -	uint32_t execlist_context_descriptor_hi;
> -
> -	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
> -} __packed;
> -
> -#define vgtif_reg(x) \
> -	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
> -
> -/* vGPU display status to be used by the host side */
> -#define VGT_DRV_DISPLAY_NOT_READY 0
> -#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
> +#include "i915_pvinfo.h"
>  
>  extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
>  extern int intel_vgt_balloon(struct drm_device *dev);
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 2/9] drm/i915: Fold vGPU active check into inner functions
  2016-06-02 16:36 ` [PATCH v6 2/9] drm/i915: Fold vGPU active check into inner functions Zhi Wang
@ 2016-06-03  8:49   ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  8:49 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> v5:
> - Let functions take struct drm_i915_private *. (Tvrtko)
> 
> - Fold vGPU related active check into the inner functions. (Kevin)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++-------
>  drivers/gpu/drm/i915/i915_vgpu.c    | 13 +++++++++----
>  drivers/gpu/drm/i915/i915_vgpu.h    |  4 ++--
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4668477..6f203fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2732,11 +2732,9 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>  	i915_address_space_init(&ggtt->base, dev_priv);
>  	ggtt->base.total += PAGE_SIZE;
>  
> -	if (intel_vgpu_active(dev_priv)) {
> -		ret = intel_vgt_balloon(dev);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = intel_vgt_balloon(dev_priv);
> +	if (ret)
> +		return ret;
>  
>  	if (!HAS_LLC(dev))
>  		ggtt->base.mm.color_adjust = i915_gtt_color_adjust;
> @@ -2836,8 +2834,7 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev)
>  	i915_gem_cleanup_stolen(dev);
>  
>  	if (drm_mm_initialized(&ggtt->base.mm)) {
> -		if (intel_vgpu_active(dev_priv))
> -			intel_vgt_deballoon();
> +		intel_vgt_deballoon(dev_priv);
>  
>  		drm_mm_takedown(&ggtt->base.mm);
>  		list_del(&ggtt->base.global_link);
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index d5a7a5e..5312816 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -101,10 +101,13 @@ static struct _balloon_info_ bl_info;
>   * This function is called to deallocate the ballooned-out graphic memory, when
>   * driver is unloaded or when ballooning fails.
>   */
> -void intel_vgt_deballoon(void)
> +void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
>  {
>  	int i;
>  
> +	if (!intel_vgpu_active(dev_priv))
> +		return;
> +
>  	DRM_DEBUG("VGT deballoon.\n");
>  
>  	for (i = 0; i < 4; i++) {
> @@ -177,9 +180,8 @@ static int vgt_balloon_space(struct drm_mm *mm,
>   * Returns:
>   * zero on success, non-zero if configuration invalid or ballooning failed
>   */
> -int intel_vgt_balloon(struct drm_device *dev)
> +int intel_vgt_balloon(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	unsigned long ggtt_end = ggtt->base.start + ggtt->base.total;
>  
> @@ -187,6 +189,9 @@ int intel_vgt_balloon(struct drm_device *dev)
>  	unsigned long unmappable_base, unmappable_size, unmappable_end;
>  	int ret;
>  
> +	if (!intel_vgpu_active(dev_priv))
> +		return 0;
> +
>  	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
>  	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
>  	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
> @@ -258,6 +263,6 @@ int intel_vgt_balloon(struct drm_device *dev)
>  
>  err:
>  	DRM_ERROR("VGT balloon fail\n");
> -	intel_vgt_deballoon();
> +	intel_vgt_deballoon(dev_priv);

This function (and any others similar to it) needs to be changed to
have a proper goto teardown path as a follow-up patch. Destructors/fini
functions are only expected to be called after a succesful
initialization, so calling it at random phase in init function is bad.

But as this patch is a step in the right direction;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index 07e67d5..f8917c6 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -27,7 +27,7 @@
>  #include "i915_pvinfo.h"
>  
>  extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
> -extern int intel_vgt_balloon(struct drm_device *dev);
> -extern void intel_vgt_deballoon(void);
> +extern int intel_vgt_balloon(struct drm_i915_private *dev_priv);
> +extern void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
>  
>  #endif /* _I915_VGPU_H_ */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g
  2016-06-02 16:36 ` [PATCH v6 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
@ 2016-06-03  9:14   ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  9:14 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> This patch introduces the very basic framework of GVT-g device model,
> includes basic prototypes, definitions, initialization.
> 
> v6:
> - Refine introduction in Kconfig. (Chris)
> - The exposed API functions will take struct intel_gvt * instead of
> void *. (Chris/Tvrtko)
> - Remove most memebers of strct intel_gvt_device_info. Will add them
> in the device model patches.(Chris)
> - Remove gvt_info() and gvt_err() in debug.h. (Chris)
> - Move GVT kernel parameter into i915_params. (Chris)
> - Remove include/drm/i915_gvt.h, as GVT-g will be built within i915.
> - Remove the redundant struct i915_gvt *, as the functions in i915
> will directly take struct intel_gvt *.
> - Add more comments for reviewer.
> 
> v5:
> Take Tvrtko's comments:
> - Fix the misspelled words in Kconfig
> - Let functions take drm_i915_private * instead of struct drm_device *
> - Remove redundant prints/local varible initialization
> 
> v3:
> Take Joonas' comments:
> - Change file name i915_gvt.* to intel_gvt.*
> - Move GVT kernel parameter into intel_gvt.c
> - Remove redundant debug macros
> - Change error handling style
> - Add introductions for some stub functions
> - Introduce drm/i915_gvt.h.
> 
> Take Kevin's comments:
> - Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c
> 
> v2:
> - Introduce i915_gvt.c.
> It's necessary to introduce the stubs between i915 driver and GVT-g host,
> as GVT-g components is configurable in kernel config. When disabled, the
> stubs here do nothing.
> 
> Take Joonas' comments:
> - Replace boolean return value with int.
> - Replace customized info/warn/debug macros with DRM macros.
> - Document all non-static functions like i915.
> - Remove empty and unused functions.
> - Replace magic number with marcos.
> - Set GVT-g in kernel config to "n" by default.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/Kconfig         |  19 ++++
>  drivers/gpu/drm/i915/Makefile        |   5 +
>  drivers/gpu/drm/i915/gvt/Makefile    |   5 +
>  drivers/gpu/drm/i915/gvt/debug.h     |  34 +++++++
>  drivers/gpu/drm/i915/gvt/gvt.c       | 181 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.h       |  75 +++++++++++++++
>  drivers/gpu/drm/i915/gvt/hypercall.h |  38 ++++++++
>  drivers/gpu/drm/i915/gvt/mpt.h       |  49 ++++++++++
>  drivers/gpu/drm/i915/i915_dma.c      |  17 +++-
>  drivers/gpu/drm/i915/i915_drv.h      |  10 ++
>  drivers/gpu/drm/i915/i915_params.c   |   5 +
>  drivers/gpu/drm/i915/i915_params.h   |   1 +
>  drivers/gpu/drm/i915/intel_gvt.c     |  90 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_gvt.h     |  46 +++++++++
>  14 files changed, 571 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
>  create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
>  create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
>  create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
>  create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
>  create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
>  create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
>  create mode 100644 drivers/gpu/drm/i915/intel_gvt.h
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 29a32b1..e4fd9da 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -57,6 +57,25 @@ config DRM_I915_USERPTR
>  
>  	  If in doubt, say "Y".
>  
> +config DRM_I915_GVT
> +        bool "Enable Intel GVT-g graphics virtualization host support"
> +        depends on DRM_I915
> +        default n
> +        help
> +	  Choose this option if you want to enable Intel GVT-g graphics
> +	  virtualization technology host support with integrated graphics.
> +	  With GVT-g, it's possible to have one integrated graphics
> +	  device shared by multiple VMs under different hypervisors.
> +
> +	  Note that at least one hypervisor like Xen or KVM is required for
> +	  this driver to work, and it only supports newer device from
> +	  Broadwell+. For further information and setup guide, you can
> +	  visit: http://01.org/zh/igvt-g.

Maybe link should be http://01.org/igvt-g (first result in Google)
which currently redirects to that link, /zh/ would indicate the page to
be in Chinese, so maybe the page ought to be fixed to change the page
language to English in settings.

> +
> +	  The first version is still preliminary. Use it under you own risk.
> +
> +	  If in doubt, say "N".
> +
>  menu "drm/i915 Debugging"
>  depends on DRM_I915
>  depends on EXPERT
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 7e29444..276abf1 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -104,6 +104,11 @@ i915-y += i915_vgpu.o
>  # legacy horrors
>  i915-y += i915_dma.o
>  
> +ifeq ($(CONFIG_DRM_I915_GVT),y)
> +i915-y += intel_gvt.o
> +include $(src)/gvt/Makefile
> +endif
> +
>  obj-$(CONFIG_DRM_I915)  += i915.o
>  
>  CFLAGS_i915_trace_points.o := -I$(src)
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> new file mode 100644
> index 0000000..d0f21a6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -0,0 +1,5 @@
> +GVT_DIR := gvt
> +GVT_SOURCE := gvt.o
> +
> +ccflags-y                      += -I$(src) -I$(src)/$(GVT_DIR) -Wall
> +i915-y			       += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
> diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
> new file mode 100644
> index 0000000..7ef412b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/debug.h
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef __GVT_DEBUG_H__
> +#define __GVT_DEBUG_H__
> +
> +#define gvt_dbg_core(fmt, args...) \
> +	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> +
> +/*
> + * Other GVT debug stuff will be introduced in the GVT device model patches.
> + */
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> new file mode 100644
> index 0000000..9f1d3e7
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -0,0 +1,181 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "gvt.h"
> +
> +struct intel_gvt_host intel_gvt_host;
> +
> +static const char * const supported_hypervisors[] = {
> +	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
> +	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
> +};
> +
> +#define MB(x) (x * 1024ULL * 1024ULL)
> +#define GB(x) (x * MB(1024))
> +
> +/* Load MPT modules and detect if we're running in host */
> +static int init_gvt_host(void)
> +{
> +	if (intel_gvt_host.initialized)
> +		return 0;
> +
> +	/* Xen DOM U */
> +	if (xen_domain() && !xen_initial_domain())
> +		return -ENODEV;
> +
> +	/* Try to load MPT modules for hypervisors */
> +	if (xen_initial_domain()) {
> +		/* In Xen dom0 */
> +		intel_gvt_host.mpt = try_then_request_module(
> +				symbol_get(xengt_mpt), "xengt");
> +		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_XEN;
> +	} else {
> +		/* not in Xen. Try KVMGT */
> +		intel_gvt_host.mpt = try_then_request_module(
> +				symbol_get(kvmgt_mpt), "kvm");
> +		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_KVM;
> +	}
> +
> +	/* Fail to load MPT modules - bail out */
> +	if (!intel_gvt_host.mpt)
> +		return -EINVAL;
> +
> +	/* Try to detect if we're running in host instead of VM. */
> +	if (!intel_gvt_hypervisor_detect_host())
> +		return -ENODEV;
> +
> +	gvt_dbg_core("Running with hypervisor %s in host mode\n",
> +			supported_hypervisors[intel_gvt_host.hypervisor_type]);
> +
> +	idr_init(&intel_gvt_host.gvt_idr);
> +	mutex_init(&intel_gvt_host.gvt_idr_lock);
> +	intel_gvt_host.initialized = true;
> +	return 0;
> +}
> +
> +static void init_device_info(struct intel_gvt *gvt)
> +{
> +	if (IS_BROADWELL(gvt->dev_priv))
> +		gvt->device_info.max_support_vgpus = 8;
> +	/* This function will grow large in GVT device model patches. */
> +}
> +
> +static void free_gvt_device(struct intel_gvt *gvt)
> +{
> +	mutex_lock(&intel_gvt_host.gvt_idr_lock);
> +	idr_remove(&intel_gvt_host.gvt_idr, gvt->id);
> +	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
> +
> +	vfree(gvt);
> +}
> +
> +static struct intel_gvt *alloc_gvt_device(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_gvt *gvt;
> +	int ret;
> +
> +	/*
> +	 * This data structure will grow large in future, use vzalloc() at
> +	 * the beginning.
> +	 */
> +	gvt = vzalloc(sizeof(*gvt));
> +	if (!gvt)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mutex_lock(&intel_gvt_host.gvt_idr_lock);
> +	ret = idr_alloc(&intel_gvt_host.gvt_idr, gvt, 0, 0, GFP_KERNEL);
> +	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
> +
> +	if (ret < 0)
> +		goto err;
> +
> +	gvt->id = ret;
> +	mutex_init(&gvt->lock);
> +	gvt->dev_priv = dev_priv;
> +	idr_init(&gvt->vgpu_idr);
> +
> +	return gvt;
> +err:

Nack. Free/fini/etc. functions only to be called after successful init.

For example in this case if the ID allocation fails (lets assume we run
out of IDs) gvt->id is not assigned to so 0 and the free_gvt_device
ends up removing ID 0 which could be rather bad?

This is the _exact_ reason why one needs to implement a proper teardown
path.

> +	free_gvt_device(gvt);
> +	return ERR_PTR(ret);
> +}
> +
> +/**
> + * intel_gvt_destroy_device - destroy a GVT device
> + * @gvt: intel gvt device
> + *
> + * This function is called at the driver unloading stage, to destroy a
> + * GVT device and free the related resources.
> + *
> + */
> +void intel_gvt_destroy_device(struct intel_gvt *gvt)
> +{
> +	/* Another de-initialization of GVT components will be introduced. */
> +	free_gvt_device(gvt);
> +}
> +
> +/**
> + * intel_gvt_create_device - create a GVT device
> + * @dev_priv: drm i915 private data
> + *
> + * This function is called at the initialization stage, to create a
> + * GVT device and initialize necessary GVT components for it.
> + *
> + * Returns:
> + * pointer to the intel gvt device structure, error pointer if failed.
> + *
> + */
> +struct intel_gvt *intel_gvt_create_device(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_gvt *gvt;
> +	int ret;
> +
> +	ret = init_gvt_host();
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	gvt_dbg_core("create new gvt device\n");
> +
> +	gvt = alloc_gvt_device(dev_priv);
> +	if (IS_ERR(gvt)) {
> +		ret = PTR_ERR(gvt);
> +		goto out_err;
> +	}
> +
> +	gvt_dbg_core("init gvt device, id %d\n", gvt->id);
> +
> +	init_device_info(gvt);
> +	/*
> +	 * Other initialization of GVT components will be called here.
> +	 */
> +	gvt_dbg_core("gvt device creation is done, id %d\n", gvt->id);
> +
> +	return gvt;
> +
> +out_err:
> +	return ERR_PTR(ret);
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> new file mode 100644
> index 0000000..f021487
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -0,0 +1,75 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_H_
> +#define _GVT_H_
> +
> +#include "i915_drv.h"
> +#include "i915_pvinfo.h"
> +
> +#include "debug.h"
> +#include "hypercall.h"
> +
> +#define GVT_MAX_VGPU 8
> +#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) - 1)))

There's round_down and round_up and whatnot in linux/kernel.h, please
use those or add a comment what super special this achieves.

> +
> +enum {
> +	INTEL_GVT_HYPERVISOR_XEN = 0,
> +	INTEL_GVT_HYPERVISOR_KVM,
> +};
> +
> +struct intel_gvt_host {
> +	bool initialized;
> +	int hypervisor_type;
> +	struct mutex gvt_idr_lock;
> +	struct idr gvt_idr;
> +	struct intel_gvt_mpt *mpt;
> +};
> +
> +extern struct intel_gvt_host intel_gvt_host;
> +
> +/* Describe per-platform limitations. */
> +struct intel_gvt_device_info {
> +	u32 max_support_vgpus;
> +	/* This data structure will grow bigger in GVT device model patches */
> +};
> +
> +struct intel_vgpu {
> +	struct intel_gvt *gvt;
> +	int id;
> +	unsigned long handle; /* vGPU handle used by hypervisor MPT modules */
> +};
> +
> +struct intel_gvt {
> +	struct mutex lock;
> +	int id;
> +
> +	struct drm_i915_private *dev_priv;
> +	struct idr vgpu_idr;	/* vGPU IDR pool */
> +
> +	struct intel_gvt_device_info device_info;
> +};
> +
> +#include "mpt.h"
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
> new file mode 100644
> index 0000000..254df8b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> @@ -0,0 +1,38 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_HYPERCALL_H_
> +#define _GVT_HYPERCALL_H_
> +
> +/*
> + * Specific GVT-g MPT modules function collections. Currently GVT-g supports
> + * both Xen and KVM by providing dedicated hypervisor-related MPT modules.
> + */
> +struct intel_gvt_mpt {
> +	int (*detect_host)(void);
> +};
> +
> +extern struct intel_gvt_mpt xengt_mpt;
> +extern struct intel_gvt_mpt kvmgt_mpt;
> +
> +#endif /* _GVT_HYPERCALL_H_ */
> diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
> new file mode 100644
> index 0000000..783f4f8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_MPT_H_
> +#define _GVT_MPT_H_
> +
> +/**
> + * DOC: Hypervisor Service APIs for GVT-g Core Logic
> + *
> + * This is the glue layer between specific hypervisor MPT modules and GVT-g core
> + * logic. Each kind of hypervisor MPT module provides a collection of function
> + * callbacks via gvt_kernel_dm and will be attached to GVT host when driver
> + * loading. GVT-g core logic will call these APIs to request specific services
> + * from hypervisor.
> + */
> +
> +/**
> + * intel_gvt_hypervisor_detect_host - check if GVT-g is running within
> + * hypervisor host/privilged domain
> + *
> + * Returns:
> + * Zero on success, -ENODEV if current kernel is running inside a VM
> + */
> +static inline int intel_gvt_hypervisor_detect_host(void)
> +{
> +	return intel_gvt_host.mpt->detect_host();
> +}
> +
> +#endif /* _GVT_MPT_H_ */
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 07edaed..0baf64a 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -35,6 +35,7 @@
>  #include "intel_drv.h"
>  #include 
>  #include "i915_drv.h"
> +#include "intel_gvt.h"
>  #include "i915_vgpu.h"
>  #include "i915_trace.h"
>  #include 
> @@ -1244,18 +1245,22 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  		goto out_ggtt;
>  	}
>  
> +	ret = intel_gvt_init(dev_priv);
> +	if (ret)
> +		goto out_ggtt;
> +
>  	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
>  	 * otherwise the vga fbdev driver falls over. */
>  	ret = i915_kick_out_firmware_fb(dev_priv);
>  	if (ret) {
>  		DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
> -		goto out_ggtt;
> +		goto out_gvt;
>  	}
>  
>  	ret = i915_kick_out_vgacon(dev_priv);
>  	if (ret) {
>  		DRM_ERROR("failed to remove conflicting VGA console\n");
> -		goto out_ggtt;
> +		goto out_gvt;
>  	}
>  
>  	pci_set_master(dev->pdev);
> @@ -1266,7 +1271,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  		if (ret) {
>  			DRM_ERROR("failed to set DMA mask\n");
>  
> -			goto out_ggtt;
> +			goto out_gvt;
>  		}
>  	}
>  
> @@ -1296,7 +1301,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  				     aperture_size);
>  	if (!ggtt->mappable) {
>  		ret = -EIO;
> -		goto out_ggtt;
> +		goto out_gvt;
>  	}
>  
>  	ggtt->mtrr = arch_phys_wc_add(ggtt->mappable_base,
> @@ -1329,6 +1334,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  
>  	return 0;
>  
> +out_gvt:
> +	intel_gvt_cleanup(dev_priv);
>  out_ggtt:
>  	i915_ggtt_cleanup_hw(dev);
>  
> @@ -1487,6 +1494,8 @@ int i915_driver_unload(struct drm_device *dev)
>  
>  	intel_fbdev_fini(dev);
>  
> +	intel_gvt_cleanup(dev_priv);
> +
>  	ret = i915_gem_suspend(dev);
>  	if (ret) {
>  		DRM_ERROR("failed to idle hardware: %d\n", ret);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e4c8e34..bdf499f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -61,6 +61,8 @@
>  #include "i915_gem_gtt.h"
>  #include "i915_gem_render_state.h"
>  
> +#include "intel_gvt.h"
> +
>  /* General customization:
>   */
>  
> @@ -1737,6 +1739,8 @@ struct drm_i915_private {
>  
>  	struct i915_virtual_gpu vgpu;
>  
> +	struct intel_gvt *gvt;

Same criticism as with GPU scheduler, why not data member like all
around?

> +
>  	struct intel_guc guc;
>  
>  	struct intel_csr csr;
> @@ -2938,6 +2942,12 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
>  u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
>  
>  void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
> +
> +static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
> +{
> +	return dev_priv->gvt ? true : false;
> +}
> +
>  static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
>  {
>  	return dev_priv->vgpu.active;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 5e18cf9..c9615ce 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -60,6 +60,7 @@ struct i915_params i915 __read_mostly = {
>  	.enable_dp_mst = true,
>  	.inject_load_failure = 0,
>  	.enable_dpcd_backlight = false,
> +	.enable_gvt = false,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -222,3 +223,7 @@ MODULE_PARM_DESC(inject_load_failure,
>  module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
>  MODULE_PARM_DESC(enable_dpcd_backlight,
>  	"Enable support for DPCD backlight control (default:false)");
> +
> +module_param_named(enable_gvt, i915.enable_gvt, bool, 0600);
> +MODULE_PARM_DESC(enable_gvt,
> +	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 1323261..0ad020b 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -63,6 +63,7 @@ struct i915_params {
>  	bool nuclear_pageflip;
>  	bool enable_dp_mst;
>  	bool enable_dpcd_backlight;
> +	bool enable_gvt;
>  };
>  
>  extern struct i915_params i915 __read_mostly;
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> new file mode 100644
> index 0000000..8556f58
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -0,0 +1,90 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "i915_drv.h"
> +#include "intel_gvt.h"
> +
> +/**
> + * DOC: Intel GVT-g host support
> + *
> + * Intel GVT-g is a graphics virtualization technology which shares the
> + * GPU among multiple virtual machines on a time-sharing basis. Each
> + * virtual machine is presented a virtual GPU (vGPU), which has equivalent
> + * features as the underlying physical GPU (pGPU), so i915 driver can run
> + * seamlessly in a virtual machine. This file provides the englightments
> + * of GVT and the necessary components used by GVT in i915 driver.
> + */
> +
> +static bool is_supported_device(struct drm_i915_private *dev_priv)
> +{
> +	if (IS_BROADWELL(dev_priv))
> +		return true;
> +	return false;
> +}
> +
> +/**
> + * intel_gvt_init - initialize GVT components
> + * @dev_priv: drm i915 private data
> + *
> + * This function is called at the initialization stage to create a GVT device.
> + *
> + * Returns:
> + * Zero on success, negative error code if failed.
> + *
> + */
> +int intel_gvt_init(struct drm_i915_private *dev_priv)
> +{
> +	if (!i915.enable_gvt) {
> +		DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
> +		return 0;
> +	}
> +
> +	if (!is_supported_device(dev_priv)) {
> +		DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
> +		return 0;
> +	}
> +
> +	dev_priv->gvt = intel_gvt_create_device(dev_priv);
> +	if (IS_ERR(dev_priv->gvt)) {
> +		DRM_DEBUG_DRIVER("GVT-g is disabled\n");
> +		dev_priv->gvt = NULL;
> +		return 0;
> +	}
> +	return 0;
> +}
> +
> +/**
> + * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
> + * @dev_priv: drm i915 private *
> + *
> + * This function is called at the i915 driver unloading stage, to shutdown
> + * GVT components and release the related resources.
> + */
> +void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
> +{
> +	if (!intel_gvt_active(dev_priv))
> +		return;
> +
> +	intel_gvt_destroy_device(dev_priv->gvt);
> +	dev_priv->gvt = NULL;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
> new file mode 100644
> index 0000000..b9b361b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.h
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _INTEL_GVT_H_
> +#define _INTEL_GVT_H_
> +
> +#ifdef CONFIG_DRM_I915_GVT
> +#include "gvt/gvt.h"
> +extern int intel_gvt_init(struct drm_i915_private *dev_priv);
> +extern void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
> +extern struct intel_gvt *intel_gvt_create_device(
> +		struct drm_i915_private *dev_priv);
> +extern void intel_gvt_destroy_device(struct intel_gvt *gvt);
> +#else
> +struct intel_gvt {
> +};

I think this might not be the best idea. I'd just have the structure
declared always, in this header (because we're in intel_gvt.h after
all, what a better place to look?) and have it as a data (not-pointer)
member in the drm_i915_private struct.

Regards, Joonas

> +static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
> +{
> +	return 0;
> +}
> +static inline void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
> +{
> +}
> +#endif
> +
> +#endif /* _INTEL_GVT_H_ */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 4/9] drm/i915: Introduce host graphics memory partition for GVT-g
  2016-06-02 16:36 ` [PATCH v6 4/9] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
@ 2016-06-03  9:17   ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  9:17 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> From: Bing Niu <bing.niu@intel.com>
> 
> This patch introduces host graphics memory partition when GVT-g
> is enabled.
> 
> Under GVT-g, i915 host driver only owned limited graphics resources,
> others are managed by GVT-g resource allocator and kept for other vGPUs.
> 
> v6:
> 
> - Remove kernel parameters used to configure GGTT owned by host. (Chris)
> - Other coding style comments from Chris.
> - Add more comments for reviewer.
> 
> v3:
> 
> - Remove fence partition, will use i915 fence stealing in future.(Kevin)
> - Santinize GVT host gm kernel parameters. (Joonas)
> 
> v2:
> - Address all coding-style comments from Joonas previously.
> - Fix errors and warnning reported by checkpatch.pl. (Joonas)
> - Move the graphs into the header files. (Daniel)
> 
> Signed-off-by: Bing Niu <bing.niu@intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_vgpu.c | 23 +++++++++++++++++------
>  drivers/gpu/drm/i915/intel_gvt.h | 25 +++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index 5312816..0d891a3 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -189,14 +189,25 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
>  	unsigned long unmappable_base, unmappable_size, unmappable_end;
>  	int ret;
>  
> -	if (!intel_vgpu_active(dev_priv))
> +	if (intel_gvt_active(dev_priv)) {
> +		/* Retrieve GGTT partition information from macros */
> +		mappable_base = 0;
> +		mappable_size = INTEL_GVT_HOST_LOW_GM_SIZE;
> +		unmappable_base = dev_priv->ggtt.mappable_end;
> +		unmappable_size = INTEL_GVT_HOST_HIGH_GM_SIZE;
> +	} else if (intel_vgpu_active(dev_priv)) {
> +		/* Retrieve GGTT partition information from PVINFO */
> +		mappable_base = I915_READ(
> +				vgtif_reg(avail_rs.mappable_gmadr.base));
> +		mappable_size = I915_READ(
> +				vgtif_reg(avail_rs.mappable_gmadr.size));
> +		unmappable_base = I915_READ(
> +				vgtif_reg(avail_rs.nonmappable_gmadr.base));
> +		unmappable_size = I915_READ(
> +				vgtif_reg(avail_rs.nonmappable_gmadr.size));
> +	} else
>  		return 0;
>  
> -	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
> -	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
> -	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
> -	unmappable_size = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size));
> -
>  	mappable_end = mappable_base + mappable_size;
>  	unmappable_end = unmappable_base + unmappable_size;
>  
> diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
> index b9b361b..f0b9aac 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.h
> +++ b/drivers/gpu/drm/i915/intel_gvt.h
> @@ -24,6 +24,31 @@
>  #ifndef _INTEL_GVT_H_
>  #define _INTEL_GVT_H_
>  
> +/*
> + * Under GVT-g, i915 host driver only owned limited graphics resources,
> + * others are managed by GVT-g resource allocator and kept for other vGPUs.
> + *
> + * For graphics memory space partition, a typical layout looks like:
> + *
> + * +-------+-----------------------+------+-----------------------+
> + * |* Host |   *GVT-g Resource     |* Host|   *GVT-g Resource     |
> + * | Owned |   Allocator Managed   | Owned|   Allocator Managed   |
> + * |       |                       |      |                       |
> + * +---------------+-------+----------------------+-------+-------+
> + * |       |       |       |       |      |       |       |       |
> + * | i915  | vm 1  | vm 2  | vm 3  | i915 | vm 1  | vm 2  | vm 3  |
> + * |       |       |       |       |      |       |       |       |
> + * +-------+-------+-------+--------------+-------+-------+-------+
> + * |           Aperture            |            Hidden            |
> + * +-------------------------------+------------------------------+
> + * |                       GGTT memory space                      |
> + * +--------------------------------------------------------------+
> + */
> +
> +/* GGTT memory space owned by host */
> +#define INTEL_GVT_HOST_LOW_GM_SIZE (96 * 1024 * 1024)
> +#define INTEL_GVT_HOST_HIGH_GM_SIZE (384 * 1024 * 1024)

Might be worth a comment where these numbers come from, with that;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> +
>  #ifdef CONFIG_DRM_I915_GVT
>  #include "gvt/gvt.h"
>  extern int intel_gvt_init(struct drm_i915_private *dev_priv);
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 5/9] drm/i915: Make ring buffer size of a LRC context configurable
  2016-06-02 16:36 ` [PATCH v6 5/9] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
@ 2016-06-03  9:20   ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  9:20 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> This patch introduces an option for configuring the ring buffer size
> of a LRC context after the context creation.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 1 +
>  drivers/gpu/drm/i915/i915_gem_context.c | 1 +
>  drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bdf499f..a4af035 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -878,6 +878,7 @@ struct i915_gem_context {
>  		int pin_count;
>  		bool initialised;
>  	} engine[I915_NUM_ENGINES];
> +	u32 lrc_ring_buffer_size;
>  
>  	struct list_head link;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index a3b11aa..1663981 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -295,6 +295,7 @@ __create_hw_context(struct drm_device *dev,
>  	ctx->remap_slice = ALL_L3_SLICES(dev_priv);
>  
>  	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
> +	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
>  
>  	return ctx;
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 5c191a1..72a0cca 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2484,7 +2484,8 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>  		return PTR_ERR(ctx_obj);
>  	}
>  
> -	ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE);
> +	ringbuf = intel_engine_create_ringbuffer(engine,
> +			ctx->lrc_ring_buffer_size);
>  	if (IS_ERR(ringbuf)) {
>  		ret = PTR_ERR(ringbuf);
>  		goto error_deref_obj;
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 6/9] drm/i915: Make addressing mode bits in context descriptor configurable
  2016-06-02 16:36 ` [PATCH v6 6/9] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
@ 2016-06-03  9:25   ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  9:25 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> Currently the addressing mode bit in context descriptor is statically
> generated from the configuration of system-wide PPGTT usage model.
> 
> GVT-g will load the PPGTT shadow page table by itself and probably one
> guest is using a different addressing mode with i915 host. The addressing
> mode bits of a LRC context should be configurable under this case.
> 
> v6:
> 
> - Directly save the addressing mode bits inside i915_gem_context. (Chris)
> - Move the LRC context addressing mode bits into intel_lrc.h. (Chris)
> 
> v5:
> 
> - Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 ++
>  drivers/gpu/drm/i915/intel_lrc.c        | 13 +------------
>  drivers/gpu/drm/i915/intel_lrc.h        | 11 +++++++++++
>  4 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a4af035..f6cb60a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -879,6 +879,7 @@ struct i915_gem_context {
>  		bool initialised;
>  	} engine[I915_NUM_ENGINES];
>  	u32 lrc_ring_buffer_size;
> +	u32 lrc_addressing_mode_bits;
>  
>  	struct list_head link;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 1663981..d9d7779 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -296,6 +296,8 @@ __create_hw_context(struct drm_device *dev,
>  
>  	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
>  	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
> +	ctx->lrc_addressing_mode_bits = GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
> +		GEN8_CTX_ADDRESSING_MODE_SHIFT;
>  
>  	return ctx;
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 72a0cca..ffb436c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -208,16 +208,6 @@
>  } while (0)
>  
>  enum {
> -	ADVANCED_CONTEXT = 0,
> -	LEGACY_32B_CONTEXT,
> -	ADVANCED_AD_CONTEXT,
> -	LEGACY_64B_CONTEXT
> -};
> -#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
> -#define GEN8_CTX_ADDRESSING_MODE(dev)  (USES_FULL_48BIT_PPGTT(dev) ?\
> -		LEGACY_64B_CONTEXT :\
> -		LEGACY_32B_CONTEXT)
> -enum {
>  	FAULT_AND_HANG = 0,
>  	FAULT_AND_HALT, /* Debug only */
>  	FAULT_AND_STREAM,
> @@ -281,8 +271,6 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
>  					(engine->id == VCS || engine->id == VCS2);
>  
>  	engine->ctx_desc_template = GEN8_CTX_VALID;
> -	engine->ctx_desc_template |= GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
> -				   GEN8_CTX_ADDRESSING_MODE_SHIFT;
>  	if (IS_GEN8(dev_priv))
>  		engine->ctx_desc_template |= GEN8_CTX_L3LLC_COHERENT;
>  	engine->ctx_desc_template |= GEN8_CTX_PRIVILEGE;
> @@ -326,6 +314,7 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
>  	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<
>  
>  	desc = engine->ctx_desc_template;			/* bits  0-11 */
> +	desc |= ctx->lrc_addressing_mode_bits;			/* bits  3-4  */
>  	desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
>  								/* bits 12-31 */
>  	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;		/* bits 32-52 */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index a8db42a..e133c33 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -28,6 +28,17 @@
>  
>  #define GEN8_LR_CONTEXT_ALIGN 4096
>  
> +enum {
> +	ADVANCED_CONTEXT = 0,
> +	LEGACY_32B_CONTEXT,
> +	ADVANCED_AD_CONTEXT,
> +	LEGACY_64B_CONTEXT
> +};

I think these should be prefixed somehow?

> +#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
> +#define GEN8_CTX_ADDRESSING_MODE(dev_priv) (USES_FULL_48BIT_PPGTT(dev_priv) ?\
> +		LEGACY_64B_CONTEXT : \
> +		LEGACY_32B_CONTEXT)
> +

I'm also unsure if there is a story behind having these regs here
instead of i915_reg.h? I would not add more at least.

Regards, Joonas

>  /* Execlists regs */
>  #define RING_ELSP(ring)				_MMIO((ring)->mmio_base + 0x230)
>  #define RING_EXECLIST_STATUS_LO(ring)		_MMIO((ring)->mmio_base + 0x234)
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification
  2016-06-02 16:36 ` [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification Zhi Wang
@ 2016-06-03  9:40   ` Joonas Lahtinen
  2016-06-07 15:29     ` Wang, Zhi A
  0 siblings, 1 reply; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  9:40 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> This patch introduces an approach to track the execlist context status
> change.
> 
> GVT-g uses GVT context as the "shadow context". The content inside GVT
> context will be copied back to guest after the context is idle. So GVT-g
> has to know the status of the execlist context.
> 
> This function is configurable in the context creation service. Currently,
> Only GVT-g will create the "status-change-notification" enabled GEM
> context.
> 
> v6:
> 
> - When !CONFIG_DRM_I915_GVT, make GVT code as dead code then compiler
> could automatically eliminate them for us. (Chris)
> - Always initialize the notifier header, so it could be switched on/off
> at runtime. (Chris)
> 
> v5:
> 
> - Only compile this feature when CONFIG_DRM_I915_GVT is enabled.(Tvrtko)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
>  drivers/gpu/drm/i915/intel_lrc.c | 24 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.h |  5 +++++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f6cb60a..dee72d3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -877,9 +877,11 @@ struct i915_gem_context {
>  		u64 lrc_desc;
>  		int pin_count;
>  		bool initialised;
> +		struct atomic_notifier_head status_notifier;

I think this could be outside engine block, just one per context.

>  	} engine[I915_NUM_ENGINES];
>  	u32 lrc_ring_buffer_size;
>  	u32 lrc_addressing_mode_bits;
> +	bool enable_lrc_status_change_notification;
>  
>  	struct list_head link;
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index ffb436c..96d20c8 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -404,6 +404,22 @@ static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
>  	spin_unlock_irq(&dev_priv->uncore.lock);
>  }
>  
> +static inline void execlists_context_status_change(
> +		struct drm_i915_gem_request *rq,
> +		unsigned long status)
> +{
> +	/* The compiler should be fine with the dead-code elimination */

The comment could rather be "Currently, only GVT-g code uses status
notifications" :)

> +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> +		return;
> +
> +	if (!rq->ctx->enable_lrc_status_change_notification)
> +		return;
> +

I think above line should be dropped, just don't register notifier for
context that does not want notifications, and if the chain is empty, no
call is made.

> +	atomic_notifier_call_chain(
> +			&rq->ctx->engine[rq->engine->id].status_notifier,
> +			status, rq);
> +}
> +
>  static void execlists_context_unqueue(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
> @@ -439,6 +455,11 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
>  	if (unlikely(!req0))
>  		return;
>  
> +	execlists_context_status_change(req0, CONTEXT_SCHEDULE_IN);
> +
> +	if (req1)
> +		execlists_context_status_change(req1, CONTEXT_SCHEDULE_IN);
> +
>  	if (req0->elsp_submitted & engine->idle_lite_restore_wa) {
>  		/*
>  		 * WaIdleLiteRestore: make sure we never cause a lite restore
> @@ -477,6 +498,8 @@ execlists_check_remove_request(struct intel_engine_cs *engine, u32 ctx_id)
>  	if (--head_req->elsp_submitted > 0)
>  		return 0;
>  
> +	execlists_context_status_change(head_req, CONTEXT_SCHEDULE_OUT);
> +
>  	list_del(&head_req->execlist_link);
>  	i915_gem_request_unreference(head_req);
>  
> @@ -2489,6 +2512,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>  	ce->ringbuf = ringbuf;
>  	ce->state = ctx_obj;
>  	ce->initialised = engine->init_context == NULL;
> +	ATOMIC_INIT_NOTIFIER_HEAD(&ce->status_notifier);
>  
>  	return 0;
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index e133c33..7a7ae8d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -68,6 +68,11 @@ enum {
>  #define GEN8_CSB_READ_PTR(csb_status) \
>  	(((csb_status) & GEN8_CSB_READ_PTR_MASK) >> 8)
>  
> +enum {
> +	CONTEXT_SCHEDULE_IN = 0,
> +	CONTEXT_SCHEDULE_OUT,
> +};
> +

Again, prefixes? At least INTEL_

With above;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

>  /* Logical Rings */
>  int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
>  int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 8/9] drm/i915: Support LRC context single submission
  2016-06-02 16:36 ` [PATCH v6 8/9] drm/i915: Support LRC context single submission Zhi Wang
@ 2016-06-03  9:47   ` Joonas Lahtinen
  2016-06-03 11:25     ` Tian, Kevin
  2016-06-07 14:13     ` Wang, Zhi A
  0 siblings, 2 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  9:47 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv, chris

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> This patch introduces the support of LRC context signle submission.

"single"

> As GVT context may come from different guests, which requires different

"require"

> configuration of render registers. It can't be combined into a dual ELSP
> submission combo.
> 
> Only GVT-g will create this kinds of GEM context currently.
> 
> v6:
> - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
> 
> v5:
> 
> - Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dee72d3..92d01e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -882,6 +882,7 @@ struct i915_gem_context {
>  	u32 lrc_ring_buffer_size;
>  	u32 lrc_addressing_mode_bits;
>  	bool enable_lrc_status_change_notification;
> +	bool enable_lrc_single_submission;
>  
>  	struct list_head link;
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 96d20c8..ce707ea 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -446,6 +446,21 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
>  			i915_gem_request_unreference(req0);
>  			req0 = cursor;
>  		} else {
> +			/* Compiler will do the dead-code elimination */
> +			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
> +				/*
> +				 * req0 (after merged) ctx requires single
> +				 * submission, stop picking
> +				 */
> +				if (req0->ctx->enable_lrc_single_submission)
> +					break;
> +				/*
> +				 * req0 ctx doesn't require single submission,
> +				 * but next req ctx requires, stop picking
> +				 */
> +				if (cursor->ctx->enable_lrc_single_submission)
> +					break;
> +			}

I remember discussing this on the F2F, we will be aware of the VM IDs
at this point, so we could use that criterion, instead of just
disabling it pessimistically. Is there some reason we could not do it?

Check would be rather simple;

#if IS_ENABLED(CONFIG_DRM_I915_GVT)
if (req0->gvt.vmid != cursor->gvt.vmid)
	break;
#endif

I'm not sure if it will be worth the #if when vmid would be zero in
DOM0 always.

Regards, Joonas

>  			req1 = cursor;
>  			WARN_ON(req1->elsp_submitted);
>  			break;
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 9/9] drm/i915: Introduce GVT context creation API
  2016-06-02 16:36 ` [PATCH v6 9/9] drm/i915: Introduce GVT context creation API Zhi Wang
@ 2016-06-03  9:59   ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-03  9:59 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx, tvrtko.ursulin, kevin.tian, zhiyuan.lv,
	chris, daniel.vetter

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> GVT workload scheduler needs special host LRC contexts, the so called
> "shadow LRC context" to submit guest workload to host i915. During the
> guest workload submission, GVT fills the shadow LRC context with the
> content of guest LRC context: engine context is copied without changes,
> ring context is mostly owned by host i915.
> 
> The GVT-g workload scheduler flow:
> 
>          +-----------+                   +-----------+
>          | GVT Guest |                   | GVT Guest |
>          +-+-----^---+                   +-+-----^---+
>            |     |                         |     |
>            |     | GVT-g                   |     | GVT-g
> vELSP write|     | emulates     vELSP write|     | emulates
>            |     | Execlist/CSB            |     | Execlist/CSB
>            |     | Status                  |     | Status
>            |     |                         |     |
>     +------v-----+-------------------------v-----+---------+
>     |           GVT Virtual Execlist Submission            |
>     +------+-------------------------------+---------------+
>            |                               |
>            | Per-VM/Ring Workoad Q         | Per-VM/Ring Workload Q
>    +---------------------+--+      +------------------------+
>        +---v--------+    ^             +---v--------+
>        |GVT Workload|... |             |GVT Workload|...
>        +------------+    |             +------------+
>                          |
>                          | Pick Workload from Q
>     +--------------------+---------------------------------+
>     |                GVT Workload Scheduler                |
>     +--------------------+---------------------------------+
>                          |         * Shadow guest LRC context
>                   +------v------+  * Shadow guest ring buffer
>                   | GVT Context |  * Scan/Patch guest RB instructions
>                   +------+------+
>                          |
>                          v
>               Host i915 GEM Submission
> 

This kind of graph should also go to some kerneldoc!

Should the picture be updated to reflect that there is GVT context per
guest context? CC'ing Daniel to comment on.

Regards, Joonas

> v6:
> 
> - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
> 
> v5:
> - Only compile this feature when CONFIG_DRM_I915_GVT is enabled. (Tvrtko)
> - Rebase the code into new repo.
> - Add a comment about the ring buffer size. (Joonas)
> 
> v2:
> 
> Mostly based on Daniel's idea. Call the refactored core logic of GEM
> context creation service and LRC context creation service to create the GVT
> context.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index d9d7779..c0259d7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -342,6 +342,38 @@ i915_gem_create_context(struct drm_device *dev,
>  	return ctx;
>  }
>  
> +/**
> + * i915_gem_create_gvt_context - create a GVT GEM context
> + * @dev: drm device *
> + *
> + * This function is used to create a GVT specific GEM context.
> + *
> + * Returns:
> + * pointer to i915_gem_context on success, error pointer if failed
> + *
> + */
> +struct i915_gem_context *
> +i915_gem_create_gvt_context(struct drm_device *dev)
> +{
> +	struct i915_gem_context *ctx;
> +
> +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> +		return ERR_PTR(-ENODEV);
> +
> +	mutex_lock(&dev->struct_mutex);
> +
> +	ctx = i915_gem_create_context(dev, NULL);
> +	if (IS_ERR(ctx))
> +		goto out;
> +
> +	ctx->enable_lrc_status_change_notification = true;
> +	ctx->enable_lrc_single_submission = true;
> +	ctx->lrc_ring_buffer_size = 512 * PAGE_SIZE; /* Max ring buffer size */
> +out:
> +	mutex_unlock(&dev->struct_mutex);
> +	return ctx;
> +}
> +
>  static void i915_gem_context_unpin(struct i915_gem_context *ctx,
>  				   struct intel_engine_cs *engine)
>  {
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 8/9] drm/i915: Support LRC context single submission
  2016-06-03  9:47   ` Joonas Lahtinen
@ 2016-06-03 11:25     ` Tian, Kevin
  2016-06-07 14:13     ` Wang, Zhi A
  1 sibling, 0 replies; 24+ messages in thread
From: Tian, Kevin @ 2016-06-03 11:25 UTC (permalink / raw)
  To: Joonas Lahtinen, Wang, Zhi A, intel-gfx, tvrtko.ursulin, Lv,
	Zhiyuan, chris

> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Friday, June 03, 2016 5:47 PM
> 
> On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> > This patch introduces the support of LRC context signle submission.
> 
> "single"
> 
> > As GVT context may come from different guests, which requires different
> 
> "require"
> 
> > configuration of render registers. It can't be combined into a dual ELSP
> > submission combo.
> >
> > Only GVT-g will create this kinds of GEM context currently.
> >
> > v6:
> > - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
> >
> > v5:
> >
> > - Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)
> >
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> >  drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index dee72d3..92d01e3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -882,6 +882,7 @@ struct i915_gem_context {
> >  	u32 lrc_ring_buffer_size;
> >  	u32 lrc_addressing_mode_bits;
> >  	bool enable_lrc_status_change_notification;
> > +	bool enable_lrc_single_submission;
> >
> >  	struct list_head link;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 96d20c8..ce707ea 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -446,6 +446,21 @@ static void execlists_context_unqueue(struct intel_engine_cs
> *engine)
> >  			i915_gem_request_unreference(req0);
> >  			req0 = cursor;
> >  		} else {
> > +			/* Compiler will do the dead-code elimination */
> > +			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
> > +				/*
> > +				 * req0 (after merged) ctx requires single
> > +				 * submission, stop picking
> > +				 */
> > +				if (req0->ctx->enable_lrc_single_submission)
> > +					break;
> > +				/*
> > +				 * req0 ctx doesn't require single submission,
> > +				 * but next req ctx requires, stop picking
> > +				 */
> > +				if (cursor->ctx->enable_lrc_single_submission)
> > +					break;
> > +			}
> 
> I remember discussing this on the F2F, we will be aware of the VM IDs
> at this point, so we could use that criterion, instead of just
> disabling it pessimistically. Is there some reason we could not do it?
> 
> Check would be rather simple;
> 
> #if IS_ENABLED(CONFIG_DRM_I915_GVT)
> if (req0->gvt.vmid != cursor->gvt.vmid)
> 	break;
> #endif
> 
> I'm not sure if it will be worth the #if when vmid would be zero in
> DOM0 always.
> 

We'd better remove explicit reference to vmid here. 'vmid' is a very
Xen specific resource. There is no 'vmid' in KVM (we patched KVM to
introduce 'vmid' today which however likely won't be accepted by
KVM upstream). Here it should be some opaque handle (for Xen
it is vmid while for KVM it will be another handle identifiable in KVM
upstream)

Thanks
Kevin
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 8/9] drm/i915: Support LRC context single submission
  2016-06-03  9:47   ` Joonas Lahtinen
  2016-06-03 11:25     ` Tian, Kevin
@ 2016-06-07 14:13     ` Wang, Zhi A
  1 sibling, 0 replies; 24+ messages in thread
From: Wang, Zhi A @ 2016-06-07 14:13 UTC (permalink / raw)
  To: Joonas Lahtinen, intel-gfx, tvrtko.ursulin, Tian, Kevin, Lv,
	Zhiyuan, chris

For now If we want to achieve that, we have to add a member in struct i915_gem_request like your code. :(

/* Assume in all host GEM request, req->vgpu == NULL*/
If (req0->vgpu = req1->vgpu)
	combine!

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Friday, June 03, 2016 12:47 PM
> To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org;
> tvrtko.ursulin@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; chris@chris-wilson.co.uk
> Subject: Re: [PATCH v6 8/9] drm/i915: Support LRC context single submission
> 
> On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> > This patch introduces the support of LRC context signle submission.
> 
> "single"
> 
> > As GVT context may come from different guests, which requires
> > different
> 
> "require"
> 
> > configuration of render registers. It can't be combined into a dual
> > ELSP submission combo.
> >
> > Only GVT-g will create this kinds of GEM context currently.
> >
> > v6:
> > - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
> >
> > v5:
> >
> > - Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)
> >
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> >  drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index dee72d3..92d01e3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -882,6 +882,7 @@ struct i915_gem_context {
> >  	u32 lrc_ring_buffer_size;
> >  	u32 lrc_addressing_mode_bits;
> >  	bool enable_lrc_status_change_notification;
> > +	bool enable_lrc_single_submission;
> >
> >  	struct list_head link;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index 96d20c8..ce707ea 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -446,6 +446,21 @@ static void execlists_context_unqueue(struct
> intel_engine_cs *engine)
> >  			i915_gem_request_unreference(req0);
> >  			req0 = cursor;
> >  		} else {
> > +			/* Compiler will do the dead-code elimination */
> > +			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
> > +				/*
> > +				 * req0 (after merged) ctx requires single
> > +				 * submission, stop picking
> > +				 */
> > +				if (req0->ctx->enable_lrc_single_submission)
> > +					break;
> > +				/*
> > +				 * req0 ctx doesn't require single submission,
> > +				 * but next req ctx requires, stop picking
> > +				 */
> > +				if (cursor->ctx->enable_lrc_single_submission)
> > +					break;
> > +			}
> 
> I remember discussing this on the F2F, we will be aware of the VM IDs at this
> point, so we could use that criterion, instead of just disabling it pessimistically.
> Is there some reason we could not do it?
> 
> Check would be rather simple;
> 
> #if IS_ENABLED(CONFIG_DRM_I915_GVT)
> if (req0->gvt.vmid != cursor->gvt.vmid)
> 	break;
> #endif
> 
> I'm not sure if it will be worth the #if when vmid would be zero in
> DOM0 always.
> 
> Regards, Joonas
> 
> >  			req1 = cursor;
> >  			WARN_ON(req1->elsp_submitted);
> >  			break;
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification
  2016-06-03  9:40   ` Joonas Lahtinen
@ 2016-06-07 15:29     ` Wang, Zhi A
  2016-06-08  7:49       ` Joonas Lahtinen
  0 siblings, 1 reply; 24+ messages in thread
From: Wang, Zhi A @ 2016-06-07 15:29 UTC (permalink / raw)
  To: Joonas Lahtinen, intel-gfx, tvrtko.ursulin, Tian, Kevin, Lv,
	Zhiyuan, chris



> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Friday, June 03, 2016 12:40 PM
> To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org;
> tvrtko.ursulin@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; chris@chris-wilson.co.uk
> Subject: Re: [PATCH v6 7/9] drm/i915: Introduce execlist context status change
> notification
> 
> On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> > This patch introduces an approach to track the execlist context status
> > change.
> >
> > GVT-g uses GVT context as the "shadow context". The content inside GVT
> > context will be copied back to guest after the context is idle. So
> > GVT-g has to know the status of the execlist context.
> >
> > This function is configurable in the context creation service.
> > Currently, Only GVT-g will create the "status-change-notification"
> > enabled GEM context.
> >
> > v6:
> >
> > - When !CONFIG_DRM_I915_GVT, make GVT code as dead code then
> compiler
> > could automatically eliminate them for us. (Chris)
> > - Always initialize the notifier header, so it could be switched
> > on/off at runtime. (Chris)
> >
> > v5:
> >
> > - Only compile this feature when CONFIG_DRM_I915_GVT is
> > enabled.(Tvrtko)
> >
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
> >  drivers/gpu/drm/i915/intel_lrc.c | 24 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_lrc.h |  5 +++++
> >  3 files changed, 31 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index f6cb60a..dee72d3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -877,9 +877,11 @@ struct i915_gem_context {
> >  		u64 lrc_desc;
> >  		int pin_count;
> >  		bool initialised;
> > +		struct atomic_notifier_head status_notifier;
> 
> I think this could be outside engine block, just one per context.
> 
> >  	} engine[I915_NUM_ENGINES];
> >  	u32 lrc_ring_buffer_size;
> >  	u32 lrc_addressing_mode_bits;
> > +	bool enable_lrc_status_change_notification;
> >
> >  	struct list_head link;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index ffb436c..96d20c8 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -404,6 +404,22 @@ static void execlists_submit_requests(struct
> drm_i915_gem_request *rq0,
> >  	spin_unlock_irq(&dev_priv->uncore.lock);
> >  }
> >
> > +static inline void execlists_context_status_change(
> > +		struct drm_i915_gem_request *rq,
> > +		unsigned long status)
> > +{
> > +	/* The compiler should be fine with the dead-code elimination */
> 
> The comment could rather be "Currently, only GVT-g code uses status
> notifications" :)
> 
> > +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> > +		return;
> > +
> > +	if (!rq->ctx->enable_lrc_status_change_notification)
> > +		return;
> > +
> 
> I think above line should be dropped, just don't register notifier for context that
> does not want notifications, and if the chain is empty, no call is made.
> 
I keep this in v7 as I think a "if" here is much cheaper than the rcu stuff in atomic_notifier_call_chain() with a lot of "if" even when the chain is empty. :)
> > +	atomic_notifier_call_chain(
> > +			&rq->ctx->engine[rq->engine->id].status_notifier,
> > +			status, rq);
> > +}
> > +
> >  static void execlists_context_unqueue(struct intel_engine_cs *engine)
> >  {
> >  	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL; @@ -439,6
> > +455,11 @@ static void execlists_context_unqueue(struct intel_engine_cs
> *engine)
> >  	if (unlikely(!req0))
> >  		return;
> >
> > +	execlists_context_status_change(req0, CONTEXT_SCHEDULE_IN);
> > +
> > +	if (req1)
> > +		execlists_context_status_change(req1, CONTEXT_SCHEDULE_IN);
> > +
> >  	if (req0->elsp_submitted & engine->idle_lite_restore_wa) {
> >  		/*
> >  		 * WaIdleLiteRestore: make sure we never cause a lite restore @@
> > -477,6 +498,8 @@ execlists_check_remove_request(struct intel_engine_cs
> *engine, u32 ctx_id)
> >  	if (--head_req->elsp_submitted > 0)
> >  		return 0;
> >
> > +	execlists_context_status_change(head_req, CONTEXT_SCHEDULE_OUT);
> > +
> >  	list_del(&head_req->execlist_link);
> >  	i915_gem_request_unreference(head_req);
> >
> > @@ -2489,6 +2512,7 @@ static int execlists_context_deferred_alloc(struct
> i915_gem_context *ctx,
> >  	ce->ringbuf = ringbuf;
> >  	ce->state = ctx_obj;
> >  	ce->initialised = engine->init_context == NULL;
> > +	ATOMIC_INIT_NOTIFIER_HEAD(&ce->status_notifier);
> >
> >  	return 0;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> > b/drivers/gpu/drm/i915/intel_lrc.h
> > index e133c33..7a7ae8d 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.h
> > +++ b/drivers/gpu/drm/i915/intel_lrc.h
> > @@ -68,6 +68,11 @@ enum {
> >  #define GEN8_CSB_READ_PTR(csb_status) \
> >  	(((csb_status) & GEN8_CSB_READ_PTR_MASK) >> 8)
> >
> > +enum {
> > +	CONTEXT_SCHEDULE_IN = 0,
> > +	CONTEXT_SCHEDULE_OUT,
> > +};
> > +
> 
> Again, prefixes? At least INTEL_
> 
> With above;
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Regards, Joonas
> 
> >  /* Logical Rings */
> >  int intel_logical_ring_alloc_request_extras(struct
> > drm_i915_gem_request *request);
> >  int intel_logical_ring_reserve_space(struct drm_i915_gem_request
> > *request);
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification
  2016-06-07 15:29     ` Wang, Zhi A
@ 2016-06-08  7:49       ` Joonas Lahtinen
  0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  7:49 UTC (permalink / raw)
  To: Wang, Zhi A, intel-gfx, tvrtko.ursulin, Tian, Kevin, Lv, Zhiyuan, chris

On ti, 2016-06-07 at 15:29 +0000, Wang, Zhi A wrote:
> 
> > 
> > -----Original Message-----
> > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > Sent: Friday, June 03, 2016 12:40 PM
> > To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org;
> > tvrtko.ursulin@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan
> > <zhiyuan.lv@intel.com>; chris@chris-wilson.co.uk
> > Subject: Re: [PATCH v6 7/9] drm/i915: Introduce execlist context status change
> > notification
> > 
> > On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> > > 
> > > +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> > > +		return;
> > > +
> > > +	if (!rq->ctx->enable_lrc_status_change_notification)
> > > +		return;
> > > +
> > I think above line should be dropped, just don't register notifier for context that
> > does not want notifications, and if the chain is empty, no call is made.
> > 
> I keep this in v7 as I think a "if" here is much cheaper than the rcu stuff in atomic_notifier_call_chain() with a lot of "if" even when the chain is empty. :)

If empty notifier call chain is too heavy, then we should improve it
instead of going around. Do we have some benchmarks on the difference?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-06-08  7:50 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-02 16:36 [PATCH v6 0/9] Introduce the implementation of GVT context Zhi Wang
2016-06-02 16:36 ` [PATCH v6 1/9] drm/i915: Factor out i915_pvinfo.h Zhi Wang
2016-06-03  8:45   ` Joonas Lahtinen
2016-06-02 16:36 ` [PATCH v6 2/9] drm/i915: Fold vGPU active check into inner functions Zhi Wang
2016-06-03  8:49   ` Joonas Lahtinen
2016-06-02 16:36 ` [PATCH v6 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
2016-06-03  9:14   ` Joonas Lahtinen
2016-06-02 16:36 ` [PATCH v6 4/9] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
2016-06-03  9:17   ` Joonas Lahtinen
2016-06-02 16:36 ` [PATCH v6 5/9] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
2016-06-03  9:20   ` Joonas Lahtinen
2016-06-02 16:36 ` [PATCH v6 6/9] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
2016-06-03  9:25   ` Joonas Lahtinen
2016-06-02 16:36 ` [PATCH v6 7/9] drm/i915: Introduce execlist context status change notification Zhi Wang
2016-06-03  9:40   ` Joonas Lahtinen
2016-06-07 15:29     ` Wang, Zhi A
2016-06-08  7:49       ` Joonas Lahtinen
2016-06-02 16:36 ` [PATCH v6 8/9] drm/i915: Support LRC context single submission Zhi Wang
2016-06-03  9:47   ` Joonas Lahtinen
2016-06-03 11:25     ` Tian, Kevin
2016-06-07 14:13     ` Wang, Zhi A
2016-06-02 16:36 ` [PATCH v6 9/9] drm/i915: Introduce GVT context creation API Zhi Wang
2016-06-03  9:59   ` Joonas Lahtinen
2016-06-03  6:43 ` ✗ Ro.CI.BAT: warning for Introduce the implementation of GVT context (rev4) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.