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.ale...@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.ceraolospu...@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 */
