[PATCH 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers

Jackie Li yaodong.li at intel.com
Mon Jan 8 21:24:12 UTC 2018


GuC WOPCM registers are write-once registers. Current driver code
accesses these registers without checking the accessibility to these
registers, this will lead unpredictable driver behaviors if these
registers were touch by other components (such as faulty BIOS code).

This patch moves the GuC WOPCM register updating operations into
intel_guc_wopcm.c and adds checks before and after the write to GuC
WOPCM registers to make sure the driver is in a known state before
and after writing to these write-once registers.

Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
Signed-off-by: Jackie Li <yaodong.li at intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h       |   2 +-
 drivers/gpu/drm/i915/intel_guc_reg.h   |   3 +
 drivers/gpu/drm/i915/intel_guc_wopcm.c | 121 +++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_guc_wopcm.h |  14 +++-
 drivers/gpu/drm/i915/intel_uc.c        |   5 +-
 5 files changed, 120 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index ea35911..7ed0c17 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -118,7 +118,7 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
 {
 	u32 offset = i915_ggtt_offset(vma);
 
-	GEM_BUG_ON(!guc->wopcm.valid);
+	GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID));
 	GEM_BUG_ON(offset < guc->wopcm.top);
 	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
 
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index 1f52fb8..250f194 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -66,6 +66,7 @@
 #define   UOS_MOVE			  (1<<4)
 #define   START_DMA			  (1<<0)
 #define DMA_GUC_WOPCM_OFFSET		_MMIO(0xc340)
+#define DMA_GUC_WOPCM_OFFSET_MASK	(0xffffc000)
 #define   HUC_LOADING_AGENT_VCR		  (0<<1)
 #define   HUC_LOADING_AGENT_GUC		  (1<<1)
 #define GUC_MAX_IDLE_COUNT		_MMIO(0xC3E4)
@@ -75,6 +76,8 @@
 
 /* Defines WOPCM space available to GuC firmware */
 #define GUC_WOPCM_SIZE			_MMIO(0xc050)
+#define GUC_WOPCM_REG_LOCKED		(1<<0)
+#define GUC_WOPCM_SIZE_MASK		(0xfffff000)
 
 /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
 #define GUC_GGTT_TOP			0xFEE00000
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
index 2523fef..488fc24 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
@@ -39,6 +39,21 @@ static inline u32 guc_reserved_wopcm_size(struct intel_guc *guc)
 	return 0;
 }
 
+/*
+ * On Gen9/CNL A0, Hardware requires guc size to be larger than or equal to
+ * HuC kernel size.
+ */
+static inline int cnl_a0_wopcm_size_check(struct drm_i915_private *i915)
+{
+	struct intel_guc_wopcm *wopcm = &i915->guc.wopcm;
+	u32 huc_size = intel_uc_fw_get_size(&i915->huc.fw);
+
+	if (unlikely(wopcm->size - GEN10_GUC_WOPCM_OFFSET < huc_size))
+		return -E2BIG;
+
+	return 0;
+}
+
 static inline int gen9_wocpm_size_check(struct drm_i915_private *i915)
 {
 	struct intel_guc_wopcm *wopcm = &i915->guc.wopcm;
@@ -58,22 +73,7 @@ static inline int gen9_wocpm_size_check(struct drm_i915_private *i915)
 	if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
 		return -E2BIG;
 
-	return 0;
-}
-
-static inline int cnl_a0_wopcm_size_check(struct drm_i915_private *i915)
-{
-	struct intel_guc_wopcm *wopcm = &i915->guc.wopcm;
-	u32 huc_size = intel_uc_fw_get_size(&i915->huc.fw);
-
-	/*
-	 * On CNL A0, hardware requires guc size to be larger than or equal to
-	 * HuC kernel size.
-	 */
-	if (unlikely(wopcm->size - GEN10_GUC_WOPCM_OFFSET < huc_size))
-		return -E2BIG;
-
-	return 0;
+	return cnl_a0_wopcm_size_check(i915);
 }
 
 static inline int guc_wopcm_size_check(struct intel_guc *guc)
@@ -89,6 +89,53 @@ static inline int guc_wopcm_size_check(struct intel_guc *guc)
 	return 0;
 }
 
+static inline bool __reg_locked(struct drm_i915_private *dev_priv,
+				 i915_reg_t reg)
+{
+	return !!(I915_READ(reg) & GUC_WOPCM_REG_LOCKED);
+}
+
+static inline bool guc_wopcm_locked(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+	bool size_reg_locked = __reg_locked(i915, GUC_WOPCM_SIZE);
+	bool offset_reg_locked = __reg_locked(i915, DMA_GUC_WOPCM_OFFSET);
+
+	return size_reg_locked && offset_reg_locked;
+}
+
+static inline void guc_wopcm_hw_update(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	/* GuC WOPCM registers should be unlocked at this point. */
+	GEM_BUG_ON(__reg_locked(dev_priv, GUC_WOPCM_SIZE));
+	GEM_BUG_ON(__reg_locked(dev_priv, DMA_GUC_WOPCM_OFFSET));
+
+	I915_WRITE(GUC_WOPCM_SIZE, guc->wopcm.size);
+	I915_WRITE(DMA_GUC_WOPCM_OFFSET,
+		   guc->wopcm.offset | HUC_LOADING_AGENT_GUC);
+
+	GEM_BUG_ON(!__reg_locked(dev_priv, GUC_WOPCM_SIZE));
+	GEM_BUG_ON(!__reg_locked(dev_priv, DMA_GUC_WOPCM_OFFSET));
+}
+
+static inline bool guc_wopcm_regs_valid(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 size, offset;
+	bool huc_loaded_by_guc;
+
+	size = I915_READ(GUC_WOPCM_SIZE) & GUC_WOPCM_SIZE_MASK;
+	offset = I915_READ(DMA_GUC_WOPCM_OFFSET);
+
+	huc_loaded_by_guc = !!(offset & HUC_LOADING_AGENT_GUC);
+	offset &= DMA_GUC_WOPCM_OFFSET_MASK;
+
+	return huc_loaded_by_guc && (size == guc->wopcm.size) &&
+		(offset == guc->wopcm.offset);
+}
+
 /*
  * intel_guc_wopcm_init() - Initialize the GuC WOPCM partition.
  * @guc: intel guc.
@@ -107,8 +154,7 @@ int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
 	u32 offset, size, top;
 	int err;
 
-	if (guc->wopcm.valid)
-		return 0;
+	GEM_BUG_ON(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID);
 
 	if (!guc_fw_size)
 		return -EINVAL;
@@ -128,6 +174,9 @@ int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
 	top = WOPCM_DEFAULT_SIZE - offset;
 	size = top - reserved;
 
+	/*size must be 4K aligned*/
+	size &= GUC_WOPCM_SIZE_MASK;
+
 	/*
 	 * GuC size needs to be less than or equal to GuC WOPCM size.
 	 * Need extra 8K stack for GuC.
@@ -144,10 +193,44 @@ int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
 	if (err)
 		return err;
 
-	guc->wopcm.valid = true;
+	guc->wopcm.flags |= INTEL_GUC_WOPCM_VALID;
 
 	DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n",
 			 offset >> 10, size >> 10, top >> 10);
 
 	return 0;
 }
+
+/*
+ * intel_guc_wopcm_init_hw() - Setup GuC WOPCM registers.
+ * @guc: intel guc.
+ *
+ * Setup the GuC WOPCM size and offset registers with the stored values. It will
+ * also check the registers locking status to determine whether these registers
+ * are unlocked and can be updated.
+ */
+void intel_guc_wopcm_init_hw(struct intel_guc *guc)
+{
+	u32 locked = guc_wopcm_locked(guc);
+
+	GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID));
+
+	/*
+	 * Bug if driver hasn't updated the HW Registers and GuC WOPCM has been
+	 * locked. Return directly if WOPCM was locked and we have updated
+	 * the registers.
+	 */
+	if (locked) {
+		/* Mark as updated if registers contained correct values. */
+		if (guc_wopcm_regs_valid(guc))
+			goto out;
+
+		GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_HW_UPDATED));
+		return;
+	}
+
+	/* Always update registers when GuC WOPCM is not locked. */
+	guc_wopcm_hw_update(guc);
+out:
+	guc->wopcm.flags |= INTEL_GUC_WOPCM_HW_UPDATED;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
index 0ce31ee..dd62b6b 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
@@ -46,11 +46,22 @@ struct intel_guc;
 #define GEN9_GUC_WOPCM_OFFSET		(0x24000)
 #define GEN10_GUC_WOPCM_OFFSET		(0x4000)
 
+/* GuC WOPCM flags*/
+#define INTEL_GUC_WOPCM_VALID		BIT(0)
+#define INTEL_GUC_WOPCM_HW_UPDATED	BIT(1)
+
+/*
+ * intel_guc_wopcm - GuC WOPCM related settings.
+ * @offset: GuC WOPCM offset.
+ * @size: size of GuC WOPCM for GuC firmware.
+ * @top: start of Non GuC WOPCM memory.
+ * @flags: bitmap of INTEL_GUC_WOPCM_<foo>.
+ */
 struct intel_guc_wopcm {
 	u32 offset;
 	u32 size;
 	u32 top;
-	bool valid;
+	u32 flags;
 };
 
 /*
@@ -68,5 +79,6 @@ static inline void intel_guc_wopcm_init_early(struct intel_guc_wopcm *wopcm)
 }
 
 int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_size, u32 huc_size);
+void intel_guc_wopcm_init_hw(struct intel_guc *guc);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 70ed297..346ba03 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -280,10 +280,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	guc_disable_communication(guc);
 	gen9_reset_guc_interrupts(dev_priv);
 
-	/* init WOPCM */
-	I915_WRITE(GUC_WOPCM_SIZE, guc->wopcm.size);
-	I915_WRITE(DMA_GUC_WOPCM_OFFSET,
-		   guc->wopcm.offset | HUC_LOADING_AGENT_GUC);
+	intel_guc_wopcm_init_hw(guc);
 
 	/* WaEnableuKernelHeaderValidFix:skl */
 	/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
-- 
2.7.4



More information about the Intel-gfx-trybot mailing list