[v6,3/8] drm/i915: vgpu ppgtt update pv optimization

Submitted by Xiaolin Zhang on June 3, 2019, 6:02 a.m.

Details

Message ID 1559541769-25279-4-git-send-email-xiaolin.zhang@intel.com
State New
Headers show
Series "i915 vgpu PV to improve vgpu performance" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Xiaolin Zhang June 3, 2019, 6:02 a.m.
This patch extends vgpu ppgtt g2v notification to notify host
GVT-g of ppgtt update from guest including alloc_4lvl, clear_4lv4
and insert_4lvl.

These updates use the shared memory page to pass struct pv_ppgtt_update
from guest to GVT which is used for pv optimiation implemeation within
host GVT side.

This patch also add one new pv_caps level to control ppgtt update.

Use PV_PPGTT_UPDATE to control this level of pv optimization.

v0: RFC.
v1: rebased.
v2: added pv callbacks for vm.{allocate_va_range, insert_entries,
clear_range} within ppgtt.
v3: rebased, disable huge page ppgtt support when using PVMMIO ppgtt
update due to complex and performance impact.
v4: moved alloc/insert/clear_4lvl pv callbacks into i915_vgpu_pv.c and
added a single intel_vgpu_config_pv_caps() for vgpu pv callbacks setup.
v5: rebase.
v6: rebase.

Signed-off-by: Xiaolin Zhang <xiaolin.zhang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c     |  3 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c |  9 ++--
 drivers/gpu/drm/i915/i915_gem_gtt.h |  8 ++++
 drivers/gpu/drm/i915/i915_pvinfo.h  |  3 ++
 drivers/gpu/drm/i915/i915_vgpu.c    | 84 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_vgpu.h    | 17 ++++++++
 6 files changed, 120 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4739a630..975c784 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1507,7 +1507,8 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 	int ret;
 
 	/* We need to fallback to 4K pages if host doesn't support huge gtt. */
-	if (intel_vgpu_active(dev_priv) && !intel_vgpu_has_huge_gtt(dev_priv))
+	if ((intel_vgpu_active(dev_priv) && !intel_vgpu_has_huge_gtt(dev_priv))
+		|| intel_vgpu_enabled_pv_caps(dev_priv, PV_PPGTT_UPDATE))
 		mkwrite_device_info(dev_priv)->page_sizes =
 			I915_GTT_PAGE_SIZE_4K;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ca8a69e..480e8f4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -926,7 +926,7 @@  static void gen8_ppgtt_set_pml4e(struct i915_pml4 *pml4,
  * This is the top-level structure in 4-level page tables used on gen8+.
  * Empty entries are always scratch pml4e.
  */
-static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
+void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 				  u64 start, u64 length)
 {
 	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
@@ -1165,7 +1165,7 @@  static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
 	} while (iter->sg);
 }
 
-static void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
+void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
 				   struct i915_vma *vma,
 				   enum i915_cache_level cache_level,
 				   u32 flags)
@@ -1447,7 +1447,7 @@  static int gen8_ppgtt_alloc_3lvl(struct i915_address_space *vm,
 				    &i915_vm_to_ppgtt(vm)->pdp, start, length);
 }
 
-static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
+int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 				 u64 start, u64 length)
 {
 	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
@@ -1579,6 +1579,9 @@  static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 		ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc_4lvl;
 		ppgtt->vm.insert_entries = gen8_ppgtt_insert_4lvl;
 		ppgtt->vm.clear_range = gen8_ppgtt_clear_4lvl;
+
+		if (intel_vgpu_active(i915))
+			intel_vgpu_config_pv_caps(i915, PV_PPGTT_UPDATE, ppgtt);
 	} else {
 		err = __pdp_init(&ppgtt->vm, &ppgtt->pdp);
 		if (err)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 73b6608..2372f03 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -646,6 +646,14 @@  int gen6_ppgtt_pin(struct i915_hw_ppgtt *base);
 void gen6_ppgtt_unpin(struct i915_hw_ppgtt *base);
 void gen6_ppgtt_unpin_all(struct i915_hw_ppgtt *base);
 
+void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
+		u64 start, u64 length);
+void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
+		struct i915_vma *vma,
+		enum i915_cache_level cache_level, u32 flags);
+int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
+		u64 start, u64 length);
+
 void i915_check_and_clear_faults(struct drm_i915_private *dev_priv);
 void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv);
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
index 4657bf7..2408a9d 100644
--- a/drivers/gpu/drm/i915/i915_pvinfo.h
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -47,6 +47,9 @@  enum vgt_g2v_type {
 	VGT_G2V_EXECLIST_CONTEXT_CREATE,
 	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
 	VGT_G2V_SHARED_PAGE_SETUP,
+	VGT_G2V_PPGTT_L4_ALLOC,
+	VGT_G2V_PPGTT_L4_CLEAR,
+	VGT_G2V_PPGTT_L4_INSERT,
 	VGT_G2V_MAX,
 };
 
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 6020515..418582c 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -80,6 +80,9 @@  void i915_check_vgpu(struct drm_i915_private *dev_priv)
 
 	dev_priv->vgpu.active = true;
 
