drm/i915/guc: Check the locking status of GuC WOPCM registers

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

This patch moves the GuC WOPCM registers updating code into intel_wopcm.c
and adds check before and after the update to GuC WOPCM registers so that
we can make sure the driver is in a known state after writing to these
write-once registers.

v6:
 - Made sure module reloading won't bug the kernel while doing
   locking status checking

v7:
 - Fixed patch format issues

v8:
 - Fixed coding style issue on register lock bit macro definition (Sagar)

v9:
 - Avoided to use redundant !! to cast uint to bool (Chris)
 - Return error code instead of GEM_BUG_ON for locked with invalid register
   values case (Sagar)
 - Updated guc_wopcm_hw_init to use guc_wopcm as first parameter (Michal)
 - Added code to set and validate the HuC_LOADING_AGENT_GUC bit in GuC
   WOPCM offset register based on the presence of HuC firmware (Michal)
 - Use bit fields instead of macros for GuC WOPCM flags (Michal)

v10:
 - Refined variable names, removed redundant comments (Joonas)
 - Introduced lockable_reg to handle the write once register write and
   propagate the write error to caller (Joonas)
 - Used lockable_reg abstraction to avoid locking bit check on generic
   i915_reg_t (Michal)
 - Added log message for error paths (Michal)
 - Removed hw_updated flag and only relies on real hardware status

v11:
 - Replaced lockable_reg with simplified function (Michal)
 - Used new macros for locking bits of WOPCM size/offset registers instead
   of using BIT(0) directly (Michal)
 - use intel_wopcm_init_hw() called from intel_gem_init_hw() to do GuC
   WOPCM register setup instead of calling from intel_uc_init_hw() (Michal)

v12:
 - Updated function kernel-doc to align with code changes (Michal)
 - Updated code to use wopcm pointer directly (Michal)

v13:
 - Updated the ordering of s-o-b/cc/r-b tags (Sagar)

BSpec: 10875, 10833

Signed-off-by: Jackie Li <yaodong.li@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> (v11)
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v12)
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1520987574-19351-5-git-send-email-yaodong.li@intel.com

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 51faa65067396c7057ef1f7fbc61945213797fc3..13d4b0e746414428dc8ad2fc2fc4e7f90965bb4c 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5137,6 +5137,12 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
                goto out;
        }
 
+       ret = intel_wopcm_init_hw(&dev_priv->wopcm);
+       if (ret) {
+               DRM_ERROR("Enabling WOPCM failed (%d)\n", ret);
+               goto out;
+       }
+
        /* We can't enable contexts until all firmware is loaded */
        ret = intel_uc_init_hw(dev_priv);
        if (ret) {

diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index 01963d085ed6a95f673f6e4a0ab34a3b2407edfa..d86084742a4a0e32e4ce982c954bc440cb26059e 100644 (file)
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -66,15 +66,18 @@
 #define   UOS_MOVE                       (1<<4)
 #define   START_DMA                      (1<<0)
 #define DMA_GUC_WOPCM_OFFSET           _MMIO(0xc340)
+#define   GUC_WOPCM_OFFSET_VALID         (1<<0)
 #define   HUC_LOADING_AGENT_VCR                  (0<<1)
 #define   HUC_LOADING_AGENT_GUC                  (1<<1)
 #define   GUC_WOPCM_OFFSET_SHIFT       14
+#define   GUC_WOPCM_OFFSET_MASK                  (0x3ffff << GUC_WOPCM_OFFSET_SHIFT)
 #define GUC_MAX_IDLE_COUNT             _MMIO(0xC3E4)
 
 #define HUC_STATUS2             _MMIO(0xD3B0)
 #define   HUC_FW_VERIFIED       (1<<7)
 
 #define GUC_WOPCM_SIZE                 _MMIO(0xc050)
+#define   GUC_WOPCM_SIZE_LOCKED                  (1<<0)
 #define   GUC_WOPCM_SIZE_SHIFT         12
 #define   GUC_WOPCM_SIZE_MASK            (0xfffff << GUC_WOPCM_SIZE_SHIFT)
 

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ed5a6fcc8557e5d5d77e56bf03032f983301506d..6316548a1c78b87a2978c234b2c060328343f257 100644 (file)
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -367,11 +367,6 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
        gen9_reset_guc_interrupts(dev_priv);
 
-       /* init WOPCM */
-       I915_WRITE(GUC_WOPCM_SIZE, dev_priv->wopcm.guc.size);
-       I915_WRITE(DMA_GUC_WOPCM_OFFSET,
-                  dev_priv->wopcm.guc.base | HUC_LOADING_AGENT_GUC);
-
        /* WaEnableuKernelHeaderValidFix:skl */
        /* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
        if (IS_GEN9(dev_priv))

diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 1fd1125f464b6b31c59a0a06eabe1576f951397e..4117886bfb053556a3aa6061854dec1d812ee27a 100644 (file)
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -207,3 +207,67 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
 
        return 0;
 }
+
+static inline int write_and_verify(struct drm_i915_private *dev_priv,
+                                  i915_reg_t reg, u32 val, u32 mask,
+                                  u32 locked_bit)
+{
+       u32 reg_val;
+
+       GEM_BUG_ON(val & ~mask);
+
+       I915_WRITE(reg, val);
+
+       reg_val = I915_READ(reg);
+
+       return (reg_val & mask) != (val | locked_bit) ? -EIO : 0;
+}
+
+/**
+ * intel_wopcm_init_hw() - Setup GuC WOPCM registers.
+ * @wopcm: pointer to intel_wopcm.
+ *
+ * Setup the GuC WOPCM size and offset registers with the calculated values. It
+ * will verify the register values to make sure the registers are locked with
+ * correct values.
+ *
+ * Return: 0 on success. -EIO if registers were locked with incorrect values.
+ */
+int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
+{
+       struct drm_i915_private *dev_priv = wopcm_to_i915(wopcm);
+       u32 huc_agent;
+       u32 mask;
+       int err;
+
+       if (!USES_GUC(dev_priv))
+               return 0;
+
+       GEM_BUG_ON(!HAS_GUC(dev_priv));
+       GEM_BUG_ON(!wopcm->guc.size);
+       GEM_BUG_ON(!wopcm->guc.base);
+
+       err = write_and_verify(dev_priv, GUC_WOPCM_SIZE, wopcm->guc.size,
+                              GUC_WOPCM_SIZE_MASK | GUC_WOPCM_SIZE_LOCKED,
+                              GUC_WOPCM_SIZE_LOCKED);
+       if (err)
+               goto err_out;
+
+       huc_agent = USES_HUC(dev_priv) ? HUC_LOADING_AGENT_GUC : 0;
+       mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
+       err = write_and_verify(dev_priv, DMA_GUC_WOPCM_OFFSET,
+                              wopcm->guc.base | huc_agent, mask,
+                              GUC_WOPCM_OFFSET_VALID);
+       if (err)
+               goto err_out;
+
+       return 0;
+
+err_out:
+       DRM_ERROR("Failed to init WOPCM registers:\n");
+       DRM_ERROR("DMA_GUC_WOPCM_OFFSET=%#x\n",
+                 I915_READ(DMA_GUC_WOPCM_OFFSET));
+       DRM_ERROR("GUC_WOPCM_SIZE=%#x\n", I915_READ(GUC_WOPCM_SIZE));
+
+       return err;
+}

diff --git a/drivers/gpu/drm/i915/intel_wopcm.h b/drivers/gpu/drm/i915/intel_wopcm.h
index 93c402ca7489ab7ae6a0e6944c78cdffedd0d8aa..6298910a384c4f74bfa88f512bcc182ba7cd061c 100644 (file)
--- a/drivers/gpu/drm/i915/intel_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_wopcm.h
@@ -26,5 +26,6 @@ struct intel_wopcm {
 
 void intel_wopcm_init_early(struct intel_wopcm *wopcm);
 int intel_wopcm_init(struct intel_wopcm *wopcm);
+int intel_wopcm_init_hw(struct intel_wopcm *wopcm);
 
 #endif