Message ID | 1520987574-19351-5-git-send-email-yaodong.li@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Quoting Jackie Li (2018-03-14 02:32:53) > 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) <SNIP> > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -5147,6 +5147,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); This (or the other instance inside) can still be removed, it's a duplicate message. Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b58402b..e19aa8b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5147,6 +5147,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 01963d0..d860847 100644 --- 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 ed5a6fc..6316548 100644 --- 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 1fd1125..4117886 100644 --- 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 93c402c..6298910 100644 --- 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