[PATCH] drm/i915/vgpu: Disallow loading on old vGPU hosts

[PATCH] drm/i915/vgpu: Disallow loading on old vGPU hosts

[Chris Wilson]

Since commit fd8526e50902 ("drm/i915/execlists: Trust the CSB") we
actually broke the force-mmio mode for our execlists implementation. No
one noticed, so ergo no one is actually using an old vGPU host (where we
required the older method) and so can simply remove the broken support.

Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Fixes: fd8526e50902 ("drm/i915/execlists: Trust the CSB")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 14 +++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        | 32 +++++++------------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  9 -------
 3 files changed, 23 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e39016713464..3e5e2efce670 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1384,6 +1384,20 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		}
 	}
 
+	if (HAS_EXECLISTS(dev_priv)) {
+		/*
+		 * Older GVT emulation depends upon intercepting CSB mmio,
+		 * which we no longer use, preferring to use the HWSP cache
+		 * instead.
+		 */
+		if (intel_vgpu_active(dev_priv) &&
+		    !intel_vgpu_has_hwsp_emulation(dev_priv)) {
+			i915_report_error(dev_priv,
+					  "old vGPU host found, support for HWSP emulation required\n");
+			return -ENXIO;
+		}
+	}
+
 	intel_sanitize_options(dev_priv);
 
 	i915_perf_init(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0a690c557113..1848ca2bf9ee 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -748,6 +748,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
 
 static void reset_csb_pointers(struct intel_engine_execlists *execlists)
 {
+	const unsigned int reset_value = GEN8_CSB_ENTRIES - 1;
+
 	/*
 	 * After a reset, the HW starts writing into CSB entry [0]. We
 	 * therefore have to set our HEAD pointer back one entry so that
@@ -757,8 +759,8 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
 	 * inline comparison of our cached head position against the last HW
 	 * write works even before the first interrupt.
 	 */
-	execlists->csb_head = execlists->csb_write_reset;
-	WRITE_ONCE(*execlists->csb_write, execlists->csb_write_reset);
+	execlists->csb_head = reset_value;
+	WRITE_ONCE(*execlists->csb_write, reset_value);
 }
 
 static void nop_submission_tasklet(unsigned long data)
@@ -2213,12 +2215,6 @@ logical_ring_setup(struct intel_engine_cs *engine)
 	logical_ring_default_irqs(engine);
 }
 