+	/* guest driver PV capability */
+	dev_priv->vgpu.pv_caps = PV_PPGTT_UPDATE;
+
 	if (!intel_vgpu_check_pv_caps(dev_priv)) {
 		DRM_INFO("Virtual GPU for Intel GVT-g detected.\n");
 		return;
@@ -289,6 +292,87 @@  int intel_vgt_balloon(struct drm_i915_private *dev_priv)
  * i915 vgpu PV support for Linux
  */
 
+static void gen8_ppgtt_clear_4lvl_pv(struct i915_address_space *vm,
+				  u64 start, u64 length)
+{
+	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
+	struct i915_pml4 *pml4 = &ppgtt->pml4;
+	struct drm_i915_private *dev_priv = vm->i915;
+	struct pv_ppgtt_update *pv_ppgtt =
+			&dev_priv->vgpu.shared_page->buf.pv_ppgtt;
+	u64 orig_start = start;
+	u64 orig_length = length;
+
+	gen8_ppgtt_clear_4lvl(vm, start, length);
+
+	pv_ppgtt->pdp = px_dma(pml4);
+	pv_ppgtt->start = orig_start;
+	pv_ppgtt->length = orig_length;
+	I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_CLEAR);
+}
+
+static void gen8_ppgtt_insert_4lvl_pv(struct i915_address_space *vm,
+				   struct i915_vma *vma,
+				   enum i915_cache_level cache_level,
+				   u32 flags)
+{
+	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
+	struct drm_i915_private *dev_priv = vm->i915;
+	struct pv_ppgtt_update *pv_ppgtt =
+			&dev_priv->vgpu.shared_page->buf.pv_ppgtt;
+
+	gen8_ppgtt_insert_4lvl(vm, vma, cache_level, flags);
+
+	pv_ppgtt->pdp = px_dma(&ppgtt->pml4);
+	pv_ppgtt->start = vma->node.start;
+	pv_ppgtt->length = vma->node.size;
+	pv_ppgtt->cache_level = cache_level;
+	I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_INSERT);
+}
+
+static int gen8_ppgtt_alloc_4lvl_pv(struct i915_address_space *vm,
+				 u64 start, u64 length)
+{
+	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
+	struct i915_pml4 *pml4 = &ppgtt->pml4;
+	struct drm_i915_private *dev_priv = vm->i915;
+	struct pv_ppgtt_update *pv_ppgtt =
+			&dev_priv->vgpu.shared_page->buf.pv_ppgtt;
+	int ret;
+	u64 orig_start = start;
+	u64 orig_length = length;
+
+	ret = gen8_ppgtt_alloc_4lvl(vm, start, length);
+	if (ret)
+		return ret;
+
+	pv_ppgtt->pdp = px_dma(pml4);
+	pv_ppgtt->start = orig_start;
+	pv_ppgtt->length = orig_length;
+	I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_ALLOC);
+
+	return 0;
+}
+
+/*
+ * config guest driver PV ops for different PV features
+ */
+void intel_vgpu_config_pv_caps(struct drm_i915_private *dev_priv,
+		enum pv_caps cap, void *data)
+{
+	struct i915_hw_ppgtt *ppgtt;
+
+	if (!intel_vgpu_enabled_pv_caps(dev_priv, cap))
+		return;
+
+	if (cap == PV_PPGTT_UPDATE) {
+		ppgtt = (struct i915_hw_ppgtt *)data;
+		ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc_4lvl_pv;
+		ppgtt->vm.insert_entries = gen8_ppgtt_insert_4lvl_pv;
+		ppgtt->vm.clear_range = gen8_ppgtt_clear_4lvl_pv;
+	}
+}
+
 /*
  * shared_page setup for VGPU PV features
  */
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 1030f5a..7a39748 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -27,6 +27,13 @@ 
 #include "i915_pvinfo.h"
 
 /*
+ * define different capabilities of PV optimization
+ */
+enum pv_caps {
+	PV_PPGTT_UPDATE = 0x1,
+};
+
+/*
  * A shared page(4KB) between gvt and VM, could be allocated by guest driver
  * or a fixed location in PCI bar 0 region
  */
@@ -79,9 +86,19 @@  intel_vgpu_has_pv_caps(struct drm_i915_private *dev_priv)
 	return dev_priv->vgpu.caps & VGT_CAPS_PV;
 }
 
+static inline bool
+intel_vgpu_enabled_pv_caps(struct drm_i915_private *dev_priv,
+		enum pv_caps cap)
+{
+	return intel_vgpu_active(dev_priv) && intel_vgpu_has_pv_caps(dev_priv)
+			&& (dev_priv->vgpu.pv_caps & cap);
+}
+
 int intel_vgt_balloon(struct drm_i915_private *dev_priv);
 void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
 
 /* i915 vgpu pv related functions */
 bool intel_vgpu_check_pv_caps(struct drm_i915_private *dev_priv);
+void intel_vgpu_config_pv_caps(struct drm_i915_private *dev_priv,
+		enum pv_caps cap, void *data);
 #endif /* _I915_VGPU_H_ */

Comments

Quoting Xiaolin Zhang (2019-06-03 07:02:44)
> +static void gen8_ppgtt_clear_4lvl_pv(struct i915_address_space *vm,
> +                                 u64 start, u64 length)
> +{
> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> +       struct i915_pml4 *pml4 = &ppgtt->pml4;
> +       struct drm_i915_private *dev_priv = vm->i915;
> +       struct pv_ppgtt_update *pv_ppgtt =
> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
> +       u64 orig_start = start;
> +       u64 orig_length = length;
> +
> +       gen8_ppgtt_clear_4lvl(vm, start, length);
> +
> +       pv_ppgtt->pdp = px_dma(pml4);
> +       pv_ppgtt->start = orig_start;
> +       pv_ppgtt->length = orig_length;
> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_CLEAR);
> +}
> +
> +static void gen8_ppgtt_insert_4lvl_pv(struct i915_address_space *vm,
> +                                  struct i915_vma *vma,
> +                                  enum i915_cache_level cache_level,
> +                                  u32 flags)
> +{
> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> +       struct drm_i915_private *dev_priv = vm->i915;
> +       struct pv_ppgtt_update *pv_ppgtt =
> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
> +
> +       gen8_ppgtt_insert_4lvl(vm, vma, cache_level, flags);
> +
> +       pv_ppgtt->pdp = px_dma(&ppgtt->pml4);
> +       pv_ppgtt->start = vma->node.start;
> +       pv_ppgtt->length = vma->node.size;
> +       pv_ppgtt->cache_level = cache_level;
> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_INSERT);
> +}

For this to work, a vgpu mmio write must be trapped and suspend the
client while the host processes the trap. Otherwise, we would be
overwriting the side-channel before the host processes the command.

That sounds horrible.
-Chris
On 06/04/2019 05:01 PM, Chris Wilson wrote:
> Quoting Xiaolin Zhang (2019-06-03 07:02:44)
>> +static void gen8_ppgtt_clear_4lvl_pv(struct i915_address_space *vm,
>> +                                 u64 start, u64 length)
>> +{
>> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>> +       struct i915_pml4 *pml4 = &ppgtt->pml4;
>> +       struct drm_i915_private *dev_priv = vm->i915;
>> +       struct pv_ppgtt_update *pv_ppgtt =
>> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
>> +       u64 orig_start = start;
>> +       u64 orig_length = length;
>> +
>> +       gen8_ppgtt_clear_4lvl(vm, start, length);
>> +
>> +       pv_ppgtt->pdp = px_dma(pml4);
>> +       pv_ppgtt->start = orig_start;
>> +       pv_ppgtt->length = orig_length;
>> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_CLEAR);
>> +}
>> +
>> +static void gen8_ppgtt_insert_4lvl_pv(struct i915_address_space *vm,
>> +                                  struct i915_vma *vma,
>> +                                  enum i915_cache_level cache_level,
>> +                                  u32 flags)
>> +{
>> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>> +       struct drm_i915_private *dev_priv = vm->i915;
>> +       struct pv_ppgtt_update *pv_ppgtt =
>> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
>> +
>> +       gen8_ppgtt_insert_4lvl(vm, vma, cache_level, flags);
>> +
>> +       pv_ppgtt->pdp = px_dma(&ppgtt->pml4);
>> +       pv_ppgtt->start = vma->node.start;
>> +       pv_ppgtt->length = vma->node.size;
>> +       pv_ppgtt->cache_level = cache_level;
>> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_INSERT);
>> +}
> For this to work, a vgpu mmio write must be trapped and suspend the
> client while the host processes the trap. Otherwise, we would be
> overwriting the side-channel before the host processes the command.
>
> That sounds horrible.
> -Chris
Chris, thanks your comment. do you think is the spin_lock to protect
this VGPU MMIO write enough to eliminate the side-channel effect?
Xiaolin
Quoting Zhang, Xiaolin (2019-06-10 02:32:18)
> On 06/04/2019 05:01 PM, Chris Wilson wrote:
> > Quoting Xiaolin Zhang (2019-06-03 07:02:44)
> >> +static void gen8_ppgtt_clear_4lvl_pv(struct i915_address_space *vm,
> >> +                                 u64 start, u64 length)
> >> +{
> >> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> >> +       struct i915_pml4 *pml4 = &ppgtt->pml4;
> >> +       struct drm_i915_private *dev_priv = vm->i915;
> >> +       struct pv_ppgtt_update *pv_ppgtt =
> >> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
> >> +       u64 orig_start = start;
> >> +       u64 orig_length = length;
> >> +
> >> +       gen8_ppgtt_clear_4lvl(vm, start, length);
> >> +
> >> +       pv_ppgtt->pdp = px_dma(pml4);
> >> +       pv_ppgtt->start = orig_start;
> >> +       pv_ppgtt->length = orig_length;
> >> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_CLEAR);
> >> +}
> >> +
> >> +static void gen8_ppgtt_insert_4lvl_pv(struct i915_address_space *vm,
> >> +                                  struct i915_vma *vma,
> >> +                                  enum i915_cache_level cache_level,
> >> +                                  u32 flags)
> >> +{
> >> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> >> +       struct drm_i915_private *dev_priv = vm->i915;
> >> +       struct pv_ppgtt_update *pv_ppgtt =
> >> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
> >> +
> >> +       gen8_ppgtt_insert_4lvl(vm, vma, cache_level, flags);
> >> +
> >> +       pv_ppgtt->pdp = px_dma(&ppgtt->pml4);
> >> +       pv_ppgtt->start = vma->node.start;
> >> +       pv_ppgtt->length = vma->node.size;
> >> +       pv_ppgtt->cache_level = cache_level;
> >> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_INSERT);
> >> +}
> > For this to work, a vgpu mmio write must be trapped and suspend the
> > client while the host processes the trap. Otherwise, we would be
> > overwriting the side-channel before the host processes the command.
> >
> > That sounds horrible.
> > -Chris
> Chris, thanks your comment. do you think is the spin_lock to protect
> this VGPU MMIO write enough to eliminate the side-channel effect?

I would suggest you consider using a pair of command/response rings.
-Chris
On 06/10/2019 03:44 PM, Chris Wilson wrote:
> Quoting Zhang, Xiaolin (2019-06-10 02:32:18)
>> On 06/04/2019 05:01 PM, Chris Wilson wrote:
>>> Quoting Xiaolin Zhang (2019-06-03 07:02:44)
>>>> +static void gen8_ppgtt_clear_4lvl_pv(struct i915_address_space *vm,
>>>> +                                 u64 start, u64 length)
>>>> +{
>>>> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>>>> +       struct i915_pml4 *pml4 = &ppgtt->pml4;
>>>> +       struct drm_i915_private *dev_priv = vm->i915;
>>>> +       struct pv_ppgtt_update *pv_ppgtt =
>>>> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
>>>> +       u64 orig_start = start;
>>>> +       u64 orig_length = length;
>>>> +
>>>> +       gen8_ppgtt_clear_4lvl(vm, start, length);
>>>> +
>>>> +       pv_ppgtt->pdp = px_dma(pml4);
>>>> +       pv_ppgtt->start = orig_start;
>>>> +       pv_ppgtt->length = orig_length;
>>>> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_CLEAR);
>>>> +}
>>>> +
>>>> +static void gen8_ppgtt_insert_4lvl_pv(struct i915_address_space *vm,
>>>> +                                  struct i915_vma *vma,
>>>> +                                  enum i915_cache_level cache_level,
>>>> +                                  u32 flags)
>>>> +{
>>>> +       struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>>>> +       struct drm_i915_private *dev_priv = vm->i915;
>>>> +       struct pv_ppgtt_update *pv_ppgtt =
>>>> +                       &dev_priv->vgpu.shared_page->buf.pv_ppgtt;
>>>> +
>>>> +       gen8_ppgtt_insert_4lvl(vm, vma, cache_level, flags);
>>>> +
>>>> +       pv_ppgtt->pdp = px_dma(&ppgtt->pml4);
>>>> +       pv_ppgtt->start = vma->node.start;
>>>> +       pv_ppgtt->length = vma->node.size;
>>>> +       pv_ppgtt->cache_level = cache_level;
>>>> +       I915_WRITE(vgtif_reg(g2v_notify), VGT_G2V_PPGTT_L4_INSERT);
>>>> +}
>>> For this to work, a vgpu mmio write must be trapped and suspend the
>>> client while the host processes the trap. Otherwise, we would be
>>> overwriting the side-channel before the host processes the command.
>>>
>>> That sounds horrible.
>>> -Chris
>> Chris, thanks your comment. do you think is the spin_lock to protect
>> this VGPU MMIO write enough to eliminate the side-channel effect?
> I would suggest you consider using a pair of command/response rings.
> -Chris
>
Chris, Thanks your suggestion and prompt response. I will rework them
for that direction.
BRs, Xiaolin