Re: [Intel-gfx] [2/4] drm/i915/huc: Add fetch support for gsc-loaded HuC binary

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

minor nit: not sure if its worth mentioning in commit msg that "loaded_via_gsc"
is zero-init-ed as fw_def macros we havent added fw_defs for gsc-loaded-huc-bins
which explains why "loaded_via_gsc" not getting set anywhere in this series.

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>


On Tue, 2022-04-26 at 17:26 -0700, Daniele Ceraolo Spurio wrote:
> On newer platforms (starting DG2 G10 B-step and G11 A-step), ownership of
> HuC loading has been moved from the GuC to the GSC. As part of the
> change, the header format of the HuC binary has been updated and does not
> match the GuC anymore. The GSC will perform all the required checks on
> the binary size, so we only need to check that the version matches.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c     | 99 ++++++++++++--------
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h     |  2 +
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h |  9 ++
>  3 files changed, 72 insertions(+), 38 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 cb5dd16421d0d..8d602d87a7295 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -301,45 +301,31 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
>  	}
>  }
>  
> -/**
> - * intel_uc_fw_fetch - fetch uC firmware
> - * @uc_fw: uC firmware
> - *
> - * Fetch uC firmware into GEM obj.
> - *
> - * Return: 0 on success, a negative errno code on failure.
> - */
> -int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> +static int check_gsc_manifest(const struct firmware *fw,
> +			      struct intel_uc_fw *uc_fw)
>  {
> -	struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
> -	struct device *dev = i915->drm.dev;
> -	struct drm_i915_gem_object *obj;
> -	const struct firmware *fw = NULL;
> -	struct uc_css_header *css;
> -	size_t size;
> -	int err;
> +	u32 *dw = (u32 *)fw->data;
> +	u32 version = dw[HUC_GSC_VERSION_DW];
>  
> -	GEM_BUG_ON(!i915->wopcm.size);
> -	GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
> -
> -	err = i915_inject_probe_error(i915, -ENXIO);
> -	if (err)
> -		goto fail;
> +	uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version);
> +	uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version);
>  
> -	__force_fw_fetch_failures(uc_fw, -EINVAL);
> -	__force_fw_fetch_failures(uc_fw, -ESTALE);
> +	return 0;
> +}
>  
> -	err = request_firmware(&fw, uc_fw->path, dev);
> -	if (err)
> -		goto fail;
> +static int check_ccs_header(struct drm_i915_private *i915,
> +			    const struct firmware *fw,
> +			    struct intel_uc_fw *uc_fw)
> +{
> +	struct uc_css_header *css;
> +	size_t size;
>  
>  	/* Check the size of the blob before examining buffer contents */
>  	if (unlikely(fw->size < sizeof(struct uc_css_header))) {
>  		drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
>  			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
>  			 fw->size, sizeof(struct uc_css_header));
> -		err = -ENODATA;
> -		goto fail;
> +		return -ENODATA;
>  	}
>  
>  	css = (struct uc_css_header *)fw->data;
> @@ -352,8 +338,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>  			 "%s firmware %s: unexpected header size: %zu != %zu\n",
>  			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
>  			 fw->size, sizeof(struct uc_css_header));
> -		err = -EPROTO;
> -		goto fail;
> +		return -EPROTO;
>  	}
>  
>  	/* uCode size must calculated from other sizes */
> @@ -368,8 +353,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>  		drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n",
>  			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
>  			 fw->size, size);
> -		err = -ENOEXEC;
> -		goto fail;
> +		return -ENOEXEC;
>  	}
>  
>  	/* Sanity check whether this fw is not larger than whole WOPCM memory */
> @@ -378,8 +362,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>  		drm_warn(&i915->drm, "%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;
> +		return -E2BIG;
>  	}
>  
>  	/* Get version numbers from the CSS header */
> @@ -388,6 +371,49 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>  	uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
>  					   css->sw_version);
>  
> +	if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> +		uc_fw->private_data_size = css->private_data_size;
> +
> +	return 0;
> +}
> +
> +/**
> + * intel_uc_fw_fetch - fetch uC firmware
> + * @uc_fw: uC firmware
> + *
> + * Fetch uC firmware into GEM obj.
> + *
> + * Return: 0 on success, a negative errno code on failure.
> + */
> +int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> +{
> +	struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
> +	struct device *dev = i915->drm.dev;
> +	struct drm_i915_gem_object *obj;
> +	const struct firmware *fw = NULL;
> +	int err;
> +
> +	GEM_BUG_ON(!i915->wopcm.size);
> +	GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
> +
> +	err = i915_inject_probe_error(i915, -ENXIO);
> +	if (err)
> +		goto fail;
> +
> +	__force_fw_fetch_failures(uc_fw, -EINVAL);
> +	__force_fw_fetch_failures(uc_fw, -ESTALE);
> +
> +	err = request_firmware(&fw, uc_fw->path, dev);
> +	if (err)
> +		goto fail;
> +
> +	if (uc_fw->loaded_via_gsc)
> +		err = check_gsc_manifest(fw, uc_fw);
> +	else
> +		err = check_ccs_header(i915, fw, uc_fw);
> +	if (err)
> +		goto fail;
> +
>  	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
>  	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
>  		drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> @@ -400,9 +426,6 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>  		}
>  	}
>  
> -	if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> -		uc_fw->private_data_size = css->private_data_size;
> -
>  	if (HAS_LMEM(i915)) {
>  		obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size);
>  		if (!IS_ERR(obj))
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 3229018877d3d..4f169035f5041 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -102,6 +102,8 @@ struct intel_uc_fw {
>  	u32 ucode_size;
>  
>  	u32 private_data_size;
> +
> +	bool loaded_via_gsc;
>  };
>  
>  #ifdef CONFIG_DRM_I915_DEBUG_GUC
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> index e41ffc7a7fbcb..b05e0e35b734c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> @@ -39,6 +39,11 @@
>   * 3. Length info of each component can be found in header, in dwords.
>   * 4. Modulus and exponent key are not required by driver. They may not appear
>   *    in fw. So driver will load a truncated firmware in this case.
> + *
> + * Starting from DG2, the HuC is loaded by the GSC instead of i915. The GSC
> + * firmware performs all the required integrity checks, we just need to check
> + * the version. Note that the header for GSC-managed blobs is different from the
> + * CSS used for dma-loaded firmwares.
>   */
>  
>  struct uc_css_header {
> @@ -78,4 +83,8 @@ struct uc_css_header {
>  } __packed;
>  static_assert(sizeof(struct uc_css_header) == 128);
>  
> +#define HUC_GSC_VERSION_DW		44
> +#define   HUC_GSC_MAJOR_VER_MASK	(0xFF << 0)
> +#define   HUC_GSC_MINOR_VER_MASK	(0xFF << 16)
> +
>  #endif /* _INTEL_UC_FW_ABI_H */