* [PATCH 01/15] drm/i915: Factor out i915_pvinfo.h
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-15 17:32 ` [PATCH 02/15] drm/i915/gvt: Fold vGPU active check into inner functions Zhi Wang
` (14 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
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 | 115 +++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_vgpu.h | 86 +--------------------------
2 files changed, 116 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..6eff6d9
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -0,0 +1,115 @@
+/*
+ * 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_
+
+/* 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] 31+ messages in thread* [PATCH 02/15] drm/i915/gvt: Fold vGPU active check into inner functions
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
2016-05-15 17:32 ` [PATCH 01/15] drm/i915: Factor out i915_pvinfo.h Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 10:49 ` Tvrtko Ursulin
2016-05-15 17:32 ` [PATCH 03/15] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
` (13 subsequent siblings)
15 siblings, 1 reply; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
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 | 10 ++++++++--
drivers/gpu/drm/i915/i915_vgpu.h | 2 +-
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7eab619..5c02c5a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2737,11 +2737,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);
+ if (ret)
+ return ret;
if (!HAS_LLC(dev))
ggtt->base.mm.color_adjust = i915_gtt_color_adjust;
@@ -2841,8 +2839,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);
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..0b76c70 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_device *dev)
{
int i;
+ if (!intel_vgpu_active(to_i915(dev)))
+ return;
+
DRM_DEBUG("VGT deballoon.\n");
for (i = 0; i < 4; i++) {
@@ -187,6 +190,9 @@ int intel_vgt_balloon(struct drm_device *dev)
unsigned long unmappable_base, unmappable_size, unmappable_end;
int ret;
+ if (!intel_vgpu_active(to_i915(dev)))
+ 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 +264,6 @@ int intel_vgt_balloon(struct drm_device *dev)
err:
DRM_ERROR("VGT balloon fail\n");
- intel_vgt_deballoon();
+ intel_vgt_deballoon(dev);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 07e67d5..c0e9569 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -28,6 +28,6 @@
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 void intel_vgt_deballoon(struct drm_device *dev);
#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] 31+ messages in thread* Re: [PATCH 02/15] drm/i915/gvt: Fold vGPU active check into inner functions
2016-05-15 17:32 ` [PATCH 02/15] drm/i915/gvt: Fold vGPU active check into inner functions Zhi Wang
@ 2016-05-16 10:49 ` Tvrtko Ursulin
2016-05-16 13:56 ` Wang, Zhi A
0 siblings, 1 reply; 31+ messages in thread
From: Tvrtko Ursulin @ 2016-05-16 10:49 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
Hi,
On 15/05/16 18:32, Zhi Wang wrote:
> 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 | 10 ++++++++--
> drivers/gpu/drm/i915/i915_vgpu.h | 2 +-
> 3 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7eab619..5c02c5a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2737,11 +2737,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);
> + if (ret)
> + return ret;
>
> if (!HAS_LLC(dev))
> ggtt->base.mm.color_adjust = i915_gtt_color_adjust;
> @@ -2841,8 +2839,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);
>
> 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..0b76c70 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_device *dev)
In the spirit of current driver cleanup you should make
intel_vgt_balloon and deballoon take dev_priv. The former does not use
dev for anything anyway, just to get to dev_priv.
> {
> int i;
>
> + if (!intel_vgpu_active(to_i915(dev)))
> + return;
> +
> DRM_DEBUG("VGT deballoon.\n");
>
> for (i = 0; i < 4; i++) {
> @@ -187,6 +190,9 @@ int intel_vgt_balloon(struct drm_device *dev)
> unsigned long unmappable_base, unmappable_size, unmappable_end;
> int ret;
>
> + if (!intel_vgpu_active(to_i915(dev)))
> + return 0;
Even already has the dev_priv local variable.
> +
> 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 +264,6 @@ int intel_vgt_balloon(struct drm_device *dev)
>
> err:
> DRM_ERROR("VGT balloon fail\n");
> - intel_vgt_deballoon();
> + intel_vgt_deballoon(dev);
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index 07e67d5..c0e9569 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -28,6 +28,6 @@
>
> 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 void intel_vgt_deballoon(struct drm_device *dev);
>
> #endif /* _I915_VGPU_H_ */
>
The rest looks good to me.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 02/15] drm/i915/gvt: Fold vGPU active check into inner functions
2016-05-16 10:49 ` Tvrtko Ursulin
@ 2016-05-16 13:56 ` Wang, Zhi A
0 siblings, 0 replies; 31+ messages in thread
From: Wang, Zhi A @ 2016-05-16 13:56 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx@lists.freedesktop.org, Gordon, David S,
joonas.lahtinen@linux.intel.com, Tian, Kevin, Lv, Zhiyuan
Thanks for the comments. :)
-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
Sent: Monday, May 16, 2016 3:50 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [Intel-gfx] [PATCH 02/15] drm/i915/gvt: Fold vGPU active check into inner functions
Hi,
On 15/05/16 18:32, Zhi Wang wrote:
> 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 | 10 ++++++++--
> drivers/gpu/drm/i915/i915_vgpu.h | 2 +-
> 3 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7eab619..5c02c5a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2737,11 +2737,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);
> + if (ret)
> + return ret;
>
> if (!HAS_LLC(dev))
> ggtt->base.mm.color_adjust = i915_gtt_color_adjust; @@ -2841,8
> +2839,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);
>
> 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..0b76c70 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_device *dev)
In the spirit of current driver cleanup you should make intel_vgt_balloon and deballoon take dev_priv. The former does not use dev for anything anyway, just to get to dev_priv.
> {
> int i;
>
> + if (!intel_vgpu_active(to_i915(dev)))
> + return;
> +
> DRM_DEBUG("VGT deballoon.\n");
>
> for (i = 0; i < 4; i++) {
> @@ -187,6 +190,9 @@ int intel_vgt_balloon(struct drm_device *dev)
> unsigned long unmappable_base, unmappable_size, unmappable_end;
> int ret;
>
> + if (!intel_vgpu_active(to_i915(dev)))
> + return 0;
Even already has the dev_priv local variable.
> +
> 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 +264,6 @@ int intel_vgt_balloon(struct drm_device *dev)
>
> err:
> DRM_ERROR("VGT balloon fail\n");
> - intel_vgt_deballoon();
> + intel_vgt_deballoon(dev);
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h
> b/drivers/gpu/drm/i915/i915_vgpu.h
> index 07e67d5..c0e9569 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -28,6 +28,6 @@
>
> 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 void intel_vgt_deballoon(struct drm_device *dev);
>
> #endif /* _I915_VGPU_H_ */
>
The rest looks good to me.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 03/15] drm/i915: gvt: Introduce the basic architecture of GVT-g
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
2016-05-15 17:32 ` [PATCH 01/15] drm/i915: Factor out i915_pvinfo.h Zhi Wang
2016-05-15 17:32 ` [PATCH 02/15] drm/i915/gvt: Fold vGPU active check into inner functions Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 12:03 ` Tvrtko Ursulin
2016-05-15 17:32 ` [PATCH 04/15] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
` (12 subsequent siblings)
15 siblings, 1 reply; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
This patch introduces the very basic framework of GVT-g device model,
includes basic prototypes, definitions, 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 | 15 +++
drivers/gpu/drm/i915/Makefile | 5 +
drivers/gpu/drm/i915/gvt/Makefile | 5 +
drivers/gpu/drm/i915/gvt/debug.h | 36 ++++++
drivers/gpu/drm/i915/gvt/gvt.c | 209 +++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/gvt/gvt.h | 85 ++++++++++++++
drivers/gpu/drm/i915/gvt/hypercall.h | 38 +++++++
drivers/gpu/drm/i915/gvt/mpt.h | 51 +++++++++
drivers/gpu/drm/i915/i915_dma.c | 17 ++-
drivers/gpu/drm/i915/i915_drv.h | 12 ++
drivers/gpu/drm/i915/intel_gvt.c | 106 ++++++++++++++++++
drivers/gpu/drm/i915/intel_gvt.h | 49 ++++++++
include/drm/i915_gvt.h | 31 ++++++
13 files changed, 655 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
create mode 100644 include/drm/i915_gvt.h
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 29a32b1..782c97c 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -57,6 +57,21 @@ config DRM_I915_USERPTR
If in doubt, say "Y".
+config DRM_I915_GVT
+ bool "Intel GVT-g host driver"
+ depends on DRM_I915
+ default n
+ help
+ Enabling GVT-g mediated graphics passthrough technique for Intel i915
+ based integrated graphics card. With GVT-g, it's possible to have one
+ integrated i915 device shared by multiple VMs. Performance critical
+ opterations such as apperture accesses and ring buffer operations
+ are pass-throughed to VM, with a minimal set of conflicting resources
+ (e.g. display settings) mediated by vGT driver. The benefit of vGT
+ is on both the performance, given that each VM could directly operate
+ its aperture space and submit commands like running on native, and
+ the feature completeness, given that a true GEN hardware is exposed.
+
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 63c4d2b..e48145b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -103,6 +103,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..5b067d2
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/debug.h
@@ -0,0 +1,36 @@
+/*
+ * 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_info(fmt, args...) \
+ DRM_INFO("gvt: "fmt, ##args)
+
+#define gvt_err(fmt, args...) \
+ DRM_ERROR("gvt: "fmt, ##args)
+
+#define gvt_dbg_core(fmt, args...) \
+ DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
+
+#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..72ca301
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -0,0 +1,209 @@
+/*
+ * 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))
+
+/*
+ * The layout of BAR0 in BDW:
+ * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
+ *
+ * GTT offset in BAR0 starts from 8MB to 16MB, and
+ * Whatever GTT size is configured in BIOS,
+ * the size of BAR0 is always 16MB. The actual configured
+ * GTT size can be found in GMCH_CTRL.
+ */
+static struct intel_gvt_device_info broadwell_device_info = {
+ .max_gtt_gm_sz = GB(4), /* 4GB */
+ .gtt_start_offset = MB(8), /* 8MB */
+ .max_gtt_size = MB(8), /* 8MB */
+ .gtt_entry_size = 8,
+ .gtt_entry_size_shift = 3,
+ .gmadr_bytes_in_cmd = 8,
+ .mmio_size = MB(2), /* 2MB */
+ .mmio_bar = 0, /* BAR 0 */
+ .max_support_vgpu = 8,
+ .max_surface_size = MB(36),
+};
+
+static int init_gvt_host(void)
+{
+ if (WARN(intel_gvt_host.initialized,
+ "Intel GVT host has been initialized\n"))
+ return -EINVAL;
+
+ /* Xen DOM U */
+ if (xen_domain() && !xen_initial_domain())
+ return -ENODEV;
+
+ if (xen_initial_domain()) {
+ /* 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;
+ }
+
+ if (!intel_gvt_host.mpt) {
+ gvt_err("Fail to load any MPT modules.\n");
+ return -EINVAL;
+ }
+
+ if (!intel_gvt_hypervisor_detect_host())
+ return -ENODEV;
+
+ gvt_info("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 int init_device_info(struct intel_gvt *gvt)
+{
+ if (IS_BROADWELL(gvt->dev_priv))
+ gvt->device_info = &broadwell_device_info;
+ else
+ return -ENODEV;
+
+ return 0;
+}
+
+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 = NULL;
+ int ret;
+
+ 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_device: gvt device
+ *
+ * This function is called at the driver unloading stage, to destroy a
+ * GVT device and free the related resources.
+ *
+ * Returns:
+ * None
+ */
+void intel_gvt_destroy_device(void *gvt_device)
+{
+ struct intel_gvt *gvt = (struct intel_gvt *)gvt_device;
+
+ free_gvt_device(gvt);
+}
+
+/**
+ * intel_gvt_create_device - create a GVT device
+ * @dev: drm device
+ *
+ * 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.
+ */
+void *intel_gvt_create_device(void *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_gvt *gvt = NULL;
+ int ret;
+
+ if (!intel_gvt_host.initialized) {
+ ret = init_gvt_host();
+ if (ret)
+ return ERR_PTR(ret);
+ }
+
+ gvt_dbg_core("create new gvt device, i915 dev_priv: %p\n", dev_priv);
+
+ 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);
+
+ ret = init_device_info(gvt);
+ if (ret)
+ goto out_free_gvt_device;
+
+ gvt_dbg_core("gvt device creation done, id %d\n", gvt->id);
+
+ return gvt;
+
+out_free_gvt_device:
+ free_gvt_device(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..5ef9e1b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -0,0 +1,85 @@
+/*
+ * 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_vgpu.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 the limitation of HW.*/
+struct intel_gvt_device_info {
+ u64 max_gtt_gm_sz;
+ u32 gtt_start_offset;
+ u32 gtt_end_offset;
+ u32 max_gtt_size;
+ u32 gtt_entry_size;
+ u32 gtt_entry_size_shift;
+ u32 gmadr_bytes_in_cmd;
+ u32 mmio_size;
+ u32 mmio_bar;
+ u32 max_support_vgpu;
+ u32 max_surface_size;
+};
+
+struct intel_vgpu {
+ struct intel_gvt *gvt;
+ int id;
+ int vm_id;
+ bool warn_untrack;
+};
+
+struct intel_gvt {
+ struct mutex lock;
+ int id;
+
+ struct drm_i915_private *dev_priv;
+ struct idr vgpu_idr;
+
+ 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..d3f23cc
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -0,0 +1,51 @@
+/*
+ * 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)
+{
+ if (WARN_ON(!intel_gvt_host.mpt))
+ return -ENODEV;
+ 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 547100f..795a5cf 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>
@@ -1245,18 +1246,22 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
goto out_ggtt;
}
+ ret = intel_gvt_init(dev);
+ 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);
@@ -1267,7 +1272,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;
}
}
@@ -1297,7 +1302,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,
@@ -1330,6 +1335,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
return 0;
+out_gvt:
+ intel_gvt_cleanup(dev);
out_ggtt:
i915_ggtt_cleanup_hw(dev);
@@ -1488,6 +1495,8 @@ int i915_driver_unload(struct drm_device *dev)
intel_fbdev_fini(dev);
+ intel_gvt_cleanup(dev);
+
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 72f0b02..b256ba7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1703,6 +1703,10 @@ struct i915_workarounds {
u32 hw_whitelist_count[I915_NUM_ENGINES];
};
+struct i915_gvt {
+ void *gvt_device;
+};
+
struct i915_virtual_gpu {
bool active;
};
@@ -1742,6 +1746,8 @@ struct drm_i915_private {
struct i915_virtual_gpu vgpu;
+ struct i915_gvt gvt;
+
struct intel_guc guc;
struct intel_csr csr;
@@ -2868,6 +2874,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.gvt_device ? 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/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
new file mode 100644
index 0000000..57b4910
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -0,0 +1,106 @@
+/*
+ * 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.
+ */
+
+struct gvt_kernel_params gvt_kparams = {
+ .enable = false,
+};
+
+/* i915.gvt_enable */
+module_param_named(gvt_enable, gvt_kparams.enable, bool, 0600);
+MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
+
+static bool is_supported_device(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ if (IS_BROADWELL(dev_priv))
+ return true;
+
+ return false;
+}
+
+/**
+ * intel_gvt_init - initialize GVT components
+ * @dev: drm device *
+ *
+ * This function is called at the initialization stage to initialize the
+ * GVT components.
+ */
+int intel_gvt_init(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ void *device;
+
+ if (!gvt_kparams.enable) {
+ DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
+ return 0;
+ }
+
+ if (!is_supported_device(dev)) {
+ DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
+ return 0;
+ }
+
+ device = intel_gvt_create_device(dev);
+ if (IS_ERR(device)) {
+ DRM_DEBUG_DRIVER("GVT-g is disabled\n");
+ return 0;
+ }
+
+ dev_priv->gvt.gvt_device = device;
+ DRM_DEBUG_DRIVER("GVT-g is running in host mode\n");
+
+ return 0;
+}
+
+/**
+ * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
+ * @dev: drm device *
+ *
+ * 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_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ if (!intel_gvt_active(dev_priv))
+ return;
+
+ intel_gvt_destroy_device(dev_priv->gvt.gvt_device);
+ dev_priv->gvt.gvt_device = 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..d9b55ac50
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.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 _INTEL_GVT_H_
+#define _INTEL_GVT_H_
+
+#ifdef CONFIG_DRM_I915_GVT
+
+#include <drm/i915_gvt.h>
+
+struct gvt_kernel_params {
+ bool enable;
+};
+
+extern struct gvt_kernel_params gvt_kparams;
+
+extern int intel_gvt_init(struct drm_device *dev);
+extern void intel_gvt_cleanup(struct drm_device *dev);
+#else
+static inline int intel_gvt_init(struct drm_device *dev)
+{
+ return 0;
+}
+static inline void intel_gvt_cleanup(struct drm_device *dev)
+{
+}
+#endif
+
+#endif /* _INTEL_GVT_H_ */
diff --git a/include/drm/i915_gvt.h b/include/drm/i915_gvt.h
new file mode 100644
index 0000000..e126536
--- /dev/null
+++ b/include/drm/i915_gvt.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ * 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, sub license, 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_GVT_H
+#define _I915_GVT_H
+
+extern void *intel_gvt_create_device(void *dev);
+extern void intel_gvt_destroy_device(void *gvt_device);
+
+#endif /* _I915_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] 31+ messages in thread* Re: [PATCH 03/15] drm/i915: gvt: Introduce the basic architecture of GVT-g
2016-05-15 17:32 ` [PATCH 03/15] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
@ 2016-05-16 12:03 ` Tvrtko Ursulin
2016-05-17 2:55 ` Wang, Zhi A
0 siblings, 1 reply; 31+ messages in thread
From: Tvrtko Ursulin @ 2016-05-16 12:03 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
Hi,
Just a few comments from a non-assigned reviewer.
On 15/05/16 18:32, Zhi Wang wrote:
> This patch introduces the very basic framework of GVT-g device model,
> includes basic prototypes, definitions, 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 | 15 +++
> drivers/gpu/drm/i915/Makefile | 5 +
> drivers/gpu/drm/i915/gvt/Makefile | 5 +
> drivers/gpu/drm/i915/gvt/debug.h | 36 ++++++
> drivers/gpu/drm/i915/gvt/gvt.c | 209 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/gvt/gvt.h | 85 ++++++++++++++
> drivers/gpu/drm/i915/gvt/hypercall.h | 38 +++++++
> drivers/gpu/drm/i915/gvt/mpt.h | 51 +++++++++
> drivers/gpu/drm/i915/i915_dma.c | 17 ++-
> drivers/gpu/drm/i915/i915_drv.h | 12 ++
> drivers/gpu/drm/i915/intel_gvt.c | 106 ++++++++++++++++++
> drivers/gpu/drm/i915/intel_gvt.h | 49 ++++++++
> include/drm/i915_gvt.h | 31 ++++++
> 13 files changed, 655 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
> create mode 100644 include/drm/i915_gvt.h
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 29a32b1..782c97c 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -57,6 +57,21 @@ config DRM_I915_USERPTR
>
> If in doubt, say "Y".
>
> +config DRM_I915_GVT
> + bool "Intel GVT-g host driver"
> + depends on DRM_I915
> + default n
> + help
> + Enabling GVT-g mediated graphics passthrough technique for Intel i915
pass-through
> + based integrated graphics card. With GVT-g, it's possible to have one
> + integrated i915 device shared by multiple VMs. Performance critical
> + opterations such as apperture accesses and ring buffer operations
operations, aperture
> + are pass-throughed to VM, with a minimal set of conflicting resources
passed-through to the host or hypervisor ?
> + (e.g. display settings) mediated by vGT driver. The benefit of vGT
> + is on both the performance, given that each VM could directly operate
> + its aperture space and submit commands like running on native, and
> + the feature completeness, given that a true GEN hardware is exposed.
> +
> 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 63c4d2b..e48145b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -103,6 +103,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..5b067d2
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/debug.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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_info(fmt, args...) \
> + DRM_INFO("gvt: "fmt, ##args)
> +
> +#define gvt_err(fmt, args...) \
> + DRM_ERROR("gvt: "fmt, ##args)
> +
> +#define gvt_dbg_core(fmt, args...) \
> + DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> +
> +#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..72ca301
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -0,0 +1,209 @@
> +/*
> + * 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))
> +
> +/*
> + * The layout of BAR0 in BDW:
> + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
> + *
> + * GTT offset in BAR0 starts from 8MB to 16MB, and
> + * Whatever GTT size is configured in BIOS,
> + * the size of BAR0 is always 16MB. The actual configured
> + * GTT size can be found in GMCH_CTRL.
> + */
> +static struct intel_gvt_device_info broadwell_device_info = {
> + .max_gtt_gm_sz = GB(4), /* 4GB */
> + .gtt_start_offset = MB(8), /* 8MB */
> + .max_gtt_size = MB(8), /* 8MB */
> + .gtt_entry_size = 8,
> + .gtt_entry_size_shift = 3,
> + .gmadr_bytes_in_cmd = 8,
> + .mmio_size = MB(2), /* 2MB */
> + .mmio_bar = 0, /* BAR 0 */
> + .max_support_vgpu = 8,
> + .max_surface_size = MB(36),
> +};
> +
> +static int init_gvt_host(void)
> +{
> + if (WARN(intel_gvt_host.initialized,
> + "Intel GVT host has been initialized\n"))
Maybe add "already" for extra clarity?
> + return -EINVAL;
> +
> + /* Xen DOM U */
> + if (xen_domain() && !xen_initial_domain())
> + return -ENODEV;
> +
> + if (xen_initial_domain()) {
> + /* 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;
> + }
> +
> + if (!intel_gvt_host.mpt) {
> + gvt_err("Fail to load any MPT modules.\n");
> + return -EINVAL;
> + }
> +
> + if (!intel_gvt_hypervisor_detect_host())
> + return -ENODEV;
> +
> + gvt_info("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 int init_device_info(struct intel_gvt *gvt)
> +{
> + if (IS_BROADWELL(gvt->dev_priv))
> + gvt->device_info = &broadwell_device_info;
> + else
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +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 = NULL;
Don't need to initialize since it is assigned to unconditionally below.
> + int ret;
> +
> + gvt = vzalloc(sizeof(*gvt));
struct intel_gvt does not seem that large - why not cheaper kzalloc ?
> + 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_device: gvt device
> + *
> + * This function is called at the driver unloading stage, to destroy a
> + * GVT device and free the related resources.
> + *
> + * Returns:
> + * None
> + */
> +void intel_gvt_destroy_device(void *gvt_device)
> +{
> + struct intel_gvt *gvt = (struct intel_gvt *)gvt_device;
Hm, why does this function need to take a void * instead of the correct
type?
> +
> + free_gvt_device(gvt);
> +}
> +
> +/**
> + * intel_gvt_create_device - create a GVT device
> + * @dev: drm device
> + *
> + * 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.
> + */
> +void *intel_gvt_create_device(void *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_gvt *gvt = NULL;
No need to initialize.
> + int ret;
> +
> + if (!intel_gvt_host.initialized) {
> + ret = init_gvt_host();
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + gvt_dbg_core("create new gvt device, i915 dev_priv: %p\n", dev_priv);
Probably not that interesting to log dev_priv address ? Can't remember
every seeing any part of the driver doing it.
> +
> + 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);
> +
> + ret = init_device_info(gvt);
> + if (ret)
> + goto out_free_gvt_device;
There is some redundacy in supported platform checking between
init_device_info and is_supported_device. If you don't need both perhaps
try to simplify the code a bit by eliminating one of them?
> +
> + gvt_dbg_core("gvt device creation done, id %d\n", gvt->id);
> +
> + return gvt;
> +
> +out_free_gvt_device:
> + free_gvt_device(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..5ef9e1b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -0,0 +1,85 @@
> +/*
> + * 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_vgpu.h"
> +
> +#include "debug.h"
> +#include "hypercall.h"
> +
> +#define GVT_MAX_VGPU 8
> +#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) - 1)))
Same as existing kernel's ALIGN ?
> +
> +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 the limitation of HW.*/
> +struct intel_gvt_device_info {
> + u64 max_gtt_gm_sz;
> + u32 gtt_start_offset;
> + u32 gtt_end_offset;
> + u32 max_gtt_size;
> + u32 gtt_entry_size;
> + u32 gtt_entry_size_shift;
> + u32 gmadr_bytes_in_cmd;
> + u32 mmio_size;
> + u32 mmio_bar;
> + u32 max_support_vgpu;
> + u32 max_surface_size;
What surface is this?
Maybe add some comments for the fields?
> +};
> +
> +struct intel_vgpu {
> + struct intel_gvt *gvt;
> + int id;
> + int vm_id;
> + bool warn_untrack;
> +};
> +
> +struct intel_gvt {
> + struct mutex lock;
> + int id;
> +
> + struct drm_i915_private *dev_priv;
> + struct idr vgpu_idr;
> +
> + 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..d3f23cc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -0,0 +1,51 @@
> +/*
> + * 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)
> +{
> + if (WARN_ON(!intel_gvt_host.mpt))
> + return -ENODEV;
Is this condition impossible due the check in init_gvt_host ?
> + 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 547100f..795a5cf 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>
> @@ -1245,18 +1246,22 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> goto out_ggtt;
> }
>
> + ret = intel_gvt_init(dev);
> + 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);
> @@ -1267,7 +1272,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;
> }
> }
>
> @@ -1297,7 +1302,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,
> @@ -1330,6 +1335,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>
> return 0;
>
> +out_gvt:
> + intel_gvt_cleanup(dev);
> out_ggtt:
> i915_ggtt_cleanup_hw(dev);
>
> @@ -1488,6 +1495,8 @@ int i915_driver_unload(struct drm_device *dev)
>
> intel_fbdev_fini(dev);
>
> + intel_gvt_cleanup(dev);
> +
> 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 72f0b02..b256ba7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1703,6 +1703,10 @@ struct i915_workarounds {
> u32 hw_whitelist_count[I915_NUM_ENGINES];
> };
>
> +struct i915_gvt {
> + void *gvt_device;
Hm, again, why void * ? Will it be possible for this to hold some non
i915 pointer in the future?
> +};
> +
> struct i915_virtual_gpu {
> bool active;
> };
> @@ -1742,6 +1746,8 @@ struct drm_i915_private {
>
> struct i915_virtual_gpu vgpu;
>
> + struct i915_gvt gvt;
> +
> struct intel_guc guc;
>
> struct intel_csr csr;
> @@ -2868,6 +2874,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.gvt_device ? 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/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> new file mode 100644
> index 0000000..57b4910
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -0,0 +1,106 @@
> +/*
> + * 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.
> + */
> +
> +struct gvt_kernel_params gvt_kparams = {
> + .enable = false,
> +};
> +
> +/* i915.gvt_enable */
> +module_param_named(gvt_enable, gvt_kparams.enable, bool, 0600);
> +MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
> +
> +static bool is_supported_device(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (IS_BROADWELL(dev_priv))
> + return true;
> +
> + return false;
> +}
> +
> +/**
> + * intel_gvt_init - initialize GVT components
> + * @dev: drm device *
> + *
> + * This function is called at the initialization stage to initialize the
> + * GVT components.
> + */
> +int intel_gvt_init(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + void *device;
> +
> + if (!gvt_kparams.enable) {
> + DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
> + return 0;
> + }
> +
> + if (!is_supported_device(dev)) {
> + DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
> + return 0;
> + }
> +
> + device = intel_gvt_create_device(dev);
> + if (IS_ERR(device)) {
> + DRM_DEBUG_DRIVER("GVT-g is disabled\n");
> + return 0;
> + }
> +
> + dev_priv->gvt.gvt_device = device;
> + DRM_DEBUG_DRIVER("GVT-g is running in host mode\n");
Slightly redundant since init_gvt_host already would have logged a
gvt_info message to the same effect.
On the topic of which, perhaps code would be clearer if init_gvt_host
was called explicitly here from intel_gvt_init, and not behind the
scenes from intel_gvt_create_device ? Or is there a reason it has to be
like it is which I missed?
> +
> + return 0;
> +}
> +
> +/**
> + * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
> + * @dev: drm device *
> + *
> + * 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_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!intel_gvt_active(dev_priv))
> + return;
> +
> + intel_gvt_destroy_device(dev_priv->gvt.gvt_device);
> + dev_priv->gvt.gvt_device = 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..d9b55ac50
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.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 _INTEL_GVT_H_
> +#define _INTEL_GVT_H_
> +
> +#ifdef CONFIG_DRM_I915_GVT
> +
> +#include <drm/i915_gvt.h>
> +
> +struct gvt_kernel_params {
> + bool enable;
> +};
> +
> +extern struct gvt_kernel_params gvt_kparams;
> +
> +extern int intel_gvt_init(struct drm_device *dev);
> +extern void intel_gvt_cleanup(struct drm_device *dev);
> +#else
> +static inline int intel_gvt_init(struct drm_device *dev)
> +{
> + return 0;
> +}
> +static inline void intel_gvt_cleanup(struct drm_device *dev)
> +{
> +}
> +#endif
> +
> +#endif /* _INTEL_GVT_H_ */
> diff --git a/include/drm/i915_gvt.h b/include/drm/i915_gvt.h
> new file mode 100644
> index 0000000..e126536
> --- /dev/null
> +++ b/include/drm/i915_gvt.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + * 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, sub license, 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_GVT_H
> +#define _I915_GVT_H
> +
> +extern void *intel_gvt_create_device(void *dev);
> +extern void intel_gvt_destroy_device(void *gvt_device);
> +
> +#endif /* _I915_GVT_H */
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 03/15] drm/i915: gvt: Introduce the basic architecture of GVT-g
2016-05-16 12:03 ` Tvrtko Ursulin
@ 2016-05-17 2:55 ` Wang, Zhi A
0 siblings, 0 replies; 31+ messages in thread
From: Wang, Zhi A @ 2016-05-17 2:55 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx@lists.freedesktop.org, Gordon, David S,
joonas.lahtinen@linux.intel.com, Tian, Kevin, Lv, Zhiyuan
-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
Sent: Monday, May 16, 2016 5:03 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [Intel-gfx] [PATCH 03/15] drm/i915: gvt: Introduce the basic architecture of GVT-g
Hi,
Just a few comments from a non-assigned reviewer.
On 15/05/16 18:32, Zhi Wang wrote:
> This patch introduces the very basic framework of GVT-g device model,
> includes basic prototypes, definitions, 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 | 15 +++
> drivers/gpu/drm/i915/Makefile | 5 +
> drivers/gpu/drm/i915/gvt/Makefile | 5 +
> drivers/gpu/drm/i915/gvt/debug.h | 36 ++++++
> drivers/gpu/drm/i915/gvt/gvt.c | 209 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/gvt/gvt.h | 85 ++++++++++++++
> drivers/gpu/drm/i915/gvt/hypercall.h | 38 +++++++
> drivers/gpu/drm/i915/gvt/mpt.h | 51 +++++++++
> drivers/gpu/drm/i915/i915_dma.c | 17 ++-
> drivers/gpu/drm/i915/i915_drv.h | 12 ++
> drivers/gpu/drm/i915/intel_gvt.c | 106 ++++++++++++++++++
> drivers/gpu/drm/i915/intel_gvt.h | 49 ++++++++
> include/drm/i915_gvt.h | 31 ++++++
> 13 files changed, 655 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
> create mode 100644 include/drm/i915_gvt.h
>
> diff --git a/drivers/gpu/drm/i915/Kconfig
> b/drivers/gpu/drm/i915/Kconfig index 29a32b1..782c97c 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -57,6 +57,21 @@ config DRM_I915_USERPTR
>
> If in doubt, say "Y".
>
> +config DRM_I915_GVT
> + bool "Intel GVT-g host driver"
> + depends on DRM_I915
> + default n
> + help
> + Enabling GVT-g mediated graphics passthrough technique for
> +Intel i915
pass-through
> + based integrated graphics card. With GVT-g, it's possible to have one
> + integrated i915 device shared by multiple VMs. Performance critical
> + opterations such as apperture accesses and ring buffer
> + operations
operations, aperture
> + are pass-throughed to VM, with a minimal set of conflicting
> + resources
passed-through to the host or hypervisor ?
> + (e.g. display settings) mediated by vGT driver. The benefit of vGT
> + is on both the performance, given that each VM could directly operate
> + its aperture space and submit commands like running on native, and
> + the feature completeness, given that a true GEN hardware is exposed.
> +
> 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 63c4d2b..e48145b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -103,6 +103,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..5b067d2
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/debug.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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_info(fmt, args...) \
> + DRM_INFO("gvt: "fmt, ##args)
> +
> +#define gvt_err(fmt, args...) \
> + DRM_ERROR("gvt: "fmt, ##args)
> +
> +#define gvt_dbg_core(fmt, args...) \
> + DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> +
> +#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..72ca301
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -0,0 +1,209 @@
> +/*
> + * 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))
> +
> +/*
> + * The layout of BAR0 in BDW:
> + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
> + *
> + * GTT offset in BAR0 starts from 8MB to 16MB, and
> + * Whatever GTT size is configured in BIOS,
> + * the size of BAR0 is always 16MB. The actual configured
> + * GTT size can be found in GMCH_CTRL.
> + */
> +static struct intel_gvt_device_info broadwell_device_info = {
> + .max_gtt_gm_sz = GB(4), /* 4GB */
> + .gtt_start_offset = MB(8), /* 8MB */
> + .max_gtt_size = MB(8), /* 8MB */
> + .gtt_entry_size = 8,
> + .gtt_entry_size_shift = 3,
> + .gmadr_bytes_in_cmd = 8,
> + .mmio_size = MB(2), /* 2MB */
> + .mmio_bar = 0, /* BAR 0 */
> + .max_support_vgpu = 8,
> + .max_surface_size = MB(36),
> +};
> +
> +static int init_gvt_host(void)
> +{
> + if (WARN(intel_gvt_host.initialized,
> + "Intel GVT host has been initialized\n"))
Maybe add "already" for extra clarity?
> + return -EINVAL;
> +
> + /* Xen DOM U */
> + if (xen_domain() && !xen_initial_domain())
> + return -ENODEV;
> +
> + if (xen_initial_domain()) {
> + /* 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;
> + }
> +
> + if (!intel_gvt_host.mpt) {
> + gvt_err("Fail to load any MPT modules.\n");
> + return -EINVAL;
> + }
> +
> + if (!intel_gvt_hypervisor_detect_host())
> + return -ENODEV;
> +
> + gvt_info("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 int init_device_info(struct intel_gvt *gvt) {
> + if (IS_BROADWELL(gvt->dev_priv))
> + gvt->device_info = &broadwell_device_info;
> + else
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +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 = NULL;
Don't need to initialize since it is assigned to unconditionally below.
> + int ret;
> +
> + gvt = vzalloc(sizeof(*gvt));
struct intel_gvt does not seem that large - why not cheaper kzalloc ?
As this is just a stub patch, in the following patches, this structure will become huge. So use vzalloc here.
> + 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_device: gvt device
> + *
> + * This function is called at the driver unloading stage, to destroy
> +a
> + * GVT device and free the related resources.
> + *
> + * Returns:
> + * None
> + */
> +void intel_gvt_destroy_device(void *gvt_device) {
> + struct intel_gvt *gvt = (struct intel_gvt *)gvt_device;
Hm, why does this function need to take a void * instead of the correct type?
I don't want i915 to include gvt/gvt.h...
> +
> + free_gvt_device(gvt);
> +}
> +
> +/**
> + * intel_gvt_create_device - create a GVT device
> + * @dev: drm device
> + *
> + * 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.
> + */
> +void *intel_gvt_create_device(void *dev) {
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_gvt *gvt = NULL;
No need to initialize.
> + int ret;
> +
> + if (!intel_gvt_host.initialized) {
> + ret = init_gvt_host();
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + gvt_dbg_core("create new gvt device, i915 dev_priv: %p\n",
> +dev_priv);
Probably not that interesting to log dev_priv address ? Can't remember every seeing any part of the driver doing it.
Willl remove that
> +
> + 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);
> +
> + ret = init_device_info(gvt);
> + if (ret)
> + goto out_free_gvt_device;
There is some redundacy in supported platform checking between init_device_info and is_supported_device. If you don't need both perhaps try to simplify the code a bit by eliminating one of them?
Can we really remove platform check in init_device_info, anyway we have to attach different platform device info for different platform..
> +
> + gvt_dbg_core("gvt device creation done, id %d\n", gvt->id);
> +
> + return gvt;
> +
> +out_free_gvt_device:
> + free_gvt_device(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..5ef9e1b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -0,0 +1,85 @@
> +/*
> + * 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_vgpu.h"
> +
> +#include "debug.h"
> +#include "hypercall.h"
> +
> +#define GVT_MAX_VGPU 8
> +#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) -
> +1)))
Same as existing kernel's ALIGN ?
Nope, kernel ALIGN is a up-ALIGN, this is a down-ALIGN
> +
> +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 the limitation of HW.*/
> +struct intel_gvt_device_info {
> + u64 max_gtt_gm_sz;
> + u32 gtt_start_offset;
> + u32 gtt_end_offset;
> + u32 max_gtt_size;
> + u32 gtt_entry_size;
> + u32 gtt_entry_size_shift;
> + u32 gmadr_bytes_in_cmd;
> + u32 mmio_size;
> + u32 mmio_bar;
> + u32 max_support_vgpu;
> + u32 max_surface_size;
What surface is this?
Maybe add some comments for the fields?
Sure, will do.
> +};
> +
> +struct intel_vgpu {
> + struct intel_gvt *gvt;
> + int id;
> + int vm_id;
> + bool warn_untrack;
> +};
> +
> +struct intel_gvt {
> + struct mutex lock;
> + int id;
> +
> + struct drm_i915_private *dev_priv;
> + struct idr vgpu_idr;
> +
> + 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..d3f23cc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -0,0 +1,51 @@
> +/*
> + * 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)
> +{
> + if (WARN_ON(!intel_gvt_host.mpt))
> + return -ENODEV;
Is this condition impossible due the check in init_gvt_host ?
Will remove that.
> + 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 547100f..795a5cf 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>
> @@ -1245,18 +1246,22 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> goto out_ggtt;
> }
>
> + ret = intel_gvt_init(dev);
> + 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);
> @@ -1267,7 +1272,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;
> }
> }
>
> @@ -1297,7 +1302,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,
> @@ -1330,6 +1335,8 @@ static int i915_driver_init_hw(struct
> drm_i915_private *dev_priv)
>
> return 0;
>
> +out_gvt:
> + intel_gvt_cleanup(dev);
> out_ggtt:
> i915_ggtt_cleanup_hw(dev);
>
> @@ -1488,6 +1495,8 @@ int i915_driver_unload(struct drm_device *dev)
>
> intel_fbdev_fini(dev);
>
> + intel_gvt_cleanup(dev);
> +
> 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 72f0b02..b256ba7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1703,6 +1703,10 @@ struct i915_workarounds {
> u32 hw_whitelist_count[I915_NUM_ENGINES];
> };
>
> +struct i915_gvt {
> + void *gvt_device;
Hm, again, why void * ? Will it be possible for this to hold some non
i915 pointer in the future?
> +};
> +
> struct i915_virtual_gpu {
> bool active;
> };
> @@ -1742,6 +1746,8 @@ struct drm_i915_private {
>
> struct i915_virtual_gpu vgpu;
>
> + struct i915_gvt gvt;
> +
> struct intel_guc guc;
>
> struct intel_csr csr;
> @@ -2868,6 +2874,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.gvt_device ? 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/intel_gvt.c
> b/drivers/gpu/drm/i915/intel_gvt.c
> new file mode 100644
> index 0000000..57b4910
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -0,0 +1,106 @@
> +/*
> + * 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.
> + */
> +
> +struct gvt_kernel_params gvt_kparams = {
> + .enable = false,
> +};
> +
> +/* i915.gvt_enable */
> +module_param_named(gvt_enable, gvt_kparams.enable, bool, 0600);
> +MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
> +
> +static bool is_supported_device(struct drm_device *dev) {
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (IS_BROADWELL(dev_priv))
> + return true;
> +
> + return false;
> +}
> +
> +/**
> + * intel_gvt_init - initialize GVT components
> + * @dev: drm device *
> + *
> + * This function is called at the initialization stage to initialize
> +the
> + * GVT components.
> + */
> +int intel_gvt_init(struct drm_device *dev) {
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + void *device;
> +
> + if (!gvt_kparams.enable) {
> + DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
> + return 0;
> + }
> +
> + if (!is_supported_device(dev)) {
> + DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
> + return 0;
> + }
> +
> + device = intel_gvt_create_device(dev);
> + if (IS_ERR(device)) {
> + DRM_DEBUG_DRIVER("GVT-g is disabled\n");
> + return 0;
> + }
> +
> + dev_priv->gvt.gvt_device = device;
> + DRM_DEBUG_DRIVER("GVT-g is running in host mode\n");
Slightly redundant since init_gvt_host already would have logged a gvt_info message to the same effect.
On the topic of which, perhaps code would be clearer if init_gvt_host was called explicitly here from intel_gvt_init, and not behind the scenes from intel_gvt_create_device ? Or is there a reason it has to be like it is which I missed?
I thought init_gvt_host() is a part of GVT-g, it might be better to call them in create_device.
> +
> + return 0;
> +}
> +
> +/**
> + * intel_gvt_cleanup - cleanup GVT components when i915 driver is
> +unloading
> + * @dev: drm device *
> + *
> + * 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_device *dev) {
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!intel_gvt_active(dev_priv))
> + return;
> +
> + intel_gvt_destroy_device(dev_priv->gvt.gvt_device);
> + dev_priv->gvt.gvt_device = 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..d9b55ac50
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.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 _INTEL_GVT_H_
> +#define _INTEL_GVT_H_
> +
> +#ifdef CONFIG_DRM_I915_GVT
> +
> +#include <drm/i915_gvt.h>
> +
> +struct gvt_kernel_params {
> + bool enable;
> +};
> +
> +extern struct gvt_kernel_params gvt_kparams;
> +
> +extern int intel_gvt_init(struct drm_device *dev); extern void
> +intel_gvt_cleanup(struct drm_device *dev); #else static inline int
> +intel_gvt_init(struct drm_device *dev) {
> + return 0;
> +}
> +static inline void intel_gvt_cleanup(struct drm_device *dev) { }
> +#endif
> +
> +#endif /* _INTEL_GVT_H_ */
> diff --git a/include/drm/i915_gvt.h b/include/drm/i915_gvt.h new file
> mode 100644 index 0000000..e126536
> --- /dev/null
> +++ b/include/drm/i915_gvt.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + * 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, sub license, 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_GVT_H
> +#define _I915_GVT_H
> +
> +extern void *intel_gvt_create_device(void *dev); extern void
> +intel_gvt_destroy_device(void *gvt_device);
> +
> +#endif /* _I915_GVT_H */
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 04/15] drm/i915: Introduce host graphics memory partition for GVT-g
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (2 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 03/15] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-15 17:32 ` [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation Zhi Wang
` (11 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
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.
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_drv.h | 22 +++++++++++++++++++++
drivers/gpu/drm/i915/i915_vgpu.c | 21 +++++++++++++++-----
drivers/gpu/drm/i915/intel_gvt.c | 42 ++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_gvt.h | 2 ++
4 files changed, 82 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b256ba7..c4c72ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1703,8 +1703,30 @@ struct i915_workarounds {
u32 hw_whitelist_count[I915_NUM_ENGINES];
};
+/*
+ * 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 |
+ * +--------------------------------------------------------------+
+ */
struct i915_gvt {
void *gvt_device;
+ u64 low_gm_size;
+ u64 high_gm_size;
};
struct i915_virtual_gpu {
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 0b76c70..95e2a7a 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -190,13 +190,24 @@ int intel_vgt_balloon(struct drm_device *dev)
unsigned long unmappable_base, unmappable_size, unmappable_end;
int ret;
- if (!intel_vgpu_active(to_i915(dev)))
+ if (!intel_vgpu_active(dev_priv) && !intel_gvt_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));
- unmappable_size = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size));
+ if (intel_gvt_active(dev_priv)) {
+ mappable_base = 0;
+ mappable_size = dev_priv->gvt.low_gm_size;
+ unmappable_base = dev_priv->ggtt.mappable_end;
+ unmappable_size = dev_priv->gvt.high_gm_size;
+ } else if (intel_vgpu_active(dev_priv)) {
+ 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.c b/drivers/gpu/drm/i915/intel_gvt.c
index 57b4910..ab0d9ed 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -43,6 +43,46 @@ struct gvt_kernel_params gvt_kparams = {
module_param_named(gvt_enable, gvt_kparams.enable, bool, 0600);
MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
+/* i915.gvt_low_gm_size */
+module_param_named(gvt_low_gm_size, gvt_kparams.low_gm_size, charp, 0600);
+MODULE_PARM_DESC(gvt_low_gm_size, "GVT low graphics memory size");
+
+/* i915.gvt_high_gm_size */
+module_param_named(gvt_high_gm_size, gvt_kparams.high_gm_size, charp, 0600);
+MODULE_PARM_DESC(gvt_high_gm_size, "GVT high graphics memory size");
+
+#define KB(x) ((x) * 1024)
+#define MB(x) (KB(x) * 1024)
+
+#define MAX_GVT_LOW_GM_SIZE MB(96)
+#define MAX_GVT_HIGH_GM_SIZE MB(384)
+
+static void sanitize_gm_size(struct drm_i915_private *dev_priv)
+{
+ u64 low_gm_size, high_gm_size;
+
+ low_gm_size = high_gm_size = 0;
+
+ /* Try to parse kernel parameter first */
+ if (gvt_kparams.low_gm_size)
+ low_gm_size = memparse(gvt_kparams.low_gm_size, NULL);
+
+ if (gvt_kparams.high_gm_size)
+ high_gm_size = memparse(gvt_kparams.high_gm_size, NULL);
+
+ if (!low_gm_size || low_gm_size > MAX_GVT_LOW_GM_SIZE)
+ low_gm_size = MAX_GVT_LOW_GM_SIZE;
+
+ if (!high_gm_size || high_gm_size > MAX_GVT_HIGH_GM_SIZE)
+ high_gm_size = MAX_GVT_HIGH_GM_SIZE;
+
+ dev_priv->gvt.low_gm_size = low_gm_size;
+ dev_priv->gvt.high_gm_size = high_gm_size;
+
+ DRM_DEBUG_DRIVER("GVT low graphics memory size: %llx\n", low_gm_size);
+ DRM_DEBUG_DRIVER("GVT high graphics memory size: %llx\n", high_gm_size);
+}
+
static bool is_supported_device(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -75,6 +115,8 @@ int intel_gvt_init(struct drm_device *dev)
return 0;
}
+ sanitize_gm_size(dev_priv);
+
device = intel_gvt_create_device(dev);
if (IS_ERR(device)) {
DRM_DEBUG_DRIVER("GVT-g is disabled\n");
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index d9b55ac50..7644959 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -30,6 +30,8 @@
struct gvt_kernel_params {
bool enable;
+ char *low_gm_size;
+ char *high_gm_size;
};
extern struct gvt_kernel_params gvt_kparams;
--
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] 31+ messages in thread* [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (3 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 04/15] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 13:30 ` Tvrtko Ursulin
2016-05-15 17:32 ` [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT Zhi Wang
` (10 subsequent siblings)
15 siblings, 1 reply; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
Currently ctx->ppgtt would only be initialized when full PPGTT is used.
For aliasing PPGTT mode, ctx->ppgtt will be set when LRC context is
populated.
This patch moves the assignment into i915_gem_create_context() for better
code structure.
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 4 +++-
drivers/gpu/drm/i915/intel_lrc.c | 3 ---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2aedd18..21498e5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -300,6 +300,7 @@ static struct intel_context *
i915_gem_create_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
{
+ struct drm_i915_private *dev_priv = dev->dev_private;
const bool is_global_default_ctx = file_priv == NULL;
struct intel_context *ctx;
int ret = 0;
@@ -337,7 +338,8 @@ i915_gem_create_context(struct drm_device *dev,
}
ctx->ppgtt = ppgtt;
- }
+ } else
+ ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
trace_i915_context_create(ctx);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index db10c96..397fe65 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2286,9 +2286,6 @@ populate_lr_context(struct intel_context *ctx,
u32 *reg_state;
int ret;
- if (!ppgtt)
- ppgtt = dev_priv->mm.aliasing_ppgtt;
-
ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
if (ret) {
DRM_DEBUG_DRIVER("Could not set to CPU domain\n");
--
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] 31+ messages in thread* Re: [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
2016-05-15 17:32 ` [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation Zhi Wang
@ 2016-05-16 13:30 ` Tvrtko Ursulin
2016-05-16 14:16 ` Wang, Zhi A
2016-05-16 14:26 ` Wang, Zhi A
0 siblings, 2 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2016-05-16 13:30 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
On 15/05/16 18:32, Zhi Wang wrote:
> Currently ctx->ppgtt would only be initialized when full PPGTT is used.
> For aliasing PPGTT mode, ctx->ppgtt will be set when LRC context is
> populated.
>
> This patch moves the assignment into i915_gem_create_context() for better
> code structure.
Hm, it doesn't move the assignment it adds it.
Previously ctx->ppgtt was always NULL when !USES_FULL_PPGTT. Now it will
point to aliasing ppgtt.
Since there are various places in the code which do "if (ctx->ppgtt)"
(more or less) I think you need to explain why those will still work OK.
i915_gem_context_free for example?
Regards,
Tvrtko
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 4 +++-
> drivers/gpu/drm/i915/intel_lrc.c | 3 ---
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 2aedd18..21498e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -300,6 +300,7 @@ static struct intel_context *
> i915_gem_create_context(struct drm_device *dev,
> struct drm_i915_file_private *file_priv)
> {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> const bool is_global_default_ctx = file_priv == NULL;
> struct intel_context *ctx;
> int ret = 0;
> @@ -337,7 +338,8 @@ i915_gem_create_context(struct drm_device *dev,
> }
>
> ctx->ppgtt = ppgtt;
> - }
> + } else
> + ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
>
> trace_i915_context_create(ctx);
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index db10c96..397fe65 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2286,9 +2286,6 @@ populate_lr_context(struct intel_context *ctx,
> u32 *reg_state;
> int ret;
>
> - if (!ppgtt)
> - ppgtt = dev_priv->mm.aliasing_ppgtt;
> -
> ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
> if (ret) {
> DRM_DEBUG_DRIVER("Could not set to CPU domain\n");
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
2016-05-16 13:30 ` Tvrtko Ursulin
@ 2016-05-16 14:16 ` Wang, Zhi A
2016-05-16 14:26 ` Wang, Zhi A
1 sibling, 0 replies; 31+ messages in thread
From: Wang, Zhi A @ 2016-05-16 14:16 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx@lists.freedesktop.org, Gordon, David S,
joonas.lahtinen@linux.intel.com, Tian, Kevin, Lv, Zhiyuan
I understand what you say. Let me think about other approach. My requirement is to do nothing if a context doesn't have a ppgtt, but ctx->ppgtt == NULL means context is using aliasing ppgtt, so I need to find another way to achieve this.
-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
Sent: Monday, May 16, 2016 6:31 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [Intel-gfx] [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
On 15/05/16 18:32, Zhi Wang wrote:
> Currently ctx->ppgtt would only be initialized when full PPGTT is used.
> For aliasing PPGTT mode, ctx->ppgtt will be set when LRC context is
> populated.
>
> This patch moves the assignment into i915_gem_create_context() for
> better code structure.
Hm, it doesn't move the assignment it adds it.
Previously ctx->ppgtt was always NULL when !USES_FULL_PPGTT. Now it will point to aliasing ppgtt.
Since there are various places in the code which do "if (ctx->ppgtt)"
(more or less) I think you need to explain why those will still work OK.
i915_gem_context_free for example?
Regards,
Tvrtko
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 4 +++-
> drivers/gpu/drm/i915/intel_lrc.c | 3 ---
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 2aedd18..21498e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -300,6 +300,7 @@ static struct intel_context *
> i915_gem_create_context(struct drm_device *dev,
> struct drm_i915_file_private *file_priv)
> {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> const bool is_global_default_ctx = file_priv == NULL;
> struct intel_context *ctx;
> int ret = 0;
> @@ -337,7 +338,8 @@ i915_gem_create_context(struct drm_device *dev,
> }
>
> ctx->ppgtt = ppgtt;
> - }
> + } else
> + ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
>
> trace_i915_context_create(ctx);
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index db10c96..397fe65 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2286,9 +2286,6 @@ populate_lr_context(struct intel_context *ctx,
> u32 *reg_state;
> int ret;
>
> - if (!ppgtt)
> - ppgtt = dev_priv->mm.aliasing_ppgtt;
> -
> ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
> if (ret) {
> DRM_DEBUG_DRIVER("Could not set to CPU domain\n");
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
2016-05-16 13:30 ` Tvrtko Ursulin
2016-05-16 14:16 ` Wang, Zhi A
@ 2016-05-16 14:26 ` Wang, Zhi A
1 sibling, 0 replies; 31+ messages in thread
From: Wang, Zhi A @ 2016-05-16 14:26 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx@lists.freedesktop.org, Gordon, David S,
joonas.lahtinen@linux.intel.com, Tian, Kevin, Lv, Zhiyuan
Looks like I can drop this patch without harm. As I will load the root pointer myself. And there is patch to check if ppgtt == something then populate PDPs, so under FULL_PPGTT && !NO_PPGTT_CTX it won't load anything. Under !FULL_PPGTT && !NO_PPGTT_CTX, it will load aliasing_ppgtt_mm, but then I will load PDPs myself, so it's also OK.
-----Original Message-----
From: Wang, Zhi A
Sent: Monday, May 16, 2016 7:16 AM
To: 'Tvrtko Ursulin' <tvrtko.ursulin@linux.intel.com>; intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: RE: [Intel-gfx] [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
I understand what you say. Let me think about other approach. My requirement is to do nothing if a context doesn't have a ppgtt, but ctx->ppgtt == NULL means context is using aliasing ppgtt, so I need to find another way to achieve this.
-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
Sent: Monday, May 16, 2016 6:31 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [Intel-gfx] [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
On 15/05/16 18:32, Zhi Wang wrote:
> Currently ctx->ppgtt would only be initialized when full PPGTT is used.
> For aliasing PPGTT mode, ctx->ppgtt will be set when LRC context is
> populated.
>
> This patch moves the assignment into i915_gem_create_context() for
> better code structure.
Hm, it doesn't move the assignment it adds it.
Previously ctx->ppgtt was always NULL when !USES_FULL_PPGTT. Now it will point to aliasing ppgtt.
Since there are various places in the code which do "if (ctx->ppgtt)"
(more or less) I think you need to explain why those will still work OK.
i915_gem_context_free for example?
Regards,
Tvrtko
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 4 +++-
> drivers/gpu/drm/i915/intel_lrc.c | 3 ---
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 2aedd18..21498e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -300,6 +300,7 @@ static struct intel_context *
> i915_gem_create_context(struct drm_device *dev,
> struct drm_i915_file_private *file_priv)
> {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> const bool is_global_default_ctx = file_priv == NULL;
> struct intel_context *ctx;
> int ret = 0;
> @@ -337,7 +338,8 @@ i915_gem_create_context(struct drm_device *dev,
> }
>
> ctx->ppgtt = ppgtt;
> - }
> + } else
> + ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
>
> trace_i915_context_create(ctx);
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index db10c96..397fe65 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2286,9 +2286,6 @@ populate_lr_context(struct intel_context *ctx,
> u32 *reg_state;
> int ret;
>
> - if (!ppgtt)
> - ppgtt = dev_priv->mm.aliasing_ppgtt;
> -
> ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
> if (ret) {
> DRM_DEBUG_DRIVER("Could not set to CPU domain\n");
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (4 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 15:13 ` Chris Wilson
2016-05-15 17:32 ` [PATCH 07/15] drm/i915: Populate context PDPs if it has a PPGTT Zhi Wang
` (9 subsequent siblings)
15 siblings, 1 reply; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
GVT context will use its own shadow PPGTT, and it doesn't need a
i915_hw_ppgtt.
This patch adds a "has_ppgtt" param to i915_gem_context(), which
allows the caller to create a context without PPGTT
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 34 ++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 21498e5..b952e37 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -298,7 +298,8 @@ err_out:
*/
static struct intel_context *
i915_gem_create_context(struct drm_device *dev,
- struct drm_i915_file_private *file_priv)
+ struct drm_i915_file_private *file_priv,
+ bool has_ppgtt)
{
struct drm_i915_private *dev_priv = dev->dev_private;
const bool is_global_default_ctx = file_priv == NULL;
@@ -327,19 +328,22 @@ i915_gem_create_context(struct drm_device *dev,
}
}
- if (USES_FULL_PPGTT(dev)) {
- struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
+ if (has_ppgtt) {
+ if (USES_FULL_PPGTT(dev)) {
+ struct i915_hw_ppgtt *ppgtt =
+ i915_ppgtt_create(dev, file_priv);
- if (IS_ERR_OR_NULL(ppgtt)) {
- DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
- PTR_ERR(ppgtt));
- ret = PTR_ERR(ppgtt);
- goto err_unpin;
- }
+ if (IS_ERR_OR_NULL(ppgtt)) {
+ DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
+ PTR_ERR(ppgtt));
+ ret = PTR_ERR(ppgtt);
+ goto err_unpin;
+ }
- ctx->ppgtt = ppgtt;
- } else
- ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
+ ctx->ppgtt = ppgtt;
+ } else
+ ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
+ }
trace_i915_context_create(ctx);
@@ -416,7 +420,7 @@ int i915_gem_context_init(struct drm_device *dev)
}
}
- ctx = i915_gem_create_context(dev, NULL);
+ ctx = i915_gem_create_context(dev, NULL, true);
if (IS_ERR(ctx)) {
DRM_ERROR("Failed to create default global context (error %ld)\n",
PTR_ERR(ctx));
@@ -478,7 +482,7 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
idr_init(&file_priv->context_idr);
mutex_lock(&dev->struct_mutex);
- ctx = i915_gem_create_context(dev, file_priv);
+ ctx = i915_gem_create_context(dev, file_priv, true);
mutex_unlock(&dev->struct_mutex);
if (IS_ERR(ctx)) {
@@ -912,7 +916,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_create_context(dev, file_priv);
+ ctx = i915_gem_create_context(dev, file_priv, true);
mutex_unlock(&dev->struct_mutex);
if (IS_ERR(ctx))
return PTR_ERR(ctx);
--
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] 31+ messages in thread* Re: [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT
2016-05-15 17:32 ` [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT Zhi Wang
@ 2016-05-16 15:13 ` Chris Wilson
2016-05-16 15:18 ` Wang, Zhi A
0 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2016-05-16 15:13 UTC (permalink / raw)
To: Zhi Wang; +Cc: intel-gfx, zhiyuan.lv
On Mon, May 16, 2016 at 01:32:44AM +0800, Zhi Wang wrote:
> GVT context will use its own shadow PPGTT, and it doesn't need a
> i915_hw_ppgtt.
>
> This patch adds a "has_ppgtt" param to i915_gem_context(), which
> allows the caller to create a context without PPGTT
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 34 ++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 21498e5..b952e37 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -298,7 +298,8 @@ err_out:
> */
> static struct intel_context *
> i915_gem_create_context(struct drm_device *dev,
> - struct drm_i915_file_private *file_priv)
> + struct drm_i915_file_private *file_priv,
> + bool has_ppgtt)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> const bool is_global_default_ctx = file_priv == NULL;
> @@ -327,19 +328,22 @@ i915_gem_create_context(struct drm_device *dev,
> }
> }
>
> - if (USES_FULL_PPGTT(dev)) {
> - struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
> + if (has_ppgtt) {
> + if (USES_FULL_PPGTT(dev)) {
> + struct i915_hw_ppgtt *ppgtt =
> + i915_ppgtt_create(dev, file_priv);
>
> - if (IS_ERR_OR_NULL(ppgtt)) {
> - DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
> - PTR_ERR(ppgtt));
> - ret = PTR_ERR(ppgtt);
> - goto err_unpin;
> - }
> + if (IS_ERR_OR_NULL(ppgtt)) {
> + DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
> + PTR_ERR(ppgtt));
> + ret = PTR_ERR(ppgtt);
> + goto err_unpin;
> + }
>
> - ctx->ppgtt = ppgtt;
> - } else
> - ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
> + ctx->ppgtt = ppgtt;
> + } else
> + ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
You have to first go through the driver and update the sematics for
ctx->ppgtt == NULL. (Note in some instances you have to use the
ggtt pointer and not the appgtt, just a minor case in execbuf!).
Then tell us how you didn't spot the explosion when trying to use the
aliasing_ppgtt after freeing it.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT
2016-05-16 15:13 ` Chris Wilson
@ 2016-05-16 15:18 ` Wang, Zhi A
0 siblings, 0 replies; 31+ messages in thread
From: Wang, Zhi A @ 2016-05-16 15:18 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx@lists.freedesktop.org, Lv, Zhiyuan
Yes. Thanks! Seems there is an assumption if ctx->ppgtt == NULL then go back to use aliasing ppgtt. I see. I will drop that patch, without that patch we could also work
-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
Sent: Monday, May 16, 2016 8:13 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [Intel-gfx] [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT
On Mon, May 16, 2016 at 01:32:44AM +0800, Zhi Wang wrote:
> GVT context will use its own shadow PPGTT, and it doesn't need a
> i915_hw_ppgtt.
>
> This patch adds a "has_ppgtt" param to i915_gem_context(), which
> allows the caller to create a context without PPGTT
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 34
> ++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 21498e5..b952e37 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -298,7 +298,8 @@ err_out:
> */
> static struct intel_context *
> i915_gem_create_context(struct drm_device *dev,
> - struct drm_i915_file_private *file_priv)
> + struct drm_i915_file_private *file_priv,
> + bool has_ppgtt)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> const bool is_global_default_ctx = file_priv == NULL; @@ -327,19
> +328,22 @@ i915_gem_create_context(struct drm_device *dev,
> }
> }
>
> - if (USES_FULL_PPGTT(dev)) {
> - struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv);
> + if (has_ppgtt) {
> + if (USES_FULL_PPGTT(dev)) {
> + struct i915_hw_ppgtt *ppgtt =
> + i915_ppgtt_create(dev, file_priv);
>
> - if (IS_ERR_OR_NULL(ppgtt)) {
> - DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
> - PTR_ERR(ppgtt));
> - ret = PTR_ERR(ppgtt);
> - goto err_unpin;
> - }
> + if (IS_ERR_OR_NULL(ppgtt)) {
> + DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
> + PTR_ERR(ppgtt));
> + ret = PTR_ERR(ppgtt);
> + goto err_unpin;
> + }
>
> - ctx->ppgtt = ppgtt;
> - } else
> - ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
> + ctx->ppgtt = ppgtt;
> + } else
> + ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
You have to first go through the driver and update the sematics for
ctx->ppgtt == NULL. (Note in some instances you have to use the
ggtt pointer and not the appgtt, just a minor case in execbuf!).
Then tell us how you didn't spot the explosion when trying to use the aliasing_ppgtt after freeing it.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 07/15] drm/i915: Populate context PDPs if it has a PPGTT
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (5 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-15 17:32 ` [PATCH 08/15] drm/i915: Introduce an option for skipping engine context initialization Zhi Wang
` (8 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
As we allow contexts without PPGTT to be created, we should check if
a context has a PPGTT before populating PDPs from it.
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 397fe65..c8fbdfb 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2386,19 +2386,22 @@ populate_lr_context(struct intel_context *ctx,
ASSIGN_CTX_REG(reg_state, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(engine, 0),
0);
- if (USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
- /* 64b PPGTT (48bit canonical)
- * PDP0_DESCRIPTOR contains the base address to PML4 and
- * other PDP Descriptors are ignored.
- */
- ASSIGN_CTX_PML4(ppgtt, reg_state);
- } else {
- /* 32b PPGTT
- * PDP*_DESCRIPTOR contains the base address of space supported.
- * With dynamic page allocation, PDPs may not be allocated at
- * this point. Point the unallocated PDPs to the scratch page
- */
- execlists_update_context_pdps(ppgtt, reg_state);
+ if (ctx->ppgtt) {
+ if (USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
+ /* 64b PPGTT (48bit canonical)
+ * PDP0_DESCRIPTOR contains the base address to PML4 and
+ * other PDP Descriptors are ignored.
+ */
+ ASSIGN_CTX_PML4(ppgtt, reg_state);
+ } else {
+ /* 32b PPGTT
+ * PDP*_DESCRIPTOR contains the base address of space
+ * supported. With dynamic page allocation, PDPs may
+ * not be allocated at this point. Point the unallocated
+ * PDPs to the scratch page
+ */
+ execlists_update_context_pdps(ppgtt, reg_state);
+ }
}
if (engine->id == RCS) {
--
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] 31+ messages in thread* [PATCH 08/15] drm/i915: Introduce an option for skipping engine context initialization
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (6 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 07/15] drm/i915: Populate context PDPs if it has a PPGTT Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-15 17:32 ` [PATCH 09/15] drm/i915: Make ring buffer size configurable Zhi Wang
` (7 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
GVT-g will copy guest context into GVT LRC context before using it, so
engine context initialization is not necessary.
This patch introduces an option, and caller could configure it to choose
if the engine context initialization should be skipped.
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 | 9 ++++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c4c72ee..48f4d6e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -882,6 +882,7 @@ struct intel_context {
u64 lrc_desc;
uint32_t *lrc_reg_state;
bool initialised;
+ bool skip_init_context;
} engine[I915_NUM_ENGINES];
struct list_head link;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c8fbdfb..08eeaf3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2478,6 +2478,13 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
return ret;
}
+static bool engine_initialised(struct intel_context *ctx,
+ struct intel_engine_cs *engine)
+{
+ return engine->init_context == NULL
+ || ctx->engine[engine->id].skip_init_context;
+}
+
/**
* execlists_context_deferred_alloc() - create the LRC specific bits of a context
* @ctx: LR context to create.
@@ -2527,7 +2534,7 @@ static int execlists_context_deferred_alloc(struct intel_context *ctx,
ctx->engine[engine->id].ringbuf = ringbuf;
ctx->engine[engine->id].state = ctx_obj;
- ctx->engine[engine->id].initialised = engine->init_context == NULL;
+ ctx->engine[engine->id].initialised = engine_initialised(ctx, engine);
return 0;
--
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] 31+ messages in thread* [PATCH 09/15] drm/i915: Make ring buffer size configurable
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (7 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 08/15] drm/i915: Introduce an option for skipping engine context initialization Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-15 17:32 ` [PATCH 10/15] drm/i915: Generate addressing mode bit from flag in context Zhi Wang
` (6 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
This patch introduces an option for configuring ring buffer size during
context creation. If no ring buffer size is specified, the default size
(4 * PAGE_SIZE) will be used.
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 | 8 ++++++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48f4d6e..4ac88b2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -883,6 +883,7 @@ struct intel_context {
uint32_t *lrc_reg_state;
bool initialised;
bool skip_init_context;
+ u32 ring_buffer_size;
} engine[I915_NUM_ENGINES];
struct list_head link;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 08eeaf3..3f04784 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2502,7 +2502,7 @@ static int execlists_context_deferred_alloc(struct intel_context *ctx,
struct intel_engine_cs *engine)
{
struct drm_i915_gem_object *ctx_obj;
- uint32_t context_size;
+ uint32_t context_size, ring_buffer_size;
struct intel_ringbuffer *ringbuf;
int ret;
@@ -2520,7 +2520,11 @@ static int execlists_context_deferred_alloc(struct intel_context *ctx,
return PTR_ERR(ctx_obj);
}
- ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE);
+ ring_buffer_size = ctx->engine[engine->id].ring_buffer_size;
+ if (!ring_buffer_size)
+ ring_buffer_size = 4 * PAGE_SIZE;
+
+ ringbuf = intel_engine_create_ringbuffer(engine, 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] 31+ messages in thread* [PATCH 10/15] drm/i915: Generate addressing mode bit from flag in context.
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (8 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 09/15] drm/i915: Make ring buffer size configurable Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 13:47 ` Tvrtko Ursulin
2016-05-15 17:32 ` [PATCH 11/15] drm/i915: Introduce execlist context status change notification Zhi Wang
` (5 subsequent siblings)
15 siblings, 1 reply; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
Previously the addressing mode bit in context descriptor is generated from
context PPGTT. As we allow context could be used without PPGTT, and we
still need to know the addressing mode during context submission, a flag
is introduced.
And the addressing mode bit will be generated from this flag.
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 | 9 +++++----
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4ac88b2..7f050a3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -885,6 +885,7 @@ struct intel_context {
bool skip_init_context;
u32 ring_buffer_size;
} engine[I915_NUM_ENGINES];
+ bool use_48bit_addressing_mode;
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 b952e37..b5b0849 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -343,6 +343,8 @@ i915_gem_create_context(struct drm_device *dev,
ctx->ppgtt = ppgtt;
} else
ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
+
+ ctx->use_48bit_addressing_mode = USES_FULL_48BIT_PPGTT(dev);
}
trace_i915_context_create(ctx);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3f04784..0a96d4a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -214,7 +214,8 @@ enum {
LEGACY_64B_CONTEXT
};
#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
-#define GEN8_CTX_ADDRESSING_MODE(dev) (USES_FULL_48BIT_PPGTT(dev) ?\
+#define GEN8_CTX_ADDRESSING_MODE(ctx) \
+ (ctx->use_48bit_addressing_mode ? \
LEGACY_64B_CONTEXT :\
LEGACY_32B_CONTEXT)
enum {
@@ -281,8 +282,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;
@@ -325,8 +324,10 @@ intel_lr_context_descriptor_update(struct intel_context *ctx,
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
desc = engine->ctx_desc_template; /* bits 0-11 */
+ desc |= GEN8_CTX_ADDRESSING_MODE(ctx) << /* bits 3-4 */
+ GEN8_CTX_ADDRESSING_MODE_SHIFT;
desc |= ctx->engine[engine->id].lrc_vma->node.start + /* bits 12-31 */
- LRC_PPHWSP_PN * PAGE_SIZE;
+ LRC_PPHWSP_PN * PAGE_SIZE;
desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT; /* bits 32-52 */
ctx->engine[engine->id].lrc_desc = desc;
--
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] 31+ messages in thread* Re: [PATCH 10/15] drm/i915: Generate addressing mode bit from flag in context.
2016-05-15 17:32 ` [PATCH 10/15] drm/i915: Generate addressing mode bit from flag in context Zhi Wang
@ 2016-05-16 13:47 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2016-05-16 13:47 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
On 15/05/16 18:32, Zhi Wang wrote:
> Previously the addressing mode bit in context descriptor is generated from
> context PPGTT. As we allow context could be used without PPGTT, and we
> still need to know the addressing mode during context submission, a flag
> is introduced.
>
> And the addressing mode bit will be generated from this flag.
>
> 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 | 9 +++++----
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4ac88b2..7f050a3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -885,6 +885,7 @@ struct intel_context {
> bool skip_init_context;
> u32 ring_buffer_size;
> } engine[I915_NUM_ENGINES];
> + bool use_48bit_addressing_mode;
>
> 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 b952e37..b5b0849 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -343,6 +343,8 @@ i915_gem_create_context(struct drm_device *dev,
> ctx->ppgtt = ppgtt;
> } else
> ctx->ppgtt = dev_priv->mm.aliasing_ppgtt;
> +
> + ctx->use_48bit_addressing_mode = USES_FULL_48BIT_PPGTT(dev);
Should be dev_priv, you added it in an earlier patch. Could replace the
one usage of to_i915 in this function with it in that patch as well.
Regards,
Tvrtko
> }
>
> trace_i915_context_create(ctx);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 3f04784..0a96d4a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -214,7 +214,8 @@ enum {
> LEGACY_64B_CONTEXT
> };
> #define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
> -#define GEN8_CTX_ADDRESSING_MODE(dev) (USES_FULL_48BIT_PPGTT(dev) ?\
> +#define GEN8_CTX_ADDRESSING_MODE(ctx) \
> + (ctx->use_48bit_addressing_mode ? \
> LEGACY_64B_CONTEXT :\
> LEGACY_32B_CONTEXT)
> enum {
> @@ -281,8 +282,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;
> @@ -325,8 +324,10 @@ intel_lr_context_descriptor_update(struct intel_context *ctx,
> BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
>
> desc = engine->ctx_desc_template; /* bits 0-11 */
> + desc |= GEN8_CTX_ADDRESSING_MODE(ctx) << /* bits 3-4 */
> + GEN8_CTX_ADDRESSING_MODE_SHIFT;
> desc |= ctx->engine[engine->id].lrc_vma->node.start + /* bits 12-31 */
> - LRC_PPHWSP_PN * PAGE_SIZE;
> + LRC_PPHWSP_PN * PAGE_SIZE;
> desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT; /* bits 32-52 */
>
> ctx->engine[engine->id].lrc_desc = desc;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 11/15] drm/i915: Introduce execlist context status change notification
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (9 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 10/15] drm/i915: Generate addressing mode bit from flag in context Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 14:00 ` Tvrtko Ursulin
2016-05-15 17:32 ` [PATCH 12/15] drm/i915: Support context single submission Zhi Wang
` (4 subsequent siblings)
15 siblings, 1 reply; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
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.
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 5 +++++
drivers/gpu/drm/i915/intel_lrc.c | 23 +++++++++++++++++++++++
drivers/gpu/drm/i915/intel_lrc.h | 5 +++++
3 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7f050a3..1c9e865 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -884,6 +884,8 @@ struct intel_context {
bool initialised;
bool skip_init_context;
u32 ring_buffer_size;
+ bool enable_status_change_notification;
+ struct atomic_notifier_head status_notifier_head;
} engine[I915_NUM_ENGINES];
bool use_48bit_addressing_mode;
@@ -2403,6 +2405,9 @@ struct drm_i915_gem_request {
/** Execlists context hardware id. */
unsigned ctx_hw_id;
+
+ /** GVT workload **/
+ void *gvt_workload;
};
struct drm_i915_gem_request * __must_check
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0a96d4a..aeaea2e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -415,6 +415,19 @@ 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 *req,
+ unsigned long status)
+{
+ if (!req->ctx->engine[req->engine->id].
+ enable_status_change_notification)
+ return;
+
+ atomic_notifier_call_chain(
+ &req->ctx->engine[req->engine->id].status_notifier_head,
+ status, req->gvt_workload);
+}
+
static void execlists_context_unqueue(struct intel_engine_cs *engine)
{
struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
@@ -450,6 +463,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
@@ -488,6 +506,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);
@@ -2541,6 +2561,9 @@ static int execlists_context_deferred_alloc(struct intel_context *ctx,
ctx->engine[engine->id].state = ctx_obj;
ctx->engine[engine->id].initialised = engine_initialised(ctx, engine);
+ if (ctx->engine[engine->id].enable_status_change_notification)
+ ATOMIC_INIT_NOTIFIER_HEAD(
+ &ctx->engine[engine->id].status_notifier_head);
return 0;
error_ringbuf:
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 1afba03..98c94ee 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -57,6 +57,11 @@
#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] 31+ messages in thread* Re: [PATCH 11/15] drm/i915: Introduce execlist context status change notification
2016-05-15 17:32 ` [PATCH 11/15] drm/i915: Introduce execlist context status change notification Zhi Wang
@ 2016-05-16 14:00 ` Tvrtko Ursulin
2016-05-16 14:28 ` Wang, Zhi A
0 siblings, 1 reply; 31+ messages in thread
From: Tvrtko Ursulin @ 2016-05-16 14:00 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
On 15/05/16 18:32, 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.
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +++++
> drivers/gpu/drm/i915/intel_lrc.c | 23 +++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_lrc.h | 5 +++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7f050a3..1c9e865 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -884,6 +884,8 @@ struct intel_context {
> bool initialised;
> bool skip_init_context;
> u32 ring_buffer_size;
> + bool enable_status_change_notification;
This flag will be per context and not per context-engine, correct? In
which case it would be worth moving it to the context. Would make
execlists_context_status_change cheaper as well.
> + struct atomic_notifier_head status_notifier_head;
> } engine[I915_NUM_ENGINES];
> bool use_48bit_addressing_mode;
>
> @@ -2403,6 +2405,9 @@ struct drm_i915_gem_request {
>
> /** Execlists context hardware id. */
> unsigned ctx_hw_id;
> +
> + /** GVT workload **/
> + void *gvt_workload;
Why is this a void *, for what it will be used ? I can't see in this
patch series what will be stored here so asking. Is it only temporary
until further patches?
Also, is it really per-request? I am just wondering since other patches
are mostly concerned about contexts.
> };
>
> struct drm_i915_gem_request * __must_check
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0a96d4a..aeaea2e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -415,6 +415,19 @@ 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 *req,
> + unsigned long status)
> +{
> + if (!req->ctx->engine[req->engine->id].
> + enable_status_change_notification)
> + return;
> +
> + atomic_notifier_call_chain(
> + &req->ctx->engine[req->engine->id].status_notifier_head,
> + status, req->gvt_workload);
> +}
I would really like to see this compiled out when !CONFIG_DRM_I915_GVT.
Otherwise is just adding useless conditionals on submission fast paths.
> +
> static void execlists_context_unqueue(struct intel_engine_cs *engine)
> {
> struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
> @@ -450,6 +463,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
> @@ -488,6 +506,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);
>
> @@ -2541,6 +2561,9 @@ static int execlists_context_deferred_alloc(struct intel_context *ctx,
> ctx->engine[engine->id].state = ctx_obj;
> ctx->engine[engine->id].initialised = engine_initialised(ctx, engine);
>
> + if (ctx->engine[engine->id].enable_status_change_notification)
> + ATOMIC_INIT_NOTIFIER_HEAD(
> + &ctx->engine[engine->id].status_notifier_head);
> return 0;
>
> error_ringbuf:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 1afba03..98c94ee 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -57,6 +57,11 @@
> #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);
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 11/15] drm/i915: Introduce execlist context status change notification
2016-05-16 14:00 ` Tvrtko Ursulin
@ 2016-05-16 14:28 ` Wang, Zhi A
0 siblings, 0 replies; 31+ messages in thread
From: Wang, Zhi A @ 2016-05-16 14:28 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx@lists.freedesktop.org, Gordon, David S,
joonas.lahtinen@linux.intel.com, Tian, Kevin, Lv, Zhiyuan
-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
Sent: Monday, May 16, 2016 7:00 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [Intel-gfx] [PATCH 11/15] drm/i915: Introduce execlist context status change notification
On 15/05/16 18:32, 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.
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +++++
> drivers/gpu/drm/i915/intel_lrc.c | 23 +++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_lrc.h | 5 +++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 7f050a3..1c9e865 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -884,6 +884,8 @@ struct intel_context {
> bool initialised;
> bool skip_init_context;
> u32 ring_buffer_size;
> + bool enable_status_change_notification;
This flag will be per context and not per context-engine, correct? In which case it would be worth moving it to the context. Would make execlists_context_status_change cheaper as well.
OK. Will do.
> + struct atomic_notifier_head status_notifier_head;
> } engine[I915_NUM_ENGINES];
> bool use_48bit_addressing_mode;
>
> @@ -2403,6 +2405,9 @@ struct drm_i915_gem_request {
>
> /** Execlists context hardware id. */
> unsigned ctx_hw_id;
> +
> + /** GVT workload **/
> + void *gvt_workload;
Why is this a void *, for what it will be used ? I can't see in this patch series what will be stored here so asking. Is it only temporary until further patches?
Also, is it really per-request? I am just wondering since other patches are mostly concerned about contexts.
Will remove that according to today discussion.
> };
>
> struct drm_i915_gem_request * __must_check diff --git
> a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0a96d4a..aeaea2e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -415,6 +415,19 @@ 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 *req,
> + unsigned long status)
> +{
> + if (!req->ctx->engine[req->engine->id].
> + enable_status_change_notification)
> + return;
> +
> + atomic_notifier_call_chain(
> + &req->ctx->engine[req->engine->id].status_notifier_head,
> + status, req->gvt_workload);
> +}
I would really like to see this compiled out when !CONFIG_DRM_I915_GVT.
Otherwise is just adding useless conditionals on submission fast paths.
Let's do that.
> +
> static void execlists_context_unqueue(struct intel_engine_cs *engine)
> {
> struct drm_i915_gem_request *req0 = NULL, *req1 = NULL; @@ -450,6
> +463,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 @@
> -488,6 +506,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);
>
> @@ -2541,6 +2561,9 @@ static int execlists_context_deferred_alloc(struct intel_context *ctx,
> ctx->engine[engine->id].state = ctx_obj;
> ctx->engine[engine->id].initialised = engine_initialised(ctx,
> engine);
>
> + if (ctx->engine[engine->id].enable_status_change_notification)
> + ATOMIC_INIT_NOTIFIER_HEAD(
> + &ctx->engine[engine->id].status_notifier_head);
> return 0;
>
> error_ringbuf:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> b/drivers/gpu/drm/i915/intel_lrc.h
> index 1afba03..98c94ee 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -57,6 +57,11 @@
> #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);
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 12/15] drm/i915: Support context single submission
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (10 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 11/15] drm/i915: Introduce execlist context status change notification Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-15 17:32 ` [PATCH 13/15] drm/i915: Introduce GVT context creation API Zhi Wang
` (3 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
This patch introduces the support of context signle submission. As GVT
context may come from different guests, which requires different
configuration of render registers. It can't be combined in a dual ELSP
submission combo.
We make this function as a context feature in context creation service.
Only GVT-g will create this kinds of GEM context currently.
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 | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1c9e865..54dc069 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -888,6 +888,7 @@ struct intel_context {
struct atomic_notifier_head status_notifier_head;
} engine[I915_NUM_ENGINES];
bool use_48bit_addressing_mode;
+ bool 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 aeaea2e..4ea1f32 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -454,6 +454,18 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
i915_gem_request_unreference(req0);
req0 = cursor;
} else {
+ /*
+ * req0 (after merged) ctx requires single submission,
+ * stop picking
+ */
+ if (req0->ctx->single_submission)
+ break;
+ /*
+ * req0 ctx doesn't require single submission, but
+ * next req ctx requires, stop picking req1
+ */
+ if (cursor->ctx->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] 31+ messages in thread* [PATCH 13/15] drm/i915: Introduce GVT context creation API
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (11 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 12/15] drm/i915: Support context single submission Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-15 17:32 ` [PATCH 14/15] drm/i915: Factor out and expose i915_steal_fence() Zhi Wang
` (2 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
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
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_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 34 +++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 54dc069..ae1149c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3399,6 +3399,7 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
void i915_gem_context_free(struct kref *ctx_ref);
struct drm_i915_gem_object *
i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
+struct intel_context *i915_gem_create_gvt_context(struct drm_device *dev);
static inline void i915_gem_context_reference(struct intel_context *ctx)
{
kref_get(&ctx->ref);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b5b0849..31e3542 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -360,6 +360,40 @@ err_destroy:
return ERR_PTR(ret);
}
+/**
+ * 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 intel_context on success, NULL if failed
+ *
+ */
+struct intel_context *
+i915_gem_create_gvt_context(struct drm_device *dev)
+{
+ struct intel_context *ctx;
+ int i;
+
+ mutex_lock(&dev->struct_mutex);
+
+ ctx = i915_gem_create_context(dev, NULL, false);
+ if (IS_ERR(ctx))
+ goto out;
+
+ for (i = 0; i < I915_NUM_ENGINES; i++) {
+ ctx->engine[i].skip_init_context = true;
+ ctx->engine[i].enable_status_change_notification = true;
+ ctx->engine[i].ring_buffer_size = 512 * PAGE_SIZE;
+ }
+
+ ctx->single_submission = true;
+out:
+ mutex_unlock(&dev->struct_mutex);
+ return ctx;
+}
+
static void i915_gem_context_unpin(struct intel_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] 31+ messages in thread* [PATCH 14/15] drm/i915: Factor out and expose i915_steal_fence()
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (12 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 13/15] drm/i915: Introduce GVT context creation API Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 14:57 ` Chris Wilson
2016-05-15 17:32 ` [PATCH 15/15] drm/i915: Expose i915_find_fence_reg() Zhi Wang
2016-05-16 5:29 ` ✗ Ro.CI.BAT: failure for Introduce the implementation of GVT context Patchwork
15 siblings, 1 reply; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
Factor out and expose fence stealing functionality for GVT-g. GVT-g
will use i915_find_fence_reg() to find a free/unpin fence register
and use i915_steal_fence() to steal it.
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_fence.c | 39 +++++++++++++++++++++++++++--------
2 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ae1149c..96d9abd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3376,6 +3376,7 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj)
/* i915_gem_fence.c */
int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
+int i915_steal_fence(struct drm_i915_fence_reg *reg);
bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj);
void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c
index a2b938e..e790d7b 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -346,6 +346,33 @@ deadlock:
}
/**
+ * i915_steal_fence - steal a fence from a GEM object
+ * @reg: the fence register to be stolen
+ *
+ * Returns:
+ *
+ * 0 on success, negative error code on failure.
+ */
+int i915_steal_fence(struct drm_i915_fence_reg *reg)
+{
+ int ret;
+
+ if (WARN_ON(reg->pin_count))
+ return -EBUSY;
+
+ if (reg->obj) {
+ struct drm_i915_gem_object *old = reg->obj;
+
+ ret = i915_gem_object_wait_fence(old);
+ if (ret)
+ return ret;
+
+ i915_gem_object_fence_lost(old);
+ }
+ return 0;
+}
+
+/**
* i915_gem_object_get_fence - set up fencing for an object
* @obj: object to map through a fence reg
*
@@ -397,15 +424,9 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
if (IS_ERR(reg))
return PTR_ERR(reg);
- if (reg->obj) {
- struct drm_i915_gem_object *old = reg->obj;
-
- ret = i915_gem_object_wait_fence(old);
- if (ret)
- return ret;
-
- i915_gem_object_fence_lost(old);
- }
+ ret = i915_steal_fence(reg);
+ if (ret)
+ return ret;
} else
return 0;
--
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] 31+ messages in thread* Re: [PATCH 14/15] drm/i915: Factor out and expose i915_steal_fence()
2016-05-15 17:32 ` [PATCH 14/15] drm/i915: Factor out and expose i915_steal_fence() Zhi Wang
@ 2016-05-16 14:57 ` Chris Wilson
2016-05-16 15:13 ` Wang, Zhi A
0 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2016-05-16 14:57 UTC (permalink / raw)
To: Zhi Wang; +Cc: intel-gfx, zhiyuan.lv
On Mon, May 16, 2016 at 01:32:52AM +0800, Zhi Wang wrote:
> Factor out and expose fence stealing functionality for GVT-g. GVT-g
> will use i915_find_fence_reg() to find a free/unpin fence register
> and use i915_steal_fence() to steal it.
No. This code cannot be exported as is (the code makes presumptions that
the caller will carefully control the reference). And there is nothing
to tell us what you need it for.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 14/15] drm/i915: Factor out and expose i915_steal_fence()
2016-05-16 14:57 ` Chris Wilson
@ 2016-05-16 15:13 ` Wang, Zhi A
0 siblings, 0 replies; 31+ messages in thread
From: Wang, Zhi A @ 2016-05-16 15:13 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx@lists.freedesktop.org, Lv, Zhiyuan
I see. How about merging this function into i915_find_fence_reg()?
-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
Sent: Monday, May 16, 2016 7:58 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Gordon, David S <david.s.gordon@intel.com>; joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [Intel-gfx] [PATCH 14/15] drm/i915: Factor out and expose i915_steal_fence()
On Mon, May 16, 2016 at 01:32:52AM +0800, Zhi Wang wrote:
> Factor out and expose fence stealing functionality for GVT-g. GVT-g
> will use i915_find_fence_reg() to find a free/unpin fence register and
> use i915_steal_fence() to steal it.
No. This code cannot be exported as is (the code makes presumptions that the caller will carefully control the reference). And there is nothing to tell us what you need it for.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 15/15] drm/i915: Expose i915_find_fence_reg()
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (13 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 14/15] drm/i915: Factor out and expose i915_steal_fence() Zhi Wang
@ 2016-05-15 17:32 ` Zhi Wang
2016-05-16 5:29 ` ✗ Ro.CI.BAT: failure for Introduce the implementation of GVT context Patchwork
15 siblings, 0 replies; 31+ messages in thread
From: Zhi Wang @ 2016-05-15 17:32 UTC (permalink / raw)
To: intel-gfx, david.s.gordon, joonas.lahtinen, kevin.tian,
zhiyuan.lv
Expose i915_find_fence_reg() for GVT-g to allocate the fence registers
for vGPUs.
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_fence.c | 10 +++++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 96d9abd..30a6c99 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3377,6 +3377,7 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj)
int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
int i915_steal_fence(struct drm_i915_fence_reg *reg);
+struct drm_i915_fence_reg *i915_find_fence_reg(struct drm_device *dev);
bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj);
void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c
index e790d7b..9f019e5 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -308,7 +308,15 @@ i915_gem_object_put_fence(struct drm_i915_gem_object *obj)
return 0;
}
-static struct drm_i915_fence_reg *
+/**
+ * i915_find_fence_reg - find a free or unpinned fence register
+ * @dev: drm device
+ *
+ * Returns:
+ *
+ * pointer to fence register on success, error code in pointer on failure.
+ */
+struct drm_i915_fence_reg *
i915_find_fence_reg(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
--
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] 31+ messages in thread* ✗ Ro.CI.BAT: failure for Introduce the implementation of GVT context
2016-05-15 17:32 [PATCH 00/15] Introduce the implementation of GVT context Zhi Wang
` (14 preceding siblings ...)
2016-05-15 17:32 ` [PATCH 15/15] drm/i915: Expose i915_find_fence_reg() Zhi Wang
@ 2016-05-16 5:29 ` Patchwork
15 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2016-05-16 5:29 UTC (permalink / raw)
To: Wang, Zhi A; +Cc: intel-gfx
== Series Details ==
Series: Introduce the implementation of GVT context
URL : https://patchwork.freedesktop.org/series/7208/
State : failure
== Summary ==
Series 7208v1 Introduce the implementation of GVT context
http://patchwork.freedesktop.org/api/1.0/series/7208/revisions/1/mbox
Test core_auth:
Subgroup basic-auth:
pass -> INCOMPLETE (ro-ivb-i7-3770)
pass -> INCOMPLETE (ro-ivb2-i7-3770)
pass -> INCOMPLETE (ro-hsw-i7-4770r)
pass -> INCOMPLETE (ro-snb-i7-2620M)
pass -> INCOMPLETE (ro-hsw-i3-4010u)
ro-bdw-i5-5250u total:219 pass:181 dwarn:0 dfail:0 fail:0 skip:38
ro-bdw-i7-5557U total:219 pass:206 dwarn:0 dfail:0 fail:0 skip:13
ro-bdw-i7-5600u total:219 pass:187 dwarn:0 dfail:0 fail:0 skip:32
ro-hsw-i3-4010u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
ro-hsw-i7-4770r total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
ro-ilk-i7-620lm total:219 pass:151 dwarn:0 dfail:0 fail:1 skip:67
ro-ilk1-i5-650 total:214 pass:151 dwarn:0 dfail:0 fail:2 skip:61
ro-ivb-i7-3770 total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
ro-ivb2-i7-3770 total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
ro-skl-i7-6700hq total:214 pass:189 dwarn:0 dfail:0 fail:0 skip:25
ro-snb-i7-2620M total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
ro-bsw-n3050 failed to connect after reboot
ro-byt-n2820 failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_903/
5528ede drm-intel-nightly: 2016y-05m-14d-08h-24m-24s UTC integration manifest
c2c5b55 drm/i915: Expose i915_find_fence_reg()
4af5589 drm/i915: Factor out and expose i915_steal_fence()
db6699d drm/i915: Introduce GVT context creation API
e3e4e13 drm/i915: Support context single submission
78a70c5 drm/i915: Introduce execlist context status change notification
31ababa drm/i915: Generate addressing mode bit from flag in context.
0f36bf1 drm/i915: Make ring buffer size configurable
d744b35 drm/i915: Introduce an option for skipping engine context initialization
19345d7 drm/i915: Populate context PDPs if it has a PPGTT
a05f47b drm/i915: Allow the caller to create a intel_context without PPGTT
1f880ae drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation
33fbe4c drm/i915: Introduce host graphics memory partition for GVT-g
1be618d drm/i915: gvt: Introduce the basic architecture of GVT-g
1c7026b drm/i915/gvt: Fold vGPU active check into inner functions
e612d91 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] 31+ messages in thread