-static bool csb_force_mmio(struct drm_i915_private *i915)
-{
-	/* Older GVT emulation depends upon intercepting CSB mmio */
-	return intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915);
-}
-
 static int logical_ring_init(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *i915 = engine->i915;
@@ -2250,22 +2246,12 @@ static int logical_ring_init(struct intel_engine_cs *engine)
 
 	execlists->csb_read =
 		i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
-	if (csb_force_mmio(i915)) {
-		execlists->csb_status = (u32 __force *)
-			(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)));
-
-		execlists->csb_write = (u32 __force *)execlists->csb_read;
-		execlists->csb_write_reset =
-			_MASKED_FIELD(GEN8_CSB_WRITE_PTR_MASK,
-				      GEN8_CSB_ENTRIES - 1);
-	} else {
-		execlists->csb_status =
-			&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
+	execlists->csb_status =
+		&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
+
+	execlists->csb_write =
+		&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
 
-		execlists->csb_write =
-			&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
-		execlists->csb_write_reset = GEN8_CSB_ENTRIES - 1;
-	}
 	reset_csb_pointers(execlists);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 970fb5c05c36..2f7d1ce54f1e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -338,15 +338,6 @@ struct intel_engine_execlists {
 	 */
 	u32 preempt_complete_status;
 
-	/**
-	 * @csb_write_reset: reset value for CSB write pointer
-	 *
-	 * As the CSB write pointer maybe either in HWSP or as a field
-	 * inside an mmio register, we want to reprogram it slightly
-	 * differently to avoid later confusion.
-	 */
-	u32 csb_write_reset;
-
 	/**
 	 * @csb_head: context status buffer head
 	 */
-- 
2.20.0.rc1

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

Re: [PATCH] drm/i915/vgpu: Disallow loading on old vGPU hosts

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since commit fd8526e50902 ("drm/i915/execlists: Trust the CSB") we
> actually broke the force-mmio mode for our execlists implementation. No
> one noticed, so ergo no one is actually using an old vGPU host (where we
> required the older method) and so can simply remove the broken support.
>
> Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Fixes: fd8526e50902 ("drm/i915/execlists: Trust the CSB")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 14 +++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c        | 32 +++++++------------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  9 -------
>  3 files changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e39016713464..3e5e2efce670 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1384,6 +1384,20 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  		}
>  	}
>  
> +	if (HAS_EXECLISTS(dev_priv)) {
> +		/*
> +		 * Older GVT emulation depends upon intercepting CSB mmio,
> +		 * which we no longer use, preferring to use the HWSP cache
> +		 * instead.
> +		 */
> +		if (intel_vgpu_active(dev_priv) &&
> +		    !intel_vgpu_has_hwsp_emulation(dev_priv)) {
> +			i915_report_error(dev_priv,
> +					  "old vGPU host found, support for HWSP emulation required\n");
> +			return -ENXIO;
> +		}
> +	}
> +
>  	intel_sanitize_options(dev_priv);
>  
>  	i915_perf_init(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0a690c557113..1848ca2bf9ee 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -748,6 +748,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>  
>  static void reset_csb_pointers(struct intel_engine_execlists *execlists)
>  {
> +	const unsigned int reset_value = GEN8_CSB_ENTRIES - 1;
> +
>  	/*
>  	 * After a reset, the HW starts writing into CSB entry [0]. We
>  	 * therefore have to set our HEAD pointer back one entry so that
> @@ -757,8 +759,8 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
>  	 * inline comparison of our cached head position against the last HW
>  	 * write works even before the first interrupt.
>  	 */
> -	execlists->csb_head = execlists->csb_write_reset;
> -	WRITE_ONCE(*execlists->csb_write, execlists->csb_write_reset);
> +	execlists->csb_head = reset_value;
> +	WRITE_ONCE(*execlists->csb_write, reset_value);
>  }
>  
>  static void nop_submission_tasklet(unsigned long data)
> @@ -2213,12 +2215,6 @@ logical_ring_setup(struct intel_engine_cs *engine)
>  	logical_ring_default_irqs(engine);
>  }
>  
> -static bool csb_force_mmio(struct drm_i915_private *i915)
> -{
> -	/* Older GVT emulation depends upon intercepting CSB mmio */
> -	return intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915);
> -}
> -
>  static int logical_ring_init(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_private *i915 = engine->i915;
> @@ -2250,22 +2246,12 @@ static int logical_ring_init(struct intel_engine_cs *engine)
>  
>  	execlists->csb_read =
>  		i915->regs +
>  		i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));

This can go too?
-Mika

> -	if (csb_force_mmio(i915)) {
> -		execlists->csb_status = (u32 __force *)
> -			(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)));
> -
> -		execlists->csb_write = (u32 __force *)execlists->csb_read;
> -		execlists->csb_write_reset =
> -			_MASKED_FIELD(GEN8_CSB_WRITE_PTR_MASK,
> -				      GEN8_CSB_ENTRIES - 1);
> -	} else {
> -		execlists->csb_status =
> -			&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
> +	execlists->csb_status =
> +		&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
> +
> +	execlists->csb_write =
> +		&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
>  
> -		execlists->csb_write =
> -			&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
> -		execlists->csb_write_reset = GEN8_CSB_ENTRIES - 1;
> -	}
>  	reset_csb_pointers(execlists);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 970fb5c05c36..2f7d1ce54f1e 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -338,15 +338,6 @@ struct intel_engine_execlists {
>  	 */
>  	u32 preempt_complete_status;
>  
> -	/**
> -	 * @csb_write_reset: reset value for CSB write pointer
> -	 *
> -	 * As the CSB write pointer maybe either in HWSP or as a field
> -	 * inside an mmio register, we want to reprogram it slightly
> -	 * differently to avoid later confusion.
> -	 */
> -	u32 csb_write_reset;
> -
>  	/**
>  	 * @csb_head: context status buffer head
>  	 */
> -- 
> 2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/vgpu: Disallow loading on old vGPU hosts

[Patchwork]

== Series Details ==

Series: drm/i915/vgpu: Disallow loading on old vGPU hosts
URL   : https://patchwork.freedesktop.org/series/53311/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5230 -> Patchwork_10977
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53311/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_10977:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_flip@basic-flip-vs-dpms:
    - {fi-icl-u3}:        PASS -> DMESG-WARN

  * {igt@runner@aborted}:
    - {fi-icl-u3}:        NOTRUN -> FAIL

  
Known issues
------------

  Here are the changes found in Patchwork_10977 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_coherency:
    - fi-gdg-551:         PASS -> DMESG-FAIL [fdo#107164]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-kbl-7560u:       INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-apl-guc:         DMESG-WARN [fdo#108566] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#107164]: https://bugs.freedesktop.org/show_bug.cgi?id=107164
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566


Participating hosts (50 -> 43)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * Linux: CI_DRM_5230 -> Patchwork_10977

  CI_DRM_5230: b0a2de64f8969163f6e01071d5e05748f18a8bab @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10977: 2c6a54e8583e69035ef5cb888c0f04a93ae56a89 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2c6a54e8583e drm/i915/vgpu: Disallow loading on old vGPU hosts

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10977/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915/vgpu: Disallow loading on old vGPU hosts

[Chris Wilson]

Since commit fd8526e50902 ("drm/i915/execlists: Trust the CSB") we
actually broke the force-mmio mode for our execlists implementation. No
one noticed, so ergo no one is actually using an old vGPU host (where we
required the older method) and so can simply remove the broken support.

v2: csb_read can go as well (Mika)

Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Fixes: fd8526e50902 ("drm/i915/execlists: Trust the CSB")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 14 +++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        | 32 +++++++------------------
 drivers/gpu/drm/i915/intel_ringbuffer.h | 16 -------------
 3 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e39016713464..3e5e2efce670 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1384,6 +1384,20 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		}
 	}
 
+	if (HAS_EXECLISTS(dev_priv)) {
+		/*
+		 * Older GVT emulation depends upon intercepting CSB mmio,
+		 * which we no longer use, preferring to use the HWSP cache
+		 * instead.
+		 */
+		if (intel_vgpu_active(dev_priv) &&
+		    !intel_vgpu_has_hwsp_emulation(dev_priv)) {
+			i915_report_error(dev_priv,
+					  "old vGPU host found, support for HWSP emulation required\n");
+			return -ENXIO;
+		}
+	}
+
 	intel_sanitize_options(dev_priv);
 
 	i915_perf_init(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0a690c557113..bb5abd4f7516 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -748,6 +748,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
 
 static void reset_csb_pointers(struct intel_engine_execlists *execlists)
 {
+	const unsigned int reset_value = GEN8_CSB_ENTRIES - 1;
+
 	/*
 	 * After a reset, the HW starts writing into CSB entry [0]. We
 	 * therefore have to set our HEAD pointer back one entry so that
@@ -757,8 +759,8 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
 	 * inline comparison of our cached head position against the last HW
 	 * write works even before the first interrupt.
 	 */
-	execlists->csb_head = execlists->csb_write_reset;
-	WRITE_ONCE(*execlists->csb_write, execlists->csb_write_reset);
+	execlists->csb_head = reset_value;
+	WRITE_ONCE(*execlists->csb_write, reset_value);
 }
 
 static void nop_submission_tasklet(unsigned long data)
@@ -2213,12 +2215,6 @@ logical_ring_setup(struct intel_engine_cs *engine)
 	logical_ring_default_irqs(engine);
 }
 
-static bool csb_force_mmio(struct drm_i915_private *i915)
-{
-	/* Older GVT emulation depends upon intercepting CSB mmio */
-	return intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915);
-}
-
 static int logical_ring_init(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *i915 = engine->i915;
@@ -2248,24 +2244,12 @@ static int logical_ring_init(struct intel_engine_cs *engine)
 			upper_32_bits(ce->lrc_desc);
 	}
 
-	execlists->csb_read =
-		i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
-	if (csb_force_mmio(i915)) {
-		execlists->csb_status = (u32 __force *)
-			(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)));
+	execlists->csb_status =
+		&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
 
-		execlists->csb_write = (u32 __force *)execlists->csb_read;
-		execlists->csb_write_reset =
-			_MASKED_FIELD(GEN8_CSB_WRITE_PTR_MASK,
-				      GEN8_CSB_ENTRIES - 1);
-	} else {
-		execlists->csb_status =
-			&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
+	execlists->csb_write =
+		&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
 
-		execlists->csb_write =
-			&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
-		execlists->csb_write_reset = GEN8_CSB_ENTRIES - 1;
-	}
 	reset_csb_pointers(execlists);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 970fb5c05c36..096043b784f0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -312,13 +312,6 @@ struct intel_engine_execlists {
 	 */
 	struct rb_root_cached queue;
 
-	/**
-	 * @csb_read: control register for Context Switch buffer
-	 *
-	 * Note this register is always in mmio.
-	 */
-	u32 __iomem *csb_read;
-
 	/**
 	 * @csb_write: control register for Context Switch buffer
 	 *
@@ -338,15 +331,6 @@ struct intel_engine_execlists {
 	 */
 	u32 preempt_complete_status;
 
-	/**
-	 * @csb_write_reset: reset value for CSB write pointer
-	 *
-	 * As the CSB write pointer maybe either in HWSP or as a field
-	 * inside an mmio register, we want to reprogram it slightly
-	 * differently to avoid later confusion.
-	 */
-	u32 csb_write_reset;
-
 	/**
 	 * @csb_head: context status buffer head
 	 */
-- 
2.20.0.rc1

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

Re: [PATCH v2] drm/i915/vgpu: Disallow loading on old vGPU hosts

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since commit fd8526e50902 ("drm/i915/execlists: Trust the CSB") we
> actually broke the force-mmio mode for our execlists implementation. No
> one noticed, so ergo no one is actually using an old vGPU host (where we
> required the older method) and so can simply remove the broken support.
>
> v2: csb_read can go as well (Mika)
>
> Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Fixes: fd8526e50902 ("drm/i915/execlists: Trust the CSB")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 14 +++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c        | 32 +++++++------------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 16 -------------
>  3 files changed, 22 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e39016713464..3e5e2efce670 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1384,6 +1384,20 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  		}
>  	}
>  
> +	if (HAS_EXECLISTS(dev_priv)) {
> +		/*
> +		 * Older GVT emulation depends upon intercepting CSB mmio,
> +		 * which we no longer use, preferring to use the HWSP cache
> +		 * instead.
> +		 */
> +		if (intel_vgpu_active(dev_priv) &&
> +		    !intel_vgpu_has_hwsp_emulation(dev_priv)) {
> +			i915_report_error(dev_priv,
> +					  "old vGPU host found, support for HWSP emulation required\n");
> +			return -ENXIO;
> +		}
> +	}
> +
>  	intel_sanitize_options(dev_priv);
>  
>  	i915_perf_init(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0a690c557113..bb5abd4f7516 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -748,6 +748,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>  
>  static void reset_csb_pointers(struct intel_engine_execlists *execlists)
>  {
> +	const unsigned int reset_value = GEN8_CSB_ENTRIES - 1;
> +
>  	/*
>  	 * After a reset, the HW starts writing into CSB entry [0]. We
>  	 * therefore have to set our HEAD pointer back one entry so that
> @@ -757,8 +759,8 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
>  	 * inline comparison of our cached head position against the last HW
>  	 * write works even before the first interrupt.
>  	 */
> -	execlists->csb_head = execlists->csb_write_reset;
> -	WRITE_ONCE(*execlists->csb_write, execlists->csb_write_reset);
> +	execlists->csb_head = reset_value;
> +	WRITE_ONCE(*execlists->csb_write, reset_value);
>  }
>  
>  static void nop_submission_tasklet(unsigned long data)
> @@ -2213,12 +2215,6 @@ logical_ring_setup(struct intel_engine_cs *engine)
>  	logical_ring_default_irqs(engine);
>  }
>  
> -static bool csb_force_mmio(struct drm_i915_private *i915)
> -{
> -	/* Older GVT emulation depends upon intercepting CSB mmio */
> -	return intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915);
> -}
> -
>  static int logical_ring_init(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_private *i915 = engine->i915;
> @@ -2248,24 +2244,12 @@ static int logical_ring_init(struct intel_engine_cs *engine)
>  			upper_32_bits(ce->lrc_desc);
>  	}
>  
> -	execlists->csb_read =
> -		i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
> -	if (csb_force_mmio(i915)) {
> -		execlists->csb_status = (u32 __force *)
> -			(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)));
> +	execlists->csb_status =
> +		&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
>  
> -		execlists->csb_write = (u32 __force *)execlists->csb_read;
> -		execlists->csb_write_reset =
> -			_MASKED_FIELD(GEN8_CSB_WRITE_PTR_MASK,
> -				      GEN8_CSB_ENTRIES - 1);
> -	} else {
> -		execlists->csb_status =
> -			&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
> +	execlists->csb_write =
> +		&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
>  
> -		execlists->csb_write =
> -			&engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
> -		execlists->csb_write_reset = GEN8_CSB_ENTRIES - 1;
> -	}
>  	reset_csb_pointers(execlists);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 970fb5c05c36..096043b784f0 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -312,13 +312,6 @@ struct intel_engine_execlists {
>  	 */
>  	struct rb_root_cached queue;
>  
> -	/**
> -	 * @csb_read: control register for Context Switch buffer
> -	 *
> -	 * Note this register is always in mmio.
> -	 */
> -	u32 __iomem *csb_read;
> -
>  	/**
>  	 * @csb_write: control register for Context Switch buffer
>  	 *
> @@ -338,15 +331,6 @@ struct intel_engine_execlists {
>  	 */
>  	u32 preempt_complete_status;
>  
> -	/**
> -	 * @csb_write_reset: reset value for CSB write pointer
> -	 *
> -	 * As the CSB write pointer maybe either in HWSP or as a field
> -	 * inside an mmio register, we want to reprogram it slightly
> -	 * differently to avoid later confusion.
> -	 */
> -	u32 csb_write_reset;
> -
>  	/**
>  	 * @csb_head: context status buffer head
>  	 */
> -- 
> 2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/vgpu: Disallow loading on old vGPU hosts

[Chris Wilson]

Quoting Mika Kuoppala (2018-12-03 13:40:26)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since commit fd8526e50902 ("drm/i915/execlists: Trust the CSB") we
> > actually broke the force-mmio mode for our execlists implementation. No
> > one noticed, so ergo no one is actually using an old vGPU host (where we
> > required the older method) and so can simply remove the broken support.
> >
> > v2: csb_read can go as well (Mika)
> >
> > Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Fixes: fd8526e50902 ("drm/i915/execlists: Trust the CSB")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Pushed, thanks. That should make your extended CSB patches for icl that
much more straightforward, right?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/vgpu: Disallow loading on old vGPU hosts

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2018-12-03 13:40:26)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Since commit fd8526e50902 ("drm/i915/execlists: Trust the CSB") we
>> > actually broke the force-mmio mode for our execlists implementation. No
>> > one noticed, so ergo no one is actually using an old vGPU host (where we
>> > required the older method) and so can simply remove the broken support.
>> >
>> > v2: csb_read can go as well (Mika)
>> >
>> > Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> > Fixes: fd8526e50902 ("drm/i915/execlists: Trust the CSB")
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> 
>> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>
> Pushed, thanks. That should make your extended CSB patches for icl that
> much more straightforward, right?

Very much so. Now just have to figure out how to make the
cpu/gpu dance together. The extended CSB is not enough
by itself :(

-Mika

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

✓ Fi.CI.BAT: success for drm/i915/vgpu: Disallow loading on old vGPU hosts (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/vgpu: Disallow loading on old vGPU hosts (rev2)
URL   : https://patchwork.freedesktop.org/series/53311/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5230 -> Patchwork_10979
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53311/revisions/2/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_10979:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_flip@basic-flip-vs-dpms:
    - {fi-icl-u3}:        PASS -> DMESG-WARN

  * {igt@runner@aborted}:
    - {fi-icl-u3}:        NOTRUN -> FAIL

  
Known issues
------------

  Here are the changes found in Patchwork_10979 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-kbl-7560u:       INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-apl-guc:         DMESG-WARN [fdo#108566] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735


Participating hosts (50 -> 44)
------------------------------

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * Linux: CI_DRM_5230 -> Patchwork_10979

  CI_DRM_5230: b0a2de64f8969163f6e01071d5e05748f18a8bab @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10979: 7d5f36355bfbbe1efd2cd901a6dbbbca400da0ad @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

7d5f36355bfb drm/i915/vgpu: Disallow loading on old vGPU hosts

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10979/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/vgpu: Disallow loading on old vGPU hosts (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/vgpu: Disallow loading on old vGPU hosts (rev2)
URL   : https://patchwork.freedesktop.org/series/53311/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5230_full -> Patchwork_10979_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_10979_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10979_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_10979_full:

### IGT changes ###

#### Warnings ####

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-snb:          PASS -> SKIP

  
Known issues
------------

  Here are the changes found in Patchwork_10979_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_atomic_transition@2x-modeset-transitions:
    - shard-hsw:          PASS -> DMESG-FAIL [fdo#102614]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - {shard-iclb}:       NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#103232] +1

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_fbcon_fbt@psr:
    - shard-skl:          NOTRUN -> FAIL [fdo#107882]

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@single-buffer-flip-vs-dpms-off-vs-modeset:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#102614] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-onoff:
    - {shard-iclb}:       PASS -> FAIL [fdo#103167] +1

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - {shard-iclb}:       PASS -> INCOMPLETE [fdo#107713]

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_scaling@pipe-c-scaler-with-rotation:
    - {shard-iclb}:       NOTRUN -> DMESG-WARN [fdo#107724]

  * igt@kms_setmode@basic:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@pm_backlight@fade_with_suspend:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#107847]

  * igt@pm_rpm@debugfs-read:
    - {shard-iclb}:       PASS -> INCOMPLETE [fdo#108840]

  * igt@pm_rpm@modeset-non-lpsp:
    - shard-skl:          SKIP -> INCOMPLETE [fdo#107807]

  
#### Possible fixes ####

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-skl:          TIMEOUT [fdo#108039] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-apl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          FAIL [fdo#100368] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-wc:
    - {shard-iclb}:       DMESG-FAIL [fdo#107724] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - {shard-iclb}:       FAIL [fdo#103167] -> PASS +3

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - {shard-iclb}:       INCOMPLETE [fdo#107713] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - {shard-iclb}:       FAIL [fdo#103166] -> PASS +3

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS +2

  * igt@pm_rpm@legacy-planes-dpms:
    - shard-skl:          INCOMPLETE [fdo#105959] / [fdo#107807] -> PASS

  * igt@pm_rpm@modeset-non-lpsp-stress:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP

  * igt@pm_rpm@system-suspend-devices:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1

  * igt@pm_rpm@system-suspend-execbuf:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107807] -> PASS

  * igt@sw_sync@sync_busy_fork_unixsocket:
    - {shard-iclb}:       INCOMPLETE [fdo#108889] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-glk:          DMESG-WARN [fdo#108784] -> INCOMPLETE [fdo#103359] / [fdo#106886] / [k.org#198133]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105959]: https://bugs.freedesktop.org/show_bug.cgi?id=105959
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107847]: https://bugs.freedesktop.org/show_bug.cgi?id=107847
  [fdo#107882]: https://bugs.freedesktop.org/show_bug.cgi?id=107882
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108889]: https://bugs.freedesktop.org/show_bug.cgi?id=108889
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5230 -> Patchwork_10979

  CI_DRM_5230: b0a2de64f8969163f6e01071d5e05748f18a8bab @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10979: 7d5f36355bfbbe1efd2cd901a6dbbbca400da0ad @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10979/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx