[PATCH v2 0/4] Enable guest linux i915 full ppgtt functionality

[PATCH v2 0/4] Enable guest linux i915 full ppgtt functionality

[Tina Zhang]

This patchset fixs guest i915 full ppgtt block issue in device model and
enables the full ppgtt functionality to guest i915 driver when device model
with the fixed patch can support this capability.

Changes since last version:
- Refine code with coding style.
- Rewrite the guest vgpu full ppgtt checking logic.

Tina Zhang (4):
  drm/i915/gvt: reorder the shadow ppgtt update process by adding entry
    first
  drm/i915: introduce vgt_caps to pvinfo, v2
  drm/i915/gvt: provide full ppgtt capability for guest, v2
  drm/i915: enable guest full ppgtt when device model supports, v2

 drivers/gpu/drm/i915/gvt/gtt.c      | 45 +++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/gvt/vgpu.c     |  1 +
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 +++++----
 drivers/gpu/drm/i915/i915_pvinfo.h  |  8 ++++++-
 drivers/gpu/drm/i915/i915_vgpu.c    |  7 ++++++
 drivers/gpu/drm/i915/i915_vgpu.h    |  3 +++
 7 files changed, 52 insertions(+), 24 deletions(-)

-- 
2.7.4

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

[PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt update process by adding entry first

[Tina Zhang]

Guest i915 driver may update the ppgtt table just before this workload
is going to be submitted to the hardware by device model. This case
wasn't handled well by device model before, due to the small time
window between removing old ppgtt entry and adding the new one. Errors
occur when the workload is executed by hardware during that small time
window. This patch is to remove this time window by adding the new ppgtt
entry first and then remove the old one.

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/gtt.c | 45 ++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index c6f0077..8caad86 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -975,29 +975,26 @@ static int ppgtt_populate_shadow_page(struct intel_vgpu_ppgtt_spt *spt)
 }
 
 static int ppgtt_handle_guest_entry_removal(struct intel_vgpu_guest_page *gpt,
-		unsigned long index)
+		struct intel_gvt_gtt_entry *se, unsigned long index)
 {
 	struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt);
 	struct intel_vgpu_shadow_page *sp = &spt->shadow_page;
 	struct intel_vgpu *vgpu = spt->vgpu;
 	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
-	struct intel_gvt_gtt_entry e;
 	int ret;
 
-	ppgtt_get_shadow_entry(spt, &e, index);
-
-	trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, e.val64,
+	trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, se->val64,
 			 index);
 
-	if (!ops->test_present(&e))
+	if (!ops->test_present(se))
 		return 0;
 
-	if (ops->get_pfn(&e) == vgpu->gtt.scratch_pt[sp->type].page_mfn)
+	if (ops->get_pfn(se) == vgpu->gtt.scratch_pt[sp->type].page_mfn)
 		return 0;
 
-	if (gtt_type_is_pt(get_next_pt_type(e.type))) {
+	if (gtt_type_is_pt(get_next_pt_type(se->type))) {
 		struct intel_vgpu_ppgtt_spt *s =
-			ppgtt_find_shadow_page(vgpu, ops->get_pfn(&e));
+			ppgtt_find_shadow_page(vgpu, ops->get_pfn(se));
 		if (!s) {
 			gvt_vgpu_err("fail to find guest page\n");
 			ret = -ENXIO;
@@ -1007,12 +1004,10 @@ static int ppgtt_handle_guest_entry_removal(struct intel_vgpu_guest_page *gpt,
 		if (ret)
 			goto fail;
 	}
-	ops->set_pfn(&e, vgpu->gtt.scratch_pt[sp->type].page_mfn);
-	ppgtt_set_shadow_entry(spt, &e, index);
 	return 0;
 fail:
 	gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d\n",
-			spt, e.val64, e.type);
+			spt, se->val64, se->type);
 	return ret;
 }
 
@@ -1232,22 +1227,37 @@ static int ppgtt_handle_guest_write_page_table(
 {
 	struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt);
 	struct intel_vgpu *vgpu = spt->vgpu;
+	int type = spt->shadow_page.type;
 	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
+	struct intel_gvt_gtt_entry se;
 
 	int ret;
 	int new_present;
 
 	new_present = ops->test_present(we);
 
-	ret = ppgtt_handle_guest_entry_removal(gpt, index);
-	if (ret)
-		goto fail;
+	/*
+	 * Adding the new entry first and then removing the old one, that can
+	 * guarantee the ppgtt table is validated during the window between
+	 * adding and removal.
+	 */
+	ppgtt_get_shadow_entry(spt, &se, index);
 
 	if (new_present) {
 		ret = ppgtt_handle_guest_entry_add(gpt, we, index);
 		if (ret)
 			goto fail;
 	}
+
+	ret = ppgtt_handle_guest_entry_removal(gpt, &se, index);
+	if (ret)
+		goto fail;
+
+	if (!new_present) {
+		ops->set_pfn(&se, vgpu->gtt.scratch_pt[type].page_mfn);
+		ppgtt_set_shadow_entry(spt, &se, index);
+	}
+
 	return 0;
 fail:
 	gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d.\n",
@@ -1319,7 +1329,7 @@ static int ppgtt_handle_guest_write_page_table_bytes(void *gp,
 	struct intel_vgpu *vgpu = spt->vgpu;
 	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
 	const struct intel_gvt_device_info *info = &vgpu->gvt->device_info;
-	struct intel_gvt_gtt_entry we;
+	struct intel_gvt_gtt_entry we, se;
 	unsigned long index;
 	int ret;
 
@@ -1335,7 +1345,8 @@ static int ppgtt_handle_guest_write_page_table_bytes(void *gp,
 			return ret;
 	} else {
 		if (!test_bit(index, spt->post_shadow_bitmap)) {
-			ret = ppgtt_handle_guest_entry_removal(gpt, index);
+			ppgtt_get_shadow_entry(spt, &se, index);
+			ret = ppgtt_handle_guest_entry_removal(gpt, &se, index);
 			if (ret)
 				return ret;
 		}
-- 
2.7.4

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

[PATCH v2 2/4] drm/i915: introduce vgt_caps to pvinfo

[Tina Zhang]

vgt_caps is used for guest i915 driver to get the vgpu capabilities from
the device model. VGT_CPAS_FULL_PPGTT is one of the capabilities type to
let guest i915 dirver know that the guest i915 full ppgtt is supported by
device model.

Changes since v1:
- Use u32 instead of uint32_t (Joonas)
- Move VGT_CAPS_FULL_PPGTT introduction to this patch and use #define instead
  of enum (Joonas)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/i915_pvinfo.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
index c0cb297..8dc0664 100644
--- a/drivers/gpu/drm/i915/i915_pvinfo.h
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -53,12 +53,18 @@ enum vgt_g2v_type {
 	VGT_G2V_MAX,
 };
 
+/*
+ * VGT capabilities type
+ */
+#define VGT_CAPS_FULL_PPGTT	BIT(2)
+
 struct vgt_if {
 	u64 magic;		/* VGT_MAGIC */
 	uint16_t version_major;
 	uint16_t version_minor;
 	u32 vgt_id;		/* ID of vGT instance */
-	u32 rsv1[12];		/* pad to offset 0x40 */
+	u32 vgt_caps;		/* VGT capabilities */
+	u32 rsv1[11];		/* 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.
-- 
2.7.4

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

[PATCH v2 3/4] drm/i915/gvt: provide full ppgtt capability for guest

[Tina Zhang]

This patch is to provide full ppgtt capability for guest i915 driver.

Changes since v1:
- Move VGT_CAPS_FULL_PPGTT introduction to patch 2/4. (Joonas)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/vgpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 6e3cbd8..23578c1 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -43,6 +43,7 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
 	vgpu_vreg(vgpu, vgtif_reg(version_minor)) = 0;
 	vgpu_vreg(vgpu, vgtif_reg(display_ready)) = 0;
 	vgpu_vreg(vgpu, vgtif_reg(vgt_id)) = vgpu->id;
+	vgpu_vreg(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_PPGTT;
 	vgpu_vreg(vgpu, vgtif_reg(avail_rs.mappable_gmadr.base)) =
 		vgpu_aperture_gmadr_base(vgpu);
 	vgpu_vreg(vgpu, vgtif_reg(avail_rs.mappable_gmadr.size)) =
-- 
2.7.4

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

[PATCH v2 4/4] drm/i915: enable guest full ppgtt when device model supports

[Tina Zhang]

Add full ppgtt capability check in guest i915 driver and enable the full
ppgtt in guest only when device mode supports.

Changes since v1:
- Use u32 instead of uint32_t. (Joonas)
- Rewrite the vgpu full ppgtt capability checking logic. (Joonas)
- Some coding style refine. (Joonas)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 +++++------
 drivers/gpu/drm/i915/i915_vgpu.c    |  7 +++++++
 drivers/gpu/drm/i915/i915_vgpu.h    |  3 +++
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 357b6c6..2e34927 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1909,6 +1909,7 @@ struct i915_workarounds {
 
 struct i915_virtual_gpu {
 	bool active;
+	u32 caps;
 };
 
 /* used in computing the new watermarks state */
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8bab4ae..3f110aa 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -139,13 +139,12 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 
 	has_aliasing_ppgtt = dev_priv->info.has_aliasing_ppgtt;
 	has_full_ppgtt = dev_priv->info.has_full_ppgtt;
-	has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt;
 
-	if (intel_vgpu_active(dev_priv)) {
-		/* emulation is too hard */
-		has_full_ppgtt = false;
-		has_full_48bit_ppgtt = false;
-	}
+	if (intel_vgpu_active(dev_priv))
+		has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv);
+
+	has_full_48bit_ppgtt = has_full_ppgtt &&
+				dev_priv->info.has_full_48bit_ppgtt;
 
 	if (!has_aliasing_ppgtt)
 		return 0;
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 4ab8a97..adc4eda 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -77,10 +77,17 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv)
 		return;
 	}
 
+	dev_priv->vgpu.caps = __raw_i915_read32(dev_priv, vgtif_reg(vgt_caps));
+
 	dev_priv->vgpu.active = true;
 	DRM_INFO("Virtual GPU for Intel GVT-g detected.\n");
 }
 
+bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv)
+{
+	return dev_priv->vgpu.caps & VGT_CAPS_FULL_PPGTT;
+}
+
 struct _balloon_info_ {
 	/*
 	 * There are up to 2 regions per mappable/unmappable graphic
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 3c3b2d2..b4e04eb 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -27,6 +27,9 @@
 #include "i915_pvinfo.h"
 
 void i915_check_vgpu(struct drm_i915_private *dev_priv);
+
+bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv);
+
 int intel_vgt_balloon(struct drm_i915_private *dev_priv);
 void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
 
-- 
2.7.4

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

✓ Fi.CI.BAT: success for Enable guest linux i915 full ppgtt functionality (rev2)

[Patchwork]

== Series Details ==

Series: Enable guest linux i915 full ppgtt functionality (rev2)
URL   : https://patchwork.freedesktop.org/series/24271/
State : success

== Summary ==

Series 24271v2 Enable guest linux i915 full ppgtt functionality
https://patchwork.freedesktop.org/api/1.0/series/24271/revisions/2/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:437s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:434s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:580s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:516s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:561s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:502s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:488s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:423s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:489s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:497s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:460s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:578s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:467s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:581s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:471s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:496s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:444s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:417s

51350ea90d1a5c8bb7e6532eb61c64a57e83ab9b drm-tip: 2017y-05m-11d-13h-05m-06s UTC integration manifest
c8b3314 drm/i915: enable guest full ppgtt when device model supports
4b4151c drm/i915/gvt: provide full ppgtt capability for guest
9a9e0ba drm/i915: introduce vgt_caps to pvinfo
40e3bd6 drm/i915/gvt: reorder the shadow ppgtt update process by adding entry first

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4680/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/4] drm/i915: introduce vgt_caps to pvinfo

[Joonas Lahtinen]

On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote:
> vgt_caps is used for guest i915 driver to get the vgpu capabilities from
> the device model. VGT_CPAS_FULL_PPGTT is one of the capabilities type to
> let guest i915 dirver know that the guest i915 full ppgtt is supported by
> device model.
> 
> Changes since v1:
> - Use u32 instead of uint32_t (Joonas)
> - Move VGT_CAPS_FULL_PPGTT introduction to this patch and use #define instead
>   of enum (Joonas)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>

This should probably be the first patch in the series.

As far as the code goes, this is;

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

Regards, Joonas

PS. Please remember to add Cc's for patches next time.
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/4] drm/i915/gvt: provide full ppgtt capability for guest

[Joonas Lahtinen]

On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote:
> This patch is to provide full ppgtt capability for guest i915 driver.
> 
> Changes since v1:
> - Move VGT_CAPS_FULL_PPGTT introduction to patch 2/4. (Joonas)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>

I think this should get squashed to the patch 1 in this series, which
enables the functionality in gvt.

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

Re: [PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt update process by adding entry first

[Joonas Lahtinen]

On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote:
> Guest i915 driver may update the ppgtt table just before this workload
> is going to be submitted to the hardware by device model. This case
> wasn't handled well by device model before, due to the small time
> window between removing old ppgtt entry and adding the new one. Errors
> occur when the workload is executed by hardware during that small time
> window. This patch is to remove this time window by adding the new ppgtt
> entry first and then remove the old one.
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>

This should be reviewed on the GVT mailing list, and this should
include the squashed hunk which exports the newly added capability.

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

Re: [PATCH v2 4/4] drm/i915: enable guest full ppgtt when device model supports

[Joonas Lahtinen]

On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote:
> Add full ppgtt capability check in guest i915 driver and enable the full
> ppgtt in guest only when device mode supports.
> 
> Changes since v1:
> - Use u32 instead of uint32_t. (Joonas)
> - Rewrite the vgpu full ppgtt capability checking logic. (Joonas)
> - Some coding style refine. (Joonas)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>

This patch should be prior to GVT patches in the series to avoid
breaking bisecting, which means that when the patches are applied one-
by-one, each intermediary step produces a working kernel.

Thi could be squashed to the patch that includes the declarations and
#define in i915 side so that this series would be one patch in i915
side to add the declaration and one patch in GVT side which implements
the functionality and enables the capability.

One comment below.

<SNIP>

> @@ -139,13 +139,12 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>  
>  	has_aliasing_ppgtt = dev_priv->info.has_aliasing_ppgtt;
>  	has_full_ppgtt = dev_priv->info.has_full_ppgtt;
> -	has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt;
>  
> -	if (intel_vgpu_active(dev_priv)) {
> -		/* emulation is too hard */
> -		has_full_ppgtt = false;
> -		has_full_48bit_ppgtt = false;
> -	}
> +	if (intel_vgpu_active(dev_priv))
> +		has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv);
> +
> +	has_full_48bit_ppgtt = has_full_ppgtt &&
> +				dev_priv->info.has_full_48bit_ppgtt;

This line is not correctly aligned.

With the alignment fixed, this is:

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

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

Re: [PATCH v2 0/4] Enable guest linux i915 full ppgtt functionality

[Joonas Lahtinen]

Remember to add Cc's for people who have previously commented on the
patches, and initially try to capture people who've been involved with
virtualization code reviews, to shorten the time it takes to notice the
series.

I went through the i915 portion and suggested reordering the series a
bit, the code itself is good to go.

Regards, Joonas

On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote:
> This patchset fixs guest i915 full ppgtt block issue in device model and
> enables the full ppgtt functionality to guest i915 driver when device model
> with the fixed patch can support this capability.
> 
> Changes since last version:
> - Refine code with coding style.
> - Rewrite the guest vgpu full ppgtt checking logic.
> 
> Tina Zhang (4):
>   drm/i915/gvt: reorder the shadow ppgtt update process by adding entry
>     first
>   drm/i915: introduce vgt_caps to pvinfo, v2
>   drm/i915/gvt: provide full ppgtt capability for guest, v2
>   drm/i915: enable guest full ppgtt when device model supports, v2
> 
>  drivers/gpu/drm/i915/gvt/gtt.c      | 45 +++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/gvt/vgpu.c     |  1 +
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 11 +++++----
>  drivers/gpu/drm/i915/i915_pvinfo.h  |  8 ++++++-
>  drivers/gpu/drm/i915/i915_vgpu.c    |  7 ++++++
>  drivers/gpu/drm/i915/i915_vgpu.h    |  3 +++
>  7 files changed, 52 insertions(+), 24 deletions(-)
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/4] drm/i915: introduce vgt_caps to pvinfo

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1968 bytes --]

On 2017.05.12 17:37:56 +0800, Tina Zhang wrote:
> vgt_caps is used for guest i915 driver to get the vgpu capabilities from
> the device model. VGT_CPAS_FULL_PPGTT is one of the capabilities type to
> let guest i915 dirver know that the guest i915 full ppgtt is supported by
> device model.
> 
> Changes since v1:
> - Use u32 instead of uint32_t (Joonas)
> - Move VGT_CAPS_FULL_PPGTT introduction to this patch and use #define instead
>   of enum (Joonas)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pvinfo.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
> index c0cb297..8dc0664 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -53,12 +53,18 @@ enum vgt_g2v_type {
>  	VGT_G2V_MAX,
>  };
>  
> +/*
> + * VGT capabilities type
> + */
> +#define VGT_CAPS_FULL_PPGTT	BIT(2)
> +
>  struct vgt_if {
>  	u64 magic;		/* VGT_MAGIC */
>  	uint16_t version_major;
>  	uint16_t version_minor;
>  	u32 vgt_id;		/* ID of vGT instance */
> -	u32 rsv1[12];		/* pad to offset 0x40 */
> +	u32 vgt_caps;		/* VGT capabilities */
> +	u32 rsv1[11];		/* pad to offset 0x40 */

We need to bump pvinfo version for this structural change to keep versioned
pvinfo page for compat check. So in this case I sugguest to bump minor version
and check required pvinfo version when detecting caps.

>  	/*
>  	 *  Data structure to describe the balooning info of resources.
>  	 *  Each VM can only have one portion of continuous area for now.
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Open Source Technology Center, Intel ltd.

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt update process by adding entry first

[Zhang, Tina]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Joonas Lahtinen
> Sent: Monday, May 15, 2017 6:50 PM
> To: Zhang, Tina <tina.zhang@intel.com>; intel-gvt-dev@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt
> update process by adding entry first
> 
> On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote:
> > Guest i915 driver may update the ppgtt table just before this workload
> > is going to be submitted to the hardware by device model. This case
> > wasn't handled well by device model before, due to the small time
> > window between removing old ppgtt entry and adding the new one. Errors
> > occur when the workload is executed by hardware during that small time
> > window. This patch is to remove this time window by adding the new
> > ppgtt entry first and then remove the old one.
> >
> > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> 
> This should be reviewed on the GVT mailing list, and this should include the
> squashed hunk which exports the newly added capability.
Thanks for the comments. I'm sorry I didn’t get it, here. Do you want me to remove this patch from this patch set?
Well, the reason this patch is here is because we met a GPU hang issue when guest i915 using full ppgtt. This is a blocking issue for enabling guest i915 full ppgtt functionality. This patch is to fix that issue. So, with this patch, the device model can have the capability to support guest i915 full ppgtt functionality. And the other patches in this patch set are used for guest to communicate with host whether the full ppgtt capability is supported.

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

Re: [PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt update process by adding entry first

[Joonas Lahtinen]

On to, 2017-05-18 at 02:23 +0000, Zhang, Tina wrote:
> 
> > 
> > -----Original Message-----
> > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> > Behalf Of Joonas Lahtinen
> > Sent: Monday, May 15, 2017 6:50 PM
> > > > To: Zhang, Tina <tina.zhang@intel.com>; intel-gvt-dev@lists.freedesktop.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/gvt: reorder the shadow ppgtt
> > update process by adding entry first
> > 
> > On pe, 2017-05-12 at 17:37 +0800, Tina Zhang wrote:
> > > 
> > > Guest i915 driver may update the ppgtt table just before this workload
> > > is going to be submitted to the hardware by device model. This case
> > > wasn't handled well by device model before, due to the small time
> > > window between removing old ppgtt entry and adding the new one. Errors
> > > occur when the workload is executed by hardware during that small time
> > > window. This patch is to remove this time window by adding the new
> > > ppgtt entry first and then remove the old one.
> > > 
> > > > > > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> > 
> > This should be reviewed on the GVT mailing list, and this should include the
> > squashed hunk which exports the newly added capability.
> Thanks for the comments. I'm sorry I didn’t get it, here. Do you want
> me to remove this patch from this patch set?

It might be easier to merge if this is sent as two separate patch
series, because other gets merged through gvt tree and the other
directly to tip.

> Well, the reason this patch is here is because we met a GPU hang
> issue when guest i915 using full ppgtt. This is a blocking issue for
> enabling guest i915 full ppgtt functionality. This patch is to fix
> that issue. So, with this patch, the device model can have the
> capability to support guest i915 full ppgtt functionality. And the
> other patches in this patch set are used for guest to communicate
> with host whether the full ppgtt capability is supported.

Yes, but to help bisecting, the capability should be exported in the
same patch that implements the capability.

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