Re: [PATCH 2/2] drm/i915/uc: Move FW size sanity check back to fetch

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 8/14/19 4:38 AM, Michal Wajdeczko wrote:
> From: Michał Winiarski <michal.winiarski@intel.com>
> 
> While we need to know WOPCM size to do this sanity check, it has more to
> do with FW than with WOPCM. Let's move the check to fetch phase, it's
> not like WOPCM is going to grow in the meantime.
> 
> v2: rebased
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jackie Li <yaodong.li@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 11 +++++++++++
>   drivers/gpu/drm/i915/intel_wopcm.c       | 14 ++------------
>   2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index d056e1f4bd6d..98cb0eff143f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -265,6 +265,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
>   	size_t size;
>   	int err;
>   
> +	GEM_BUG_ON(!i915->wopcm.size);
>   	GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
>   
>   	err = i915_inject_load_error(i915, -ENXIO);
> @@ -324,6 +325,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
>   		goto fail;
>   	}
>   
> +	/* Sanity check whether this fw is not larger than whole WOPCM memory */
> +	size = sizeof(struct uc_css_header) + uc_fw->ucode_size;

We could add a __intel_uc_fw_get_upload_size() that skips the 
is_available() check and use that here. With our without this:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> +	if (unlikely(size >= i915->wopcm.size)) {
> +		dev_warn(dev, "%s firmware %s: invalid size: %zu > %zu\n",
> +			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> +			 size, (size_t)i915->wopcm.size);
> +		err = -E2BIG;
> +		goto fail;
> +	}
> +
>   	/* Get version numbers from the CSS header */
>   	switch (uc_fw->type) {
>   	case INTEL_UC_FW_TYPE_GUC:
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> index e5bc7b8a433e..295978356eef 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -197,6 +197,8 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>   	GEM_BUG_ON(!wopcm->size);
>   	GEM_BUG_ON(wopcm->guc.base);
>   	GEM_BUG_ON(wopcm->guc.size);
> +	GEM_BUG_ON(guc_fw_size >= wopcm->size);
> +	GEM_BUG_ON(huc_fw_size >= wopcm->size);
>   
>   	if (i915_inject_probe_failure(i915))
>   		return;
> @@ -209,18 +211,6 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>   		goto check;
>   	}
>   
> -	if (guc_fw_size >= wopcm->size) {
> -		DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
> -			  guc_fw_size / 1024);
> -		return;
> -	}
> -
> -	if (huc_fw_size >= wopcm->size) {
> -		DRM_ERROR("HuC FW (%uKiB) is too big to fit in WOPCM.",
> -			  huc_fw_size / 1024);
> -		return;
> -	}
> -
>   	guc_wopcm_base = ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
>   			       GUC_WOPCM_OFFSET_ALIGNMENT);
>   	if ((guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx