On Mon, 2018-04-09 at 17:42 -0700, Jackie Li wrote:
> In current code, we only compare the locked WOPCM register values
> with the
> calculated values. However, we can continue loading GuC/HuC firmware
> if the
> locked (or partially locked) values were valid for current GuC/HuC
> firmware
> sizes.
>
> This patch added a new code path to verify whether the locked
> register
> values can be used for GuC/HuC firmware loading, it will recalculate
> the
> verify the new values if these registers were partially locked, so
> that we
> won't fail the GuC/HuC firmware loading even if the locked register
> values
> are different from the calculated ones.
>
> v2:
> - Update WOPCM register only if it's not locked
>
> Signed-off-by: Jackie Li <yaodong.li@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
> Cc: John Spotswood <john.a.spotswood@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Reviewed-by: John Spotswood <john.a.spotswood@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_wopcm.c | 217
> +++++++++++++++++++++++++++++++------
> 1 file changed, 185 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c
> b/drivers/gpu/drm/i915/intel_wopcm.c
> index b1c08ca..fa8d2be 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -140,6 +140,53 @@ static inline int check_hw_restriction(struct
> drm_i915_private *i915,
> return err;
> }
>
> +static inline u32 calculate_min_guc_wopcm_base(u32 huc_fw_size)
> +{
> + return ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
> + GUC_WOPCM_OFFSET_ALIGNMENT);
> +}
> +
> +static inline u32 calculate_min_guc_wopcm_size(u32 guc_fw_size)
> +{
> + return guc_fw_size + GUC_WOPCM_RESERVED +
> GUC_WOPCM_STACK_RESERVED;
> +}
> +
> +static inline int calculate_max_guc_wopcm_size(struct intel_wopcm
> *wopcm,
> + u32 guc_wopcm_base,
> + u32 *guc_wopcm_size)
> +{
> + struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> + u32 ctx_rsvd = context_reserved_size(i915);
> +
> + if (guc_wopcm_base >= wopcm->size ||
> + (guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
> + DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
> + guc_wopcm_base / 1024);
> + return -E2BIG;
> + }
> +
> + *guc_wopcm_size =
> + (wopcm->size - guc_wopcm_base - ctx_rsvd) &
> GUC_WOPCM_SIZE_MASK;
> +
> + return 0;
> +}
> +
> +static inline int verify_calculated_values(struct drm_i915_private
> *i915,
> + u32 guc_fw_size, u32
> huc_fw_size,
> + u32 guc_wopcm_base,
> + u32 guc_wopcm_size)
> +{
> + if (guc_wopcm_size <
> calculate_min_guc_wopcm_size(guc_fw_size)) {
> + DRM_ERROR("Need %uKiB WOPCM for GuC FW, %uKiB
> available.\n",
> + calculate_min_guc_wopcm_size(guc_fw_size),
> + guc_wopcm_size / 1024);
> + return -E2BIG;
> + }
> +
> + return check_hw_restriction(i915, guc_wopcm_base,
> guc_wopcm_size,
> + huc_fw_size);
> +}
> +
> /**
> * intel_wopcm_init() - Initialize the WOPCM structure.
> * @wopcm: pointer to intel_wopcm.
> @@ -157,10 +204,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
> struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> u32 guc_fw_size = intel_uc_fw_get_upload_size(&i915-
> >guc.fw);
> u32 huc_fw_size = intel_uc_fw_get_upload_size(&i915-
> >huc.fw);
> - u32 ctx_rsvd = context_reserved_size(i915);
> u32 guc_wopcm_base;
> u32 guc_wopcm_size;
> - u32 guc_wopcm_rsvd;
> int err;
>
> GEM_BUG_ON(!wopcm->size);
> @@ -177,35 +222,121 @@ int intel_wopcm_init(struct intel_wopcm
> *wopcm)
> return -E2BIG;
> }
>
> - guc_wopcm_base = ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
> - GUC_WOPCM_OFFSET_ALIGNMENT);
> - if ((guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
> - DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
> - guc_wopcm_base / 1024);
> + guc_wopcm_base = calculate_min_guc_wopcm_base(huc_fw_size);
> + err = calculate_max_guc_wopcm_size(wopcm, guc_wopcm_base,
> + &guc_wopcm_size);
> + if (err)
> + return err;
> +
> + DRM_DEBUG_DRIVER("Calculated GuC WOPCM Region: [%uKiB,
> %uKiB)\n",
> + guc_wopcm_base / 1024,
> + (guc_wopcm_base + guc_wopcm_size) / 1024);
> +
> + err = verify_calculated_values(i915, guc_fw_size,
> huc_fw_size,
> + guc_wopcm_base,
> guc_wopcm_size);
> + if (err)
> + return err;
> +
> + wopcm->guc.base = guc_wopcm_base;
> + wopcm->guc.size = guc_wopcm_size;
> +
> + return 0;
> +}
> +
> +static inline int verify_locked_values(struct intel_wopcm *wopcm,
> + u32 guc_wopcm_base, u32
> guc_wopcm_size)
> +{
> + struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> + u32 guc_fw_size = intel_uc_fw_get_upload_size(&i915-
> >guc.fw);
> + u32 huc_fw_size = intel_uc_fw_get_upload_size(&i915-
> >huc.fw);
> + u32 ctx_rsvd = context_reserved_size(i915);
> +
> + /*
> + * Locked GuC WOPCM base and size could be any values which
> are large
> + * enough to lead to a wrap around after performing add
> operations.
> + */
> + if (guc_wopcm_base >= wopcm->size) {
> + DRM_ERROR("Locked base value %uKiB >= WOPCM size
> %uKiB.\n",
> + guc_wopcm_base / 1024,
> + wopcm->size / 1024);
> return -E2BIG;
> }
>
> - guc_wopcm_size = wopcm->size - guc_wopcm_base - ctx_rsvd;
> - guc_wopcm_size &= GUC_WOPCM_SIZE_MASK;
> + if (guc_wopcm_size >= wopcm->size) {
> + DRM_ERROR("Locked size value %uKiB >= WOPCM size
> %uKiB.\n",
> + guc_wopcm_base / 1024,
> + wopcm->size / 1024);
> + return -E2BIG;
> + }
>
> - DRM_DEBUG_DRIVER("Calculated GuC WOPCM Region: [%uKiB,
> %uKiB)\n",
> - guc_wopcm_base / 1024, guc_wopcm_size /
> 1024);
> + if (guc_wopcm_base + guc_wopcm_size + ctx_rsvd > wopcm-
> >size) {
> + DRM_ERROR("Need %uKiB WOPCM in total, %uKiB
> available.\n",
> + (guc_wopcm_base + guc_wopcm_size +
> ctx_rsvd) / 1024,
> + (wopcm->size) / 1024);
> + return -E2BIG;
> + }
>
> - guc_wopcm_rsvd = GUC_WOPCM_RESERVED +
> GUC_WOPCM_STACK_RESERVED;
> - if ((guc_fw_size + guc_wopcm_rsvd) > guc_wopcm_size) {
> - DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB
> available.\n",
> - (guc_fw_size + guc_wopcm_rsvd) / 1024,
> - guc_wopcm_size / 1024);
> + if (guc_wopcm_base <
> calculate_min_guc_wopcm_base(huc_fw_size)) {
> + DRM_ERROR("Need %uKiB WOPCM for HuC FW, %uKiB
> available.\n",
> + calculate_min_guc_wopcm_base(huc_fw_size)
> / 1024,
> + guc_wopcm_base / 1024);
> return -E2BIG;
> }
>
> - err = check_hw_restriction(i915, guc_wopcm_base,
> guc_wopcm_size,
> - huc_fw_size);
> + return verify_calculated_values(i915, guc_fw_size,
> huc_fw_size,
> + guc_wopcm_base,
> guc_wopcm_size);
> +}
> +
> +static inline int verify_locked_values_and_update(struct intel_wopcm
> *wopcm,
> + bool
> *update_size_reg,
> + bool
> *update_offset_reg)
> +{
> + struct drm_i915_private *dev_priv = wopcm_to_i915(wopcm);
> + u32 huc_fw_size = intel_uc_fw_get_upload_size(&dev_priv-
> >huc.fw);
> + u32 size_val = I915_READ(GUC_WOPCM_SIZE);
> + u32 offset_val = I915_READ(DMA_GUC_WOPCM_OFFSET);
> + bool offset_reg_locked = offset_val &
> GUC_WOPCM_OFFSET_VALID;
> + bool size_reg_locked = size_val & GUC_WOPCM_SIZE_LOCKED;
> + u32 guc_wopcm_base = offset_val & GUC_WOPCM_OFFSET_MASK;
> + u32 guc_wopcm_size = size_val & GUC_WOPCM_SIZE_MASK;
> + int err;
> +
> + *update_size_reg = !size_reg_locked;
> + *update_offset_reg = !offset_reg_locked;
> +
> + if (!offset_reg_locked && !size_reg_locked)
> + return 0;
> +
> + if (guc_wopcm_base == wopcm->guc.base &&
> + guc_wopcm_size == wopcm->guc.size)
> + return 0;
> +
> + if (USES_HUC(dev_priv) && offset_reg_locked &&
> + !(offset_val & HUC_LOADING_AGENT_GUC)) {
> + DRM_ERROR("HUC_LOADING_AGENT_GUC isn't locked for
> USES_HUC.\n");
> + return -EIO;
> + }
> +
> + if (!offset_reg_locked)
> + guc_wopcm_base =
> calculate_min_guc_wopcm_base(huc_fw_size);
> +
> + if (!size_reg_locked) {
> + err = calculate_max_guc_wopcm_size(wopcm,
> guc_wopcm_base,
> + &guc_wopcm_size);
> + if (err)
> + return err;
> + }
> +
> + DRM_DEBUG_DRIVER("Recalculated GuC WOPCM Region: [%uKiB,
> %uKiB)\n",
> + guc_wopcm_base / 1024,
> + (guc_wopcm_base + guc_wopcm_size) / 1024);
> +
> + err = verify_locked_values(wopcm, guc_wopcm_base,
> guc_wopcm_size);
> if (err)
> return err;
>
> - wopcm->guc.base = guc_wopcm_base;
> wopcm->guc.size = guc_wopcm_size;
> + wopcm->guc.base = guc_wopcm_base;
>
> return 0;
> }
> @@ -233,11 +364,14 @@ static inline int write_and_verify(struct
> drm_i915_private *dev_priv,
> * 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.
> + * Return: 0 on success. Minus error code if registered were locked
> with
> + * incorrect values.-EIO if registers failed to lock with correct
> values.
> */
> int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
> {
> struct drm_i915_private *dev_priv = wopcm_to_i915(wopcm);
> + bool update_size_reg;
> + bool update_offset_reg;
> int err;
>
> if (!USES_GUC(dev_priv))
> @@ -247,27 +381,46 @@ int intel_wopcm_init_hw(struct intel_wopcm
> *wopcm)
> 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)
> + err = verify_locked_values_and_update(wopcm,
> &update_size_reg,
> + &update_offset_reg);
> + if (err) {
> + DRM_ERROR("WOPCM registers are locked with invalid
> values.\n");
> goto err_out;
> + }
>
> - err = write_and_verify(dev_priv, DMA_GUC_WOPCM_OFFSET,
> - wopcm->guc.base |
> HUC_LOADING_AGENT_GUC,
> - GUC_WOPCM_OFFSET_MASK |
> HUC_LOADING_AGENT_GUC |
> - GUC_WOPCM_OFFSET_VALID,
> - GUC_WOPCM_OFFSET_VALID);
> - if (err)
> - goto err_out;
> + if (update_size_reg) {
> + 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) {
> + DRM_ERROR("Failed to GuC WOPCM size with
> %#x.\n",
> + wopcm->guc.size);
> + goto err_out;
> + }
> + }
>
> + if (update_offset_reg) {
> + err = write_and_verify(dev_priv,
> DMA_GUC_WOPCM_OFFSET,
> + wopcm->guc.base |
> HUC_LOADING_AGENT_GUC,
> + GUC_WOPCM_OFFSET_MASK |
> + HUC_LOADING_AGENT_GUC |
> + GUC_WOPCM_OFFSET_VALID,
> + GUC_WOPCM_OFFSET_VALID);
> + if (err) {
> + DRM_ERROR("Failed to GuC WOPCM offset with
> %#x.\n",
> + wopcm->guc.base);
> + 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));
> + DRM_ERROR("A system reboot is required.\n");
>
> return err;
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
