diff mbox series

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

Message ID 20190814113821.28700-2-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/wopcm: Try to use already locked WOPCM layout | expand

Commit Message

Michal Wajdeczko Aug. 14, 2019, 11:38 a.m. UTC
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(-)

Comments

Daniele Ceraolo Spurio Aug. 14, 2019, 8:06 p.m. UTC | #1
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) {
>
diff mbox series

Patch

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;
+	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) {