[PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d

[PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d

[Chris Wilson]

One of the numerous VT-d workarounds we require is that the display
hardware reads past the end of the buffer triggering VT-d faults. This
is acknowledged in the code as being safe "since we fill the unused
portions of the GGTT with the scratch page". Alas, that is no longer
always true and so we trigger DMAR read faults.

Skylake also requires another workaround to avoid mixing VT-d and
unpopulated PTE, and so there we also need to ensure we fill unused
entries with the scratch page.

Reported-by: Mike Lothian <mike@fireburn.co.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96584
Fixes: f7770bfd9fd2 ("drm/i915: Skip clearing the GGTT on full-ppgtt systems")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  9 +++++++++
 drivers/gpu/drm/i915/i915_gem_gtt.c  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 11 +----------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 442386abd516..217ca3c9b1c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2857,6 +2857,15 @@ struct drm_i915_cmd_table {
 
 #include "i915_trace.h"
 
+static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
+{
+#ifdef CONFIG_INTEL_IOMMU
+	if (INTEL_GEN(dev_priv) >= 6 && intel_iommu_gfx_mapped)
+		return true;
+#endif
+	return false;
+}
+
 extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
 extern int i915_resume_switcheroo(struct drm_device *dev);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a8be1c2a8b9e..033fe10768e0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2608,7 +2608,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
 	ggtt->base.insert_page = gen8_ggtt_insert_page;
 	ggtt->base.clear_range = nop_clear_range;
-	if (!USES_FULL_PPGTT(dev_priv))
+	if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv))
 		ggtt->base.clear_range = gen8_ggtt_clear_range;
 
 	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a8b0ec13dfe9..8145b65c4262 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2071,15 +2071,6 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 		intel_wait_for_pipe_off(crtc);
 }
 
-static bool need_vtd_wa(struct drm_device *dev)
-{
-#ifdef CONFIG_INTEL_IOMMU
-	if (INTEL_INFO(dev)->gen >= 6 && intel_iommu_gfx_mapped)
-		return true;
-#endif
-	return false;
-}
-
 static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv)
 {
 	return IS_GEN2(dev_priv) ? 2048 : 4096;
@@ -2262,7 +2253,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 	 * we should always have valid PTE following the scanout preventing
 	 * the VT-d warning.
 	 */
-	if (need_vtd_wa(dev) && alignment < 256 * 1024)
+	if (intel_scanout_needs_vtd_wa(dev_priv) && alignment < 256 * 1024)
 		alignment = 256 * 1024;
 
 	/*
-- 
2.8.1

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

✓ Ro.CI.BAT: success for drm/i915: Fill unused GGTT with scratch pages for VT-d

[Patchwork]

== Series Details ==

Series: drm/i915: Fill unused GGTT with scratch pages for VT-d
URL   : https://patchwork.freedesktop.org/series/9140/
State : success

== Summary ==

Series 9140v1 drm/i915: Fill unused GGTT with scratch pages for VT-d
http://patchwork.freedesktop.org/api/1.0/series/9140/revisions/1/mbox

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-skl-i5-6260u  total:228  pass:202  dwarn:0   dfail:0   fail:1   skip:25 
fi-snb-i7-2600   total:228  pass:174  dwarn:0   dfail:0   fail:1   skip:53 
ro-bdw-i5-5250u  total:227  pass:201  dwarn:1   dfail:1   fail:1   skip:23 
ro-bdw-i7-5600u  total:227  pass:189  dwarn:0   dfail:1   fail:0   skip:37 
ro-bsw-n3050     total:227  pass:176  dwarn:0   dfail:1   fail:2   skip:48 
ro-byt-n2820     total:227  pass:177  dwarn:0   dfail:1   fail:4   skip:45 
ro-hsw-i3-4010u  total:227  pass:194  dwarn:0   dfail:1   fail:1   skip:31 
ro-hsw-i7-4770r  total:227  pass:194  dwarn:0   dfail:1   fail:1   skip:31 
ro-ilk-i7-620lm  total:227  pass:154  dwarn:0   dfail:1   fail:2   skip:70 
ro-ilk1-i5-650   total:222  pass:154  dwarn:0   dfail:1   fail:2   skip:65 
ro-ivb2-i7-3770  total:227  pass:189  dwarn:0   dfail:1   fail:1   skip:36 
ro-skl3-i5-6260u total:227  pass:205  dwarn:1   dfail:1   fail:1   skip:19 
ro-snb-i7-2620M  total:227  pass:178  dwarn:0   dfail:1   fail:1   skip:47 
fi-hsw-i7-4770k failed to connect after reboot
fi-kbl-qkkr failed to connect after reboot
fi-skl-i7-6700k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1299/

66e29f4 drm-intel-nightly: 2016y-06m-24d-13h-47m-00s UTC integration manifest
d0d1962 drm/i915: Fill unused GGTT with scratch pages for VT-d

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

Re: [PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d

[Chris Wilson]

On Fri, Jun 24, 2016 at 02:07:14PM +0100, Chris Wilson wrote:
> One of the numerous VT-d workarounds we require is that the display
> hardware reads past the end of the buffer triggering VT-d faults. This
> is acknowledged in the code as being safe "since we fill the unused
> portions of the GGTT with the scratch page". Alas, that is no longer
> always true and so we trigger DMAR read faults.
> 
> Skylake also requires another workaround to avoid mixing VT-d and
> unpopulated PTE, and so there we also need to ensure we fill unused
> entries with the scratch page.
> 
> Reported-by: Mike Lothian <mike@fireburn.co.uk>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96584
> Fixes: f7770bfd9fd2 ("drm/i915: Skip clearing the GGTT on full-ppgtt systems")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

Re: [PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d

[David Weinehall]

On Fri, Jun 24, 2016 at 02:07:14PM +0100, Chris Wilson wrote:
> One of the numerous VT-d workarounds we require is that the display
> hardware reads past the end of the buffer triggering VT-d faults. This
> is acknowledged in the code as being safe "since we fill the unused
> portions of the GGTT with the scratch page". Alas, that is no longer
> always true and so we trigger DMAR read faults.
> 
> Skylake also requires another workaround to avoid mixing VT-d and
> unpopulated PTE, and so there we also need to ensure we fill unused
> entries with the scratch page.

LGTM.

Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

> 
> Reported-by: Mike Lothian <mike@fireburn.co.uk>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96584
> Fixes: f7770bfd9fd2 ("drm/i915: Skip clearing the GGTT on full-ppgtt systems")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  9 +++++++++
>  drivers/gpu/drm/i915/i915_gem_gtt.c  |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 11 +----------
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 442386abd516..217ca3c9b1c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2857,6 +2857,15 @@ struct drm_i915_cmd_table {
>  
>  #include "i915_trace.h"
>  
> +static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
> +{
> +#ifdef CONFIG_INTEL_IOMMU
> +	if (INTEL_GEN(dev_priv) >= 6 && intel_iommu_gfx_mapped)
> +		return true;
> +#endif
> +	return false;
> +}
> +
>  extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
>  extern int i915_resume_switcheroo(struct drm_device *dev);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a8be1c2a8b9e..033fe10768e0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2608,7 +2608,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>  	ggtt->base.unbind_vma = ggtt_unbind_vma;
>  	ggtt->base.insert_page = gen8_ggtt_insert_page;
>  	ggtt->base.clear_range = nop_clear_range;
> -	if (!USES_FULL_PPGTT(dev_priv))
> +	if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv))
>  		ggtt->base.clear_range = gen8_ggtt_clear_range;
>  
>  	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a8b0ec13dfe9..8145b65c4262 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2071,15 +2071,6 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>  		intel_wait_for_pipe_off(crtc);
>  }
>  
> -static bool need_vtd_wa(struct drm_device *dev)
> -{
> -#ifdef CONFIG_INTEL_IOMMU
> -	if (INTEL_INFO(dev)->gen >= 6 && intel_iommu_gfx_mapped)
> -		return true;
> -#endif
> -	return false;
> -}
> -
>  static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv)
>  {
>  	return IS_GEN2(dev_priv) ? 2048 : 4096;
> @@ -2262,7 +2253,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  	 * we should always have valid PTE following the scanout preventing
>  	 * the VT-d warning.
>  	 */
> -	if (need_vtd_wa(dev) && alignment < 256 * 1024)
> +	if (intel_scanout_needs_vtd_wa(dev_priv) && alignment < 256 * 1024)
>  		alignment = 256 * 1024;
>  
>  	/*
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d

[Chris Wilson]

On Fri, Jul 08, 2016 at 03:29:30PM +0300, David Weinehall wrote:
> On Fri, Jun 24, 2016 at 02:07:14PM +0100, Chris Wilson wrote:
> > One of the numerous VT-d workarounds we require is that the display
> > hardware reads past the end of the buffer triggering VT-d faults. This
> > is acknowledged in the code as being safe "since we fill the unused
> > portions of the GGTT with the scratch page". Alas, that is no longer
> > always true and so we trigger DMAR read faults.
> > 
> > Skylake also requires another workaround to avoid mixing VT-d and
> > unpopulated PTE, and so there we also need to ensure we fill unused
> > entries with the scratch page.
> 
> LGTM.
> 
> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

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

Re: [PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d

[Joonas Lahtinen]

On pe, 2016-06-24 at 14:07 +0100, Chris Wilson wrote:
> One of the numerous VT-d workarounds we require is that the display
> hardware reads past the end of the buffer triggering VT-d faults. This
> is acknowledged in the code as being safe "since we fill the unused
> portions of the GGTT with the scratch page". Alas, that is no longer
> always true and so we trigger DMAR read faults.
> 
> Skylake also requires another workaround to avoid mixing VT-d and
> unpopulated PTE, and so there we also need to ensure we fill unused
> entries with the scratch page.

Rather agressive W/A for just scanout, so maybe drop the scanout word?

> 
> Reported-by: Mike Lothian <mike@fireburn.co.uk>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96584
> Fixes: f7770bfd9fd2 ("drm/i915: Skip clearing the GGTT on full-ppgtt systems")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  9 +++++++++
>  drivers/gpu/drm/i915/i915_gem_gtt.c  |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 11 +----------
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 442386abd516..217ca3c9b1c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2857,6 +2857,15 @@ struct drm_i915_cmd_table {
>  
>  #include "i915_trace.h"
>  
> +static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
> +{
> +#ifdef CONFIG_INTEL_IOMMU
> +	if (INTEL_GEN(dev_priv) >= 6 && intel_iommu_gfx_mapped)
> +		return true;
> +#endif
> +	return false;
> +}
> +
>  extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
>  extern int i915_resume_switcheroo(struct drm_device *dev);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a8be1c2a8b9e..033fe10768e0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2608,7 +2608,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>  	ggtt->base.unbind_vma = ggtt_unbind_vma;
>  	ggtt->base.insert_page = gen8_ggtt_insert_page;
>  	ggtt->base.clear_range = nop_clear_range;
> -	if (!USES_FULL_PPGTT(dev_priv))
> +	if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv))
>  		ggtt->base.clear_range = gen8_ggtt_clear_range;
>  
>  	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a8b0ec13dfe9..8145b65c4262 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2071,15 +2071,6 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>  		intel_wait_for_pipe_off(crtc);
>  }
>  
> -static bool need_vtd_wa(struct drm_device *dev)
> -{
> -#ifdef CONFIG_INTEL_IOMMU
> -	if (INTEL_INFO(dev)->gen >= 6 && intel_iommu_gfx_mapped)
> -		return true;
> -#endif
> -	return false;
> -}
> -
>  static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv)
>  {
>  	return IS_GEN2(dev_priv) ? 2048 : 4096;
> @@ -2262,7 +2253,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  	 * we should always have valid PTE following the scanout preventing
>  	 * the VT-d warning.
>  	 */
> -	if (need_vtd_wa(dev) && alignment < 256 * 1024)
> +	if (intel_scanout_needs_vtd_wa(dev_priv) && alignment < 256 * 1024)
>  		alignment = 256 * 1024;
>  
>  	/*
-- 
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] drm/i915: Fill unused GGTT with scratch pages for VT-d

[Chris Wilson]

On Tue, Jul 12, 2016 at 11:50:26AM +0300, Joonas Lahtinen wrote:
> On pe, 2016-06-24 at 14:07 +0100, Chris Wilson wrote:
> > One of the numerous VT-d workarounds we require is that the display
> > hardware reads past the end of the buffer triggering VT-d faults. This
> > is acknowledged in the code as being safe "since we fill the unused
> > portions of the GGTT with the scratch page". Alas, that is no longer
> > always true and so we trigger DMAR read faults.
> > 
> > Skylake also requires another workaround to avoid mixing VT-d and
> > unpopulated PTE, and so there we also need to ensure we fill unused
> > entries with the scratch page.
> 
> Rather agressive W/A for just scanout, so maybe drop the scanout word?

The original w/a was just for VT'd + scanout, I was thinking of a
comment, but it never materialised.
-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