Re: [PATCH] drm/i915/guc: Assert that all GGTT offsets used by the GuC are mappable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/24/2016 1:22 PM, Chris Wilson wrote:
On Sat, Dec 24, 2016 at 12:52:37PM -0800, Ceraolo Spurio, Daniele wrote:

On 12/24/2016 11:31 AM, Chris Wilson wrote:
Add an assertion to the plain i915_ggtt_offset() to double check that
any offset we hand to the GuC is outside of its unmappable ranges.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_guc_submission.c | 14 +++++++-------
  drivers/gpu/drm/i915/intel_guc_loader.c    |  6 +++---
  drivers/gpu/drm/i915/intel_uc.h            |  9 +++++++++
  3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 3e20fe2be811..30e012b9e93c 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -270,11 +270,11 @@ static void guc_ctx_desc_init(struct intel_guc *guc,
  		/* The state page is after PPHWSP */
  		lrc->ring_lcra =
-			i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
+			guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
  		lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
  				(guc_engine_id << GUC_ELC_ENGINE_OFFSET);
-		lrc->ring_begin = i915_ggtt_offset(ce->ring->vma);
+		lrc->ring_begin = guc_ggtt_offset(ce->ring->vma);
  		lrc->ring_end = lrc->ring_begin + ce->ring->size - 1;
  		lrc->ring_next_free_location = lrc->ring_begin;
  		lrc->ring_current_tail_pointer_value = 0;
@@ -290,7 +290,7 @@ static void guc_ctx_desc_init(struct intel_guc *guc,
  	 * The doorbell, process descriptor, and workqueue are all parts
  	 * of the client object, which the GuC will reference via the GGTT
  	 */
-	gfx_addr = i915_ggtt_offset(client->vma);
+	gfx_addr = guc_ggtt_offset(client->vma);
  	desc.db_trigger_phy = sg_dma_address(client->vma->pages->sgl) +
  				client->doorbell_offset;
  	desc.db_trigger_cpu =
@@ -1226,7 +1226,7 @@ static void guc_log_create(struct intel_guc *guc)
  		(GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) |
  		(GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT);
-	offset = i915_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */
+	offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */
  	guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
  }
@@ -1329,7 +1329,7 @@ static void guc_addon_create(struct intel_guc *guc)
  	guc_policies_init(policies);
  	ads->scheduler_policies =
-		i915_ggtt_offset(vma) + sizeof(struct guc_ads);
+		guc_ggtt_offset(vma) + sizeof(struct guc_ads);
  	/* MMIO reg state */
  	reg_state = (void *)policies + sizeof(struct guc_policies);
@@ -1495,7 +1495,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
  	/* any value greater than GUC_POWER_D0 */
  	data[1] = GUC_POWER_D1;
  	/* first page is shared data with GuC */
-	data[2] = i915_ggtt_offset(ctx->engine[RCS].state);
+	data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
  	return intel_guc_send(guc, data, ARRAY_SIZE(data));
  }
@@ -1522,7 +1522,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
  	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
  	data[1] = GUC_POWER_D0;
  	/* first page is shared data with GuC */
-	data[2] = i915_ggtt_offset(ctx->engine[RCS].state);
+	data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
  	return intel_guc_send(guc, data, ARRAY_SIZE(data));
  }
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 21db69705458..35d5690f47a2 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -220,14 +220,14 @@ static void guc_params_init(struct drm_i915_private *dev_priv)
  		params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
  	if (guc->ads_vma) {
-		u32 ads = i915_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
+		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
  		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
  		params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
  	}
  	/* If GuC submission is enabled, set up additional parameters here */
  	if (i915.enable_guc_submission) {
-		u32 pgs = i915_ggtt_offset(dev_priv->guc.ctx_pool_vma);
+		u32 pgs = guc_ggtt_offset(dev_priv->guc.ctx_pool_vma);
  		u32 ctx_in_16 = GUC_MAX_GPU_CONTEXTS / 16;
  		pgs >>= PAGE_SHIFT;
@@ -297,7 +297,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
  	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
  	/* Set the source address for the new blob */
-	offset = i915_ggtt_offset(vma) + guc_fw->header_offset;
+	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
  	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
  	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 11f56082b363..d556215e691f 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -28,6 +28,8 @@
  #include "i915_guc_reg.h"
  #include "intel_ringbuffer.h"
+#include "i915_vma.h"
+
  struct drm_i915_gem_request;
  /*
@@ -198,4 +200,11 @@ void i915_guc_register(struct drm_i915_private *dev_priv);
  void i915_guc_unregister(struct drm_i915_private *dev_priv);
  int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
+static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+	u32 offset = i915_ggtt_offset(vma);
+	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
GEM_BUG_ON(offset < GUC_WOPCM_TOP || offset > 0xFEE00000);

Maybe we could add a define for 0xFEE00000 as we might have to
re-use it elsewhere.

With the change for the GEM_BUG_ON:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
I wasn't going to do the upper check until we have the fix in place
(i.e. that patch to apply the limit would add the assertion as well). No
point in intentionally panicking the kernel, just hang the hw instead!
;)
-Chris


Oh well, in that case I'll trust you to update the check after the fix is merged ;-)
You can apply my r-b to this version.

Thanks,
Daniele

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux