[PATCH v9 2/3] drm/i915: Enable guest i915 full ppgtt functionality

[PATCH v9 2/3] drm/i915: Enable guest i915 full ppgtt functionality

[Tina Zhang]

Enable the guest i915 full ppgtt functionality when host can provide this
capability. vgt_caps is introduced to 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.

Notice that the minor version of pvinfo isn't bumped because of this
vgt_caps introduction, due to older guest would be broken by simply
increasing the pvinfo version. Although the pvinfo minor version doesn't
increase, the compatibility won't be blocked. The compatibility is ensured
by checking the value of caps field in pvinfo. Zero means no full ppgtt
support and BIT(2) means this feature is provided.

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)
- Rewrite the vgpu full ppgtt capability checking logic. (Joonas)
- Some coding style refine. (Joonas)

Changes since v2:
- Divide the whole patch set into two separate patch series, with one
  patch in i915 side to check guest i915 full ppgtt capability and enable
  it when this capability is supported by the device model, and the other
  one in gvt side which fixs the blocking issue and enables the device
  model to provide the capability to guest. And this patch focuses on guest
  i915 side. (Joonas)
- Change the title from "introduce vgt_caps to pvinfo" to
  "Enable guest i915 full ppgtt functionality". (Tina)

Change since v3:
- Add some comments about pvinfo caps and version. (Joonas)

Change since v4:
- Tested by Tina Zhang.

Change since v5:
- Add limitation about supporting 32bit full ppgtt.

Change since v6:
- Change the fallback to 48bit full ppgtt if i915.ppgtt_enable=2. (Zhenyu)

Change in v9:
- Remove the fixme comment due to no plan for 32bit full ppgtt
  support. (Zhenyu)
- Reorder the patch-set to fix compiling issue with git-bisect. (Zhenyu)
- Add print log when forcing guest 48bit full ppgtt. (Zhenyu)

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> in v2
Signed-off-by: Tina Zhang <tina.zhang@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b051122..1b95069 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1879,6 +1879,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 10aa776..090ceb7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -141,14 +141,19 @@ 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;
+		has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv);
+		/* GVT-g has no support for 32bit ppgtt */
+		if (enable_ppgtt == 2 && has_full_ppgtt) {
+			DRM_DEBUG_DRIVER("Force 48bit ppgtt for vGPU\n");
+			enable_ppgtt = 3;
+		}
 	}
 
+	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_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
index 2cfe96d3..7e958d7 100644
--- a/drivers/gpu/drm/i915/i915_pvinfo.h
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -49,12 +49,18 @@ enum vgt_g2v_type {
 	VGT_G2V_MAX,
 };
 
+/*
+ * VGT capabilities type
+ */
+#define VGT_CAPS_FULL_PPGTT_48BIT	BIT(2)
+
 struct vgt_if {
 	u64 magic;		/* VGT_MAGIC */
 	u16 version_major;
 	u16 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.
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index cf7a958..e85e27c 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -75,10 +75,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_48BIT;
+}
+
 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

Re: [PATCH v9 0/3] drm/i915/gvt: Enable guest i915 48bit full ppgtt support

[Zhenyu Wang]

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

On 2017.08.10 07:41:35 +0800, Tina Zhang wrote:
> This patch set is used to enable guest i915 48bit full ppgtt support.
> 
> Kechen Lu (1):
>   drm/i915/gvt: Add shadow context descriptor updating
> 
> Tina Zhang (2):
>   drm/i915: Enable guest i915 full ppgtt functionality
>   drm/i915/gvt: Fix guest i915 full ppgtt blocking issue
> 
>  drivers/gpu/drm/i915/gvt/execlist.c  |  2 ++
>  drivers/gpu/drm/i915/gvt/gtt.c       | 45 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/gvt/gvt.h       |  1 +
>  drivers/gpu/drm/i915/gvt/scheduler.c | 23 ++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/vgpu.c      |  1 +
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/i915_gem_gtt.c  | 13 +++++++----
>  drivers/gpu/drm/i915/i915_pvinfo.h   |  8 ++++++-
>  drivers/gpu/drm/i915/i915_vgpu.c     |  7 ++++++
>  drivers/gpu/drm/i915/i915_vgpu.h     |  3 +++
>  10 files changed, 82 insertions(+), 22 deletions(-)
> 

As agreed before and need to apply this series in order, i915 change
for this will be merged through gvt pull.

Applied this to gvt-next. Thanks!

-- 
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 v9 2/3] drm/i915: Enable guest i915 full ppgtt functionality

[Joonas Lahtinen]

On to, 2017-08-10 at 07:41 +0800, Tina Zhang wrote:
> Enable the guest i915 full ppgtt functionality when host can provide this
> capability. vgt_caps is introduced to 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.
> 
> Notice that the minor version of pvinfo isn't bumped because of this
> vgt_caps introduction, due to older guest would be broken by simply
> increasing the pvinfo version. Although the pvinfo minor version doesn't
> increase, the compatibility won't be blocked. The compatibility is ensured
> by checking the value of caps field in pvinfo. Zero means no full ppgtt
> support and BIT(2) means this feature is provided.
> 
> 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)
> - Rewrite the vgpu full ppgtt capability checking logic. (Joonas)
> - Some coding style refine. (Joonas)
> 
> Changes since v2:
> - Divide the whole patch set into two separate patch series, with one
>   patch in i915 side to check guest i915 full ppgtt capability and enable
>   it when this capability is supported by the device model, and the other
>   one in gvt side which fixs the blocking issue and enables the device
>   model to provide the capability to guest. And this patch focuses on guest
>   i915 side. (Joonas)
> - Change the title from "introduce vgt_caps to pvinfo" to
>   "Enable guest i915 full ppgtt functionality". (Tina)
> 
> Change since v3:
> - Add some comments about pvinfo caps and version. (Joonas)
> 
> Change since v4:
> - Tested by Tina Zhang.
> 
> Change since v5:
> - Add limitation about supporting 32bit full ppgtt.
> 
> Change since v6:
> - Change the fallback to 48bit full ppgtt if i915.ppgtt_enable=2. (Zhenyu)
> 
> Change in v9:
> - Remove the fixme comment due to no plan for 32bit full ppgtt
>   support. (Zhenyu)
> - Reorder the patch-set to fix compiling issue with git-bisect. (Zhenyu)
> - Add print log when forcing guest 48bit full ppgtt. (Zhenyu)
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> in v2
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>

<SNIP>

> @@ -141,14 +141,19 @@ 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;

	Leave above line.

>  
>  	if (intel_vgpu_active(dev_priv)) {
> -		/* emulation is too hard */
> -		has_full_ppgtt = false;
> -		has_full_48bit_ppgtt = false;

		has_full_ppgtt = false;
		has_full_48bit_ppgtt =
			intel_vgpu_has_full_48bit_ppgtt(dev_priv);

> +		has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv);
> +		/* GVT-g has no support for 32bit ppgtt */
> +		if (enable_ppgtt == 2 && has_full_ppgtt) {
> +			DRM_DEBUG_DRIVER("Force 48bit ppgtt for vGPU\n");
> +			enable_ppgtt = 3;

Lets not do an upgrade for the user if they explicitly requested lower
level. I'd just leave the option to be and it'll get downgraded to
aliasing ppgtt.

> +		}
>  	}
>  
> +	has_full_48bit_ppgtt = has_full_ppgtt &&
> +		dev_priv->info.has_full_48bit_ppgtt;
> +

This can then be dropped too.

I'll send a patch to disconnect the has_full_ppgtt and
has_full_48bit_ppgtt flags.


>  	if (!has_aliasing_ppgtt)
>  		return 0;
>  
> @@ -49,12 +49,18 @@ enum vgt_g2v_type {
> >  	VGT_G2V_MAX,
>  };
>  
> +/*
> + * VGT capabilities type
> + */
> +#define VGT_CAPS_FULL_PPGTT_48BIT	BIT(2)

VGT_CAPS_FULL_48BIT_PPGTT for consistency

> @@ -75,10 +75,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)

This should really be has_full_48bit_ppgtt() too.

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 v9 2/3] drm/i915: Enable guest i915 full ppgtt functionality

[Zhenyu Wang]

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

On 2017.08.11 12:46:48 +0300, Joonas Lahtinen wrote:
> 
> > @@ -141,14 +141,19 @@ 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;
> 
> 	Leave above line.
> 
> >  
> >  	if (intel_vgpu_active(dev_priv)) {
> > -		/* emulation is too hard */
> > -		has_full_ppgtt = false;
> > -		has_full_48bit_ppgtt = false;
> 
> 		has_full_ppgtt = false;
> 		has_full_48bit_ppgtt =
> 			intel_vgpu_has_full_48bit_ppgtt(dev_priv);
> 
> > +		has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv);
> > +		/* GVT-g has no support for 32bit ppgtt */
> > +		if (enable_ppgtt == 2 && has_full_ppgtt) {
> > +			DRM_DEBUG_DRIVER("Force 48bit ppgtt for vGPU\n");
> > +			enable_ppgtt = 3;
> 
> Lets not do an upgrade for the user if they explicitly requested lower
> level. I'd just leave the option to be and it'll get downgraded to
> aliasing ppgtt.
>

If user specified enable_ppgtt=2, it will get enable_ppgtt=2 instead of
aliasing ppgtt, so we try to guard against that for current 48bit ppgtt
support on vgpu. Well either upgrade to full 48bit or downgrade to aliasing.

> > +		}
> >  	}
> >  
> > +	has_full_48bit_ppgtt = has_full_ppgtt &&
> > +		dev_priv->info.has_full_48bit_ppgtt;
> > +
> 
> This can then be dropped too.
> 
> I'll send a patch to disconnect the has_full_ppgtt and
> has_full_48bit_ppgtt flags.
>

That'll be good. As we have validated this full ppgtt on vgpu for
quite some time now and plan to push for 4.14. Are you able to
align with that? So I might send first 4.14 pull without this series,
then add them later for dinf.


> 
> >  	if (!has_aliasing_ppgtt)
> >  		return 0;
> >  
> > @@ -49,12 +49,18 @@ enum vgt_g2v_type {
> > >  	VGT_G2V_MAX,
> >  };
> >  
> > +/*
> > + * VGT capabilities type
> > + */
> > +#define VGT_CAPS_FULL_PPGTT_48BIT	BIT(2)
> 
> VGT_CAPS_FULL_48BIT_PPGTT for consistency
> 
> > @@ -75,10 +75,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)
> 
> This should really be has_full_48bit_ppgtt() too.
> 
> Regards, Joonas
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation

-- 
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