diff mbox series

[v3,2/7] drm/i915/huc: Parse the GSC-enabled HuC binary

Message ID 20230527005242.1346093-3-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: HuC loading and authentication for MTL | expand

Commit Message

Daniele Ceraolo Spurio May 27, 2023, 12:52 a.m. UTC
The new binaries that support the 2-step authentication contain the
legacy-style binary, which we can use for loading the HuC via DMA. To
find out where this is located in the image, we need to parse the
manifest of the GSC-enabled HuC binary. The manifest consist of a
partition header followed by entries, one of which contains the offset
we're looking for.
Note that the DG2 GSC binary contains entries with the same names, but
it doesn't contain a full legacy binary, so we need to skip assigning
the dma offset in that case (which we can do by checking the ccs).
Also, since we're now parsing the entries, we can extract the HuC
version that way instead of using hardcoded offsets.

Note that the GSC binary uses the same structures in its binary header,
so they've been added in their own header file.

v2: fix structure names to match meu defines (s/CPT/CPD/), update commit
    message, check ccs validity, drop old version location defines.

v3: drop references to the MEU tool to reduce confusion, fix log (John)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #v2
---
 .../drm/i915/gt/uc/intel_gsc_binary_headers.h |  74 ++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_huc.c        |  11 +-
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     | 135 ++++++++++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h     |   5 +-
 drivers/gpu/drm/i915/gt/uc/intel_huc_print.h  |  21 +++
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  71 +++++----
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h      |   2 +
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |   6 -
 8 files changed, 272 insertions(+), 53 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_huc_print.h

Comments

John Harrison May 30, 2023, 11:30 p.m. UTC | #1
On 5/26/2023 17:52, Daniele Ceraolo Spurio wrote:
> The new binaries that support the 2-step authentication contain the
> legacy-style binary, which we can use for loading the HuC via DMA. To
> find out where this is located in the image, we need to parse the
> manifest of the GSC-enabled HuC binary. The manifest consist of a
> partition header followed by entries, one of which contains the offset
> we're looking for.
> Note that the DG2 GSC binary contains entries with the same names, but
> it doesn't contain a full legacy binary, so we need to skip assigning
> the dma offset in that case (which we can do by checking the ccs).
> Also, since we're now parsing the entries, we can extract the HuC
> version that way instead of using hardcoded offsets.
>
> Note that the GSC binary uses the same structures in its binary header,
> so they've been added in their own header file.
>
> v2: fix structure names to match meu defines (s/CPT/CPD/), update commit
>      message, check ccs validity, drop old version location defines.
>
> v3: drop references to the MEU tool to reduce confusion, fix log (John)
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #v2
> ---
>   .../drm/i915/gt/uc/intel_gsc_binary_headers.h |  74 ++++++++++
>   drivers/gpu/drm/i915/gt/uc/intel_huc.c        |  11 +-
>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     | 135 ++++++++++++++++++
>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h     |   5 +-
>   drivers/gpu/drm/i915/gt/uc/intel_huc_print.h  |  21 +++
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  71 +++++----
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h      |   2 +
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |   6 -
>   8 files changed, 272 insertions(+), 53 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
> new file mode 100644
> index 000000000000..714f0c256118
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _INTEL_GSC_BINARY_HEADERS_H_
> +#define _INTEL_GSC_BINARY_HEADERS_H_
> +
> +#include <linux/types.h>
> +
> +/* Code partition directory (CPD) structures */
> +struct intel_gsc_cpd_header_v2 {
> +	u32 header_marker;
> +#define INTEL_GSC_CPD_HEADER_MARKER 0x44504324
> +
> +	u32 num_of_entries;
> +	u8 header_version;
> +	u8 entry_version;
> +	u8 header_length; /* in bytes */
> +	u8 flags;
> +	u32 partition_name;
> +	u32 crc32;
> +} __packed;
> +
> +struct intel_gsc_cpd_entry {
> +	u8 name[12];
> +
> +	/*
> +	 * Bits 0-24: offset from the beginning of the code partition
> +	 * Bit 25: huffman compressed
> +	 * Bits 26-31: reserved
> +	 */
> +	u32 offset;
> +#define INTEL_GSC_CPD_ENTRY_OFFSET_MASK GENMASK(24, 0)
> +#define INTEL_GSC_CPD_ENTRY_HUFFMAN_COMP BIT(25)
> +
> +	/*
> +	 * Module/Item length, in bytes. For Huffman-compressed modules, this
> +	 * refers to the uncompressed size. For software-compressed modules,
> +	 * this refers to the compressed size.
> +	 */
> +	u32 length;
> +
> +	u8 reserved[4];
> +} __packed;
> +
> +struct intel_gsc_version {
> +	u16 major;
> +	u16 minor;
> +	u16 hotfix;
> +	u16 build;
> +} __packed;
> +
> +struct intel_gsc_manifest_header {
> +	u32 header_type; /* 0x4 for manifest type */
> +	u32 header_length; /* in dwords */
> +	u32 header_version;
> +	u32 flags;
> +	u32 vendor;
> +	u32 date;
> +	u32 size; /* In dwords, size of entire manifest (header + extensions) */
> +	u32 header_id;
> +	u32 internal_data;
> +	struct intel_gsc_version fw_version;
> +	u32 security_version;
> +	struct intel_gsc_version meu_kit_version;
> +	u32 meu_manifest_version;
> +	u8 general_data[4];
> +	u8 reserved3[56];
> +	u32 modulus_size; /* in dwords */
> +	u32 exponent_size; /* in dwords */
> +} __packed;
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 268e036f8f28..6d795438b3e4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -6,23 +6,14 @@
>   #include <linux/types.h>
>   
>   #include "gt/intel_gt.h"
> -#include "gt/intel_gt_print.h"
>   #include "intel_guc_reg.h"
>   #include "intel_huc.h"
> +#include "intel_huc_print.h"
>   #include "i915_drv.h"
>   
>   #include <linux/device/bus.h>
>   #include <linux/mei_aux.h>
>   
> -#define huc_printk(_huc, _level, _fmt, ...) \
> -	gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
> -#define huc_err(_huc, _fmt, ...)	huc_printk((_huc), err, _fmt, ##__VA_ARGS__)
> -#define huc_warn(_huc, _fmt, ...)	huc_printk((_huc), warn, _fmt, ##__VA_ARGS__)
> -#define huc_notice(_huc, _fmt, ...)	huc_printk((_huc), notice, _fmt, ##__VA_ARGS__)
> -#define huc_info(_huc, _fmt, ...)	huc_printk((_huc), info, _fmt, ##__VA_ARGS__)
> -#define huc_dbg(_huc, _fmt, ...)	huc_printk((_huc), dbg, _fmt, ##__VA_ARGS__)
> -#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), probe_error, _fmt, ##__VA_ARGS__)
> -
>   /**
>    * DOC: HuC
>    *
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index 534b0aa43316..037d2ad4879d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -5,11 +5,146 @@
>   
>   #include "gt/intel_gsc.h"
>   #include "gt/intel_gt.h"
> +#include "intel_gsc_binary_headers.h"
>   #include "intel_huc.h"
>   #include "intel_huc_fw.h"
> +#include "intel_huc_print.h"
>   #include "i915_drv.h"
>   #include "pxp/intel_pxp_huc.h"
>   
> +static void get_version_from_gsc_manifest(struct intel_uc_fw_ver *ver, const void *data)
> +{
> +	const struct intel_gsc_manifest_header *manifest = data;
> +
> +	ver->major = manifest->fw_version.major;
> +	ver->minor = manifest->fw_version.minor;
> +	ver->patch = manifest->fw_version.hotfix;
> +}
> +
> +static bool css_valid(const void *data, size_t size)
> +{
> +	const struct uc_css_header *css = data;
> +
> +	if (unlikely(size < sizeof(struct uc_css_header)))
> +		return false;
> +
> +	if (css->module_type != 0x6)
> +		return false;
> +
> +	if (css->module_vendor != PCI_VENDOR_ID_INTEL)
> +		return false;
> +
> +	return true;
> +}
> +
> +static inline u32 entry_offset(const struct intel_gsc_cpd_entry *entry)
> +{
> +	return entry->offset & INTEL_GSC_CPD_ENTRY_OFFSET_MASK;
> +}
> +
> +int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const void *data, size_t size)
> +{
> +	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> +	const struct intel_gsc_cpd_header_v2 *header = data;
> +	const struct intel_gsc_cpd_entry *entry;
> +	size_t min_size = sizeof(*header);
> +	int i;
> +
> +	if (!huc_fw->loaded_via_gsc) {
> +		huc_err(huc, "Invalid FW type GSC header parsing!\n");
I'm still not understanding what this error means. Is it meant to say 
'Invalid FW type *for* GSC header parsing'?

'fw->loaded_via_gsc' is set from the blob table, yes? And 
intel_huc_fw_get_binary_info() is only called from check_gsc_manifest() 
which is called from check_fw_header() iff fw->loaded_via_gsc is set. So 
it should be impossible for this test to ever fail, yes?

John.

> +		return -EINVAL;
> +	}
> +
> +	if (size < sizeof(*header)) {
> +		huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
> +		return -ENODATA;
> +	}
> +
> +	/*
> +	 * The GSC-enabled HuC binary starts with a directory header, followed
> +	 * by a series of entries. Each entry is identified by a name and
> +	 * points to a specific section of the binary containing the relevant
> +	 * data. The entries we're interested in are:
> +	 * - "HUCP.man": points to the GSC manifest header for the HuC, which
> +	 *               contains the version info.
> +	 * - "huc_fw": points to the legacy-style binary that can be used for
> +	 *             load via the DMA. This entry only contains a valid CSS
> +	 *             on binaries for platforms that support 2-step HuC load
> +	 *             via dma and auth via GSC (like MTL).
> +	 *
> +	 * --------------------------------------------------
> +	 * [  intel_gsc_cpd_header_v2                       ]
> +	 * --------------------------------------------------
> +	 * [  intel_gsc_cpd_entry[]                         ]
> +	 * [      entry1                                    ]
> +	 * [      ...                                       ]
> +	 * [      entryX                                    ]
> +	 * [          "HUCP.man"                            ]
> +	 * [           ...                                  ]
> +	 * [           offset  >----------------------------]------o
> +	 * [      ...                                       ]      |
> +	 * [      entryY                                    ]      |
> +	 * [          "huc_fw"                              ]      |
> +	 * [           ...                                  ]      |
> +	 * [           offset  >----------------------------]----------o
> +	 * --------------------------------------------------      |   |
> +	 *                                                         |   |
> +	 * --------------------------------------------------      |   |
> +	 * [ intel_gsc_manifest_header                      ]<-----o   |
> +	 * [  ...                                           ]          |
> +	 * [  intel_gsc_version fw_version                  ]          |
> +	 * [  ...                                           ]          |
> +	 * --------------------------------------------------          |
> +	 *                                                             |
> +	 * --------------------------------------------------          |
> +	 * [ data[]                                         ]<---------o
> +	 * [  ...                                           ]
> +	 * [  ...                                           ]
> +	 * --------------------------------------------------
> +	 */
> +
> +	if (header->header_marker != INTEL_GSC_CPD_HEADER_MARKER) {
> +		huc_err(huc, "invalid marker for CPD header: 0x%08x!\n",
> +			header->header_marker);
> +		return -EINVAL;
> +	}
> +
> +	/* we only have binaries with header v2 and entry v1 for now */
> +	if (header->header_version != 2 || header->entry_version != 1) {
> +		huc_err(huc, "invalid CPD header/entry version %u:%u!\n",
> +			header->header_version, header->entry_version);
> +		return -EINVAL;
> +	}
> +
> +	if (header->header_length < sizeof(struct intel_gsc_cpd_header_v2)) {
> +		huc_err(huc, "invalid CPD header length %u!\n",
> +			header->header_length);
> +		return -EINVAL;
> +	}
> +
> +	min_size = header->header_length + sizeof(*entry) * header->num_of_entries;
> +	if (size < min_size) {
> +		huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
> +		return -ENODATA;
> +	}
> +
> +	entry = data + header->header_length;
> +
> +	for (i = 0; i < header->num_of_entries; i++, entry++) {
> +		if (strcmp(entry->name, "HUCP.man") == 0)
> +			get_version_from_gsc_manifest(&huc_fw->file_selected.ver,
> +						      data + entry_offset(entry));
> +
> +		if (strcmp(entry->name, "huc_fw") == 0) {
> +			u32 offset = entry_offset(entry);
> +			if (offset < size && css_valid(data + offset, size - offset))
> +				huc_fw->dma_start_offset = offset;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
>   {
>   	int ret;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
> index db42e238b45f..0999ffe6f962 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
> @@ -7,8 +7,11 @@
>   #define _INTEL_HUC_FW_H_
>   
>   struct intel_huc;
> +struct intel_uc_fw;
> +
> +#include <linux/types.h>
>   
>   int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc);
>   int intel_huc_fw_upload(struct intel_huc *huc);
> -
> +int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const void *data, size_t size);
>   #endif
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
> new file mode 100644
> index 000000000000..915d310ee1df
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef __INTEL_HUC_PRINT__
> +#define __INTEL_HUC_PRINT__
> +
> +#include "gt/intel_gt.h"
> +#include "gt/intel_gt_print.h"
> +
> +#define huc_printk(_huc, _level, _fmt, ...) \
> +	gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
> +#define huc_err(_huc, _fmt, ...)	huc_printk((_huc), err, _fmt, ##__VA_ARGS__)
> +#define huc_warn(_huc, _fmt, ...)	huc_printk((_huc), warn, _fmt, ##__VA_ARGS__)
> +#define huc_notice(_huc, _fmt, ...)	huc_printk((_huc), notice, _fmt, ##__VA_ARGS__)
> +#define huc_info(_huc, _fmt, ...)	huc_printk((_huc), info, _fmt, ##__VA_ARGS__)
> +#define huc_dbg(_huc, _fmt, ...)	huc_printk((_huc), dbg, _fmt, ##__VA_ARGS__)
> +#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), probe_error, _fmt, ##__VA_ARGS__)
> +
> +#endif /* __INTEL_HUC_PRINT__ */
> 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 31776c279f32..9ced8dbf1253 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -548,33 +548,6 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
>   	}
>   }
>   
> -static int check_gsc_manifest(struct intel_gt *gt,
> -			      const struct firmware *fw,
> -			      struct intel_uc_fw *uc_fw)
> -{
> -	u32 *dw = (u32 *)fw->data;
> -	u32 version_hi, version_lo;
> -	size_t min_size;
> -
> -	/* Check the size of the blob before examining buffer contents */
> -	min_size = sizeof(u32) * (HUC_GSC_VERSION_LO_DW + 1);
> -	if (unlikely(fw->size < min_size)) {
> -		gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
> -			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
> -			fw->size, min_size);
> -		return -ENODATA;
> -	}
> -
> -	version_hi = dw[HUC_GSC_VERSION_HI_DW];
> -	version_lo = dw[HUC_GSC_VERSION_LO_DW];
> -
> -	uc_fw->file_selected.ver.major = FIELD_GET(HUC_GSC_MAJOR_VER_HI_MASK, version_hi);
> -	uc_fw->file_selected.ver.minor = FIELD_GET(HUC_GSC_MINOR_VER_HI_MASK, version_hi);
> -	uc_fw->file_selected.ver.patch = FIELD_GET(HUC_GSC_PATCH_VER_LO_MASK, version_lo);
> -
> -	return 0;
> -}
> -
>   static void uc_unpack_css_version(struct intel_uc_fw_ver *ver, u32 css_value)
>   {
>   	/* Get version numbers from the CSS header */
> @@ -631,22 +604,22 @@ static void guc_read_css_info(struct intel_uc_fw *uc_fw, struct uc_css_header *c
>   	uc_fw->private_data_size = css->private_data_size;
>   }
>   
> -static int check_ccs_header(struct intel_gt *gt,
> -			    const struct firmware *fw,
> -			    struct intel_uc_fw *uc_fw)
> +static int __check_ccs_header(struct intel_gt *gt,
> +			      const void *fw_data, size_t fw_size,
> +			      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))) {
> +	if (unlikely(fw_size < sizeof(struct uc_css_header))) {
>   		gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>   			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
> -			fw->size, sizeof(struct uc_css_header));
> +			fw_size, sizeof(struct uc_css_header));
>   		return -ENODATA;
>   	}
>   
> -	css = (struct uc_css_header *)fw->data;
> +	css = (struct uc_css_header *)fw_data;
>   
>   	/* Check integrity of size values inside CSS header */
>   	size = (css->header_size_dw - css->key_size_dw - css->modulus_size_dw -
> @@ -654,7 +627,7 @@ static int check_ccs_header(struct intel_gt *gt,
>   	if (unlikely(size != sizeof(struct uc_css_header))) {
>   		gt_warn(gt, "%s firmware %s: unexpected header size: %zu != %zu\n",
>   			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
> -			fw->size, sizeof(struct uc_css_header));
> +			fw_size, sizeof(struct uc_css_header));
>   		return -EPROTO;
>   	}
>   
> @@ -666,10 +639,10 @@ static int check_ccs_header(struct intel_gt *gt,
>   
>   	/* At least, it should have header, uCode and RSA. Size of all three. */
>   	size = sizeof(struct uc_css_header) + uc_fw->ucode_size + uc_fw->rsa_size;
> -	if (unlikely(fw->size < size)) {
> +	if (unlikely(fw_size < size)) {
>   		gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>   			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
> -			fw->size, size);
> +			fw_size, size);
>   		return -ENOEXEC;
>   	}
>   
> @@ -690,6 +663,32 @@ static int check_ccs_header(struct intel_gt *gt,
>   	return 0;
>   }
>   
> +static int check_gsc_manifest(struct intel_gt *gt,
> +			      const struct firmware *fw,
> +			      struct intel_uc_fw *uc_fw)
> +{
> +	if (uc_fw->type != INTEL_UC_FW_TYPE_HUC) {
> +		gt_err(gt, "trying to GSC-parse a non-HuC binary");
> +		return -EINVAL;
> +	}
> +
> +	intel_huc_fw_get_binary_info(uc_fw, fw->data, fw->size);
> +
> +	if (uc_fw->dma_start_offset) {
> +		u32 delta = uc_fw->dma_start_offset;
> +		__check_ccs_header(gt, fw->data + delta, fw->size - delta, uc_fw);
> +	}
> +
> +	return 0;
> +}
> +
> +static int check_ccs_header(struct intel_gt *gt,
> +			    const struct firmware *fw,
> +			    struct intel_uc_fw *uc_fw)
> +{
> +	return __check_ccs_header(gt, fw->data, fw->size, uc_fw);
> +}
> +
>   static bool is_ver_8bit(struct intel_uc_fw_ver *ver)
>   {
>   	return ver->major < 0xFF && ver->minor < 0xFF && ver->patch < 0xFF;
> 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 2be9470eb712..b3daba9526eb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -118,6 +118,8 @@ struct intel_uc_fw {
>   	u32 ucode_size;
>   	u32 private_data_size;
>   
> +	u32 dma_start_offset;
> +
>   	bool loaded_via_gsc;
>   };
>   
> 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 646fa8aa6cf1..7fe405126249 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
> @@ -84,10 +84,4 @@ struct uc_css_header {
>   } __packed;
>   static_assert(sizeof(struct uc_css_header) == 128);
>   
> -#define HUC_GSC_VERSION_HI_DW		44
> -#define   HUC_GSC_MAJOR_VER_HI_MASK	(0xFF << 0)
> -#define   HUC_GSC_MINOR_VER_HI_MASK	(0xFF << 16)
> -#define HUC_GSC_VERSION_LO_DW		45
> -#define   HUC_GSC_PATCH_VER_LO_MASK	(0xFF << 0)
> -
>   #endif /* _INTEL_UC_FW_ABI_H */
Daniele Ceraolo Spurio May 31, 2023, 12:06 a.m. UTC | #2
On 5/30/2023 4:30 PM, John Harrison wrote:
> On 5/26/2023 17:52, Daniele Ceraolo Spurio wrote:
>> The new binaries that support the 2-step authentication contain the
>> legacy-style binary, which we can use for loading the HuC via DMA. To
>> find out where this is located in the image, we need to parse the
>> manifest of the GSC-enabled HuC binary. The manifest consist of a
>> partition header followed by entries, one of which contains the offset
>> we're looking for.
>> Note that the DG2 GSC binary contains entries with the same names, but
>> it doesn't contain a full legacy binary, so we need to skip assigning
>> the dma offset in that case (which we can do by checking the ccs).
>> Also, since we're now parsing the entries, we can extract the HuC
>> version that way instead of using hardcoded offsets.
>>
>> Note that the GSC binary uses the same structures in its binary header,
>> so they've been added in their own header file.
>>
>> v2: fix structure names to match meu defines (s/CPT/CPD/), update commit
>>      message, check ccs validity, drop old version location defines.
>>
>> v3: drop references to the MEU tool to reduce confusion, fix log (John)
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #v2
>> ---
>>   .../drm/i915/gt/uc/intel_gsc_binary_headers.h |  74 ++++++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_huc.c        |  11 +-
>>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     | 135 ++++++++++++++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h     |   5 +-
>>   drivers/gpu/drm/i915/gt/uc/intel_huc_print.h  |  21 +++
>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  71 +++++----
>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h      |   2 +
>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |   6 -
>>   8 files changed, 272 insertions(+), 53 deletions(-)
>>   create mode 100644 
>> drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
>>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
>> new file mode 100644
>> index 000000000000..714f0c256118
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
>> @@ -0,0 +1,74 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#ifndef _INTEL_GSC_BINARY_HEADERS_H_
>> +#define _INTEL_GSC_BINARY_HEADERS_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/* Code partition directory (CPD) structures */
>> +struct intel_gsc_cpd_header_v2 {
>> +    u32 header_marker;
>> +#define INTEL_GSC_CPD_HEADER_MARKER 0x44504324
>> +
>> +    u32 num_of_entries;
>> +    u8 header_version;
>> +    u8 entry_version;
>> +    u8 header_length; /* in bytes */
>> +    u8 flags;
>> +    u32 partition_name;
>> +    u32 crc32;
>> +} __packed;
>> +
>> +struct intel_gsc_cpd_entry {
>> +    u8 name[12];
>> +
>> +    /*
>> +     * Bits 0-24: offset from the beginning of the code partition
>> +     * Bit 25: huffman compressed
>> +     * Bits 26-31: reserved
>> +     */
>> +    u32 offset;
>> +#define INTEL_GSC_CPD_ENTRY_OFFSET_MASK GENMASK(24, 0)
>> +#define INTEL_GSC_CPD_ENTRY_HUFFMAN_COMP BIT(25)
>> +
>> +    /*
>> +     * Module/Item length, in bytes. For Huffman-compressed modules, 
>> this
>> +     * refers to the uncompressed size. For software-compressed 
>> modules,
>> +     * this refers to the compressed size.
>> +     */
>> +    u32 length;
>> +
>> +    u8 reserved[4];
>> +} __packed;
>> +
>> +struct intel_gsc_version {
>> +    u16 major;
>> +    u16 minor;
>> +    u16 hotfix;
>> +    u16 build;
>> +} __packed;
>> +
>> +struct intel_gsc_manifest_header {
>> +    u32 header_type; /* 0x4 for manifest type */
>> +    u32 header_length; /* in dwords */
>> +    u32 header_version;
>> +    u32 flags;
>> +    u32 vendor;
>> +    u32 date;
>> +    u32 size; /* In dwords, size of entire manifest (header + 
>> extensions) */
>> +    u32 header_id;
>> +    u32 internal_data;
>> +    struct intel_gsc_version fw_version;
>> +    u32 security_version;
>> +    struct intel_gsc_version meu_kit_version;
>> +    u32 meu_manifest_version;
>> +    u8 general_data[4];
>> +    u8 reserved3[56];
>> +    u32 modulus_size; /* in dwords */
>> +    u32 exponent_size; /* in dwords */
>> +} __packed;
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> index 268e036f8f28..6d795438b3e4 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> @@ -6,23 +6,14 @@
>>   #include <linux/types.h>
>>     #include "gt/intel_gt.h"
>> -#include "gt/intel_gt_print.h"
>>   #include "intel_guc_reg.h"
>>   #include "intel_huc.h"
>> +#include "intel_huc_print.h"
>>   #include "i915_drv.h"
>>     #include <linux/device/bus.h>
>>   #include <linux/mei_aux.h>
>>   -#define huc_printk(_huc, _level, _fmt, ...) \
>> -    gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
>> -#define huc_err(_huc, _fmt, ...)    huc_printk((_huc), err, _fmt, 
>> ##__VA_ARGS__)
>> -#define huc_warn(_huc, _fmt, ...)    huc_printk((_huc), warn, _fmt, 
>> ##__VA_ARGS__)
>> -#define huc_notice(_huc, _fmt, ...)    huc_printk((_huc), notice, 
>> _fmt, ##__VA_ARGS__)
>> -#define huc_info(_huc, _fmt, ...)    huc_printk((_huc), info, _fmt, 
>> ##__VA_ARGS__)
>> -#define huc_dbg(_huc, _fmt, ...)    huc_printk((_huc), dbg, _fmt, 
>> ##__VA_ARGS__)
>> -#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), 
>> probe_error, _fmt, ##__VA_ARGS__)
>> -
>>   /**
>>    * DOC: HuC
>>    *
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>> index 534b0aa43316..037d2ad4879d 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>> @@ -5,11 +5,146 @@
>>     #include "gt/intel_gsc.h"
>>   #include "gt/intel_gt.h"
>> +#include "intel_gsc_binary_headers.h"
>>   #include "intel_huc.h"
>>   #include "intel_huc_fw.h"
>> +#include "intel_huc_print.h"
>>   #include "i915_drv.h"
>>   #include "pxp/intel_pxp_huc.h"
>>   +static void get_version_from_gsc_manifest(struct intel_uc_fw_ver 
>> *ver, const void *data)
>> +{
>> +    const struct intel_gsc_manifest_header *manifest = data;
>> +
>> +    ver->major = manifest->fw_version.major;
>> +    ver->minor = manifest->fw_version.minor;
>> +    ver->patch = manifest->fw_version.hotfix;
>> +}
>> +
>> +static bool css_valid(const void *data, size_t size)
>> +{
>> +    const struct uc_css_header *css = data;
>> +
>> +    if (unlikely(size < sizeof(struct uc_css_header)))
>> +        return false;
>> +
>> +    if (css->module_type != 0x6)
>> +        return false;
>> +
>> +    if (css->module_vendor != PCI_VENDOR_ID_INTEL)
>> +        return false;
>> +
>> +    return true;
>> +}
>> +
>> +static inline u32 entry_offset(const struct intel_gsc_cpd_entry *entry)
>> +{
>> +    return entry->offset & INTEL_GSC_CPD_ENTRY_OFFSET_MASK;
>> +}
>> +
>> +int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const 
>> void *data, size_t size)
>> +{
>> +    struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
>> +    const struct intel_gsc_cpd_header_v2 *header = data;
>> +    const struct intel_gsc_cpd_entry *entry;
>> +    size_t min_size = sizeof(*header);
>> +    int i;
>> +
>> +    if (!huc_fw->loaded_via_gsc) {
>> +        huc_err(huc, "Invalid FW type GSC header parsing!\n");
> I'm still not understanding what this error means. Is it meant to say 
> 'Invalid FW type *for* GSC header parsing'?

yes, sorry that was the idea. I'll add the "for"

>
> 'fw->loaded_via_gsc' is set from the blob table, yes? And 
> intel_huc_fw_get_binary_info() is only called from 
> check_gsc_manifest() which is called from check_fw_header() iff 
> fw->loaded_via_gsc is set. So it should be impossible for this test to 
> ever fail, yes?

Yes, this should never fail, but with GSC FW the code changes again so I 
wanted to make sure I had a check in there in case I got things wrong. 
There would be other failures anyway (because the parsing wouldn't work, 
so I can drop this check if you think it is redundant.

Daniele

>
> John.
>
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (size < sizeof(*header)) {
>> +        huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
>> +        return -ENODATA;
>> +    }
>> +
>> +    /*
>> +     * The GSC-enabled HuC binary starts with a directory header, 
>> followed
>> +     * by a series of entries. Each entry is identified by a name and
>> +     * points to a specific section of the binary containing the 
>> relevant
>> +     * data. The entries we're interested in are:
>> +     * - "HUCP.man": points to the GSC manifest header for the HuC, 
>> which
>> +     *               contains the version info.
>> +     * - "huc_fw": points to the legacy-style binary that can be 
>> used for
>> +     *             load via the DMA. This entry only contains a 
>> valid CSS
>> +     *             on binaries for platforms that support 2-step HuC 
>> load
>> +     *             via dma and auth via GSC (like MTL).
>> +     *
>> +     * --------------------------------------------------
>> +     * [  intel_gsc_cpd_header_v2                       ]
>> +     * --------------------------------------------------
>> +     * [  intel_gsc_cpd_entry[]                         ]
>> +     * [      entry1                                    ]
>> +     * [      ...                                       ]
>> +     * [      entryX                                    ]
>> +     * [          "HUCP.man"                            ]
>> +     * [           ...                                  ]
>> +     * [           offset >----------------------------]------o
>> +     * [      ...                                       ] |
>> +     * [      entryY                                    ] |
>> +     * [          "huc_fw"                              ] |
>> +     * [           ...                                  ] |
>> +     * [           offset >----------------------------]----------o
>> +     * -------------------------------------------------- |   |
>> +     * |   |
>> +     * -------------------------------------------------- |   |
>> +     * [ intel_gsc_manifest_header ]<-----o   |
>> +     * [  ... ]          |
>> +     * [  intel_gsc_version fw_version ]          |
>> +     * [  ... ]          |
>> +     * --------------------------------------------------          |
>> + *                                                             |
>> +     * --------------------------------------------------          |
>> +     * [ data[] ]<---------o
>> +     * [  ...                                           ]
>> +     * [  ...                                           ]
>> +     * --------------------------------------------------
>> +     */
>> +
>> +    if (header->header_marker != INTEL_GSC_CPD_HEADER_MARKER) {
>> +        huc_err(huc, "invalid marker for CPD header: 0x%08x!\n",
>> +            header->header_marker);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* we only have binaries with header v2 and entry v1 for now */
>> +    if (header->header_version != 2 || header->entry_version != 1) {
>> +        huc_err(huc, "invalid CPD header/entry version %u:%u!\n",
>> +            header->header_version, header->entry_version);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (header->header_length < sizeof(struct 
>> intel_gsc_cpd_header_v2)) {
>> +        huc_err(huc, "invalid CPD header length %u!\n",
>> +            header->header_length);
>> +        return -EINVAL;
>> +    }
>> +
>> +    min_size = header->header_length + sizeof(*entry) * 
>> header->num_of_entries;
>> +    if (size < min_size) {
>> +        huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
>> +        return -ENODATA;
>> +    }
>> +
>> +    entry = data + header->header_length;
>> +
>> +    for (i = 0; i < header->num_of_entries; i++, entry++) {
>> +        if (strcmp(entry->name, "HUCP.man") == 0)
>> + get_version_from_gsc_manifest(&huc_fw->file_selected.ver,
>> +                              data + entry_offset(entry));
>> +
>> +        if (strcmp(entry->name, "huc_fw") == 0) {
>> +            u32 offset = entry_offset(entry);
>> +            if (offset < size && css_valid(data + offset, size - 
>> offset))
>> +                huc_fw->dma_start_offset = offset;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
>>   {
>>       int ret;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
>> index db42e238b45f..0999ffe6f962 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
>> @@ -7,8 +7,11 @@
>>   #define _INTEL_HUC_FW_H_
>>     struct intel_huc;
>> +struct intel_uc_fw;
>> +
>> +#include <linux/types.h>
>>     int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc);
>>   int intel_huc_fw_upload(struct intel_huc *huc);
>> -
>> +int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const 
>> void *data, size_t size);
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
>> new file mode 100644
>> index 000000000000..915d310ee1df
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
>> @@ -0,0 +1,21 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#ifndef __INTEL_HUC_PRINT__
>> +#define __INTEL_HUC_PRINT__
>> +
>> +#include "gt/intel_gt.h"
>> +#include "gt/intel_gt_print.h"
>> +
>> +#define huc_printk(_huc, _level, _fmt, ...) \
>> +    gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
>> +#define huc_err(_huc, _fmt, ...)    huc_printk((_huc), err, _fmt, 
>> ##__VA_ARGS__)
>> +#define huc_warn(_huc, _fmt, ...)    huc_printk((_huc), warn, _fmt, 
>> ##__VA_ARGS__)
>> +#define huc_notice(_huc, _fmt, ...)    huc_printk((_huc), notice, 
>> _fmt, ##__VA_ARGS__)
>> +#define huc_info(_huc, _fmt, ...)    huc_printk((_huc), info, _fmt, 
>> ##__VA_ARGS__)
>> +#define huc_dbg(_huc, _fmt, ...)    huc_printk((_huc), dbg, _fmt, 
>> ##__VA_ARGS__)
>> +#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), 
>> probe_error, _fmt, ##__VA_ARGS__)
>> +
>> +#endif /* __INTEL_HUC_PRINT__ */
>> 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 31776c279f32..9ced8dbf1253 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -548,33 +548,6 @@ static void __force_fw_fetch_failures(struct 
>> intel_uc_fw *uc_fw, int e)
>>       }
>>   }
>>   -static int check_gsc_manifest(struct intel_gt *gt,
>> -                  const struct firmware *fw,
>> -                  struct intel_uc_fw *uc_fw)
>> -{
>> -    u32 *dw = (u32 *)fw->data;
>> -    u32 version_hi, version_lo;
>> -    size_t min_size;
>> -
>> -    /* Check the size of the blob before examining buffer contents */
>> -    min_size = sizeof(u32) * (HUC_GSC_VERSION_LO_DW + 1);
>> -    if (unlikely(fw->size < min_size)) {
>> -        gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>> -            intel_uc_fw_type_repr(uc_fw->type), 
>> uc_fw->file_selected.path,
>> -            fw->size, min_size);
>> -        return -ENODATA;
>> -    }
>> -
>> -    version_hi = dw[HUC_GSC_VERSION_HI_DW];
>> -    version_lo = dw[HUC_GSC_VERSION_LO_DW];
>> -
>> -    uc_fw->file_selected.ver.major = 
>> FIELD_GET(HUC_GSC_MAJOR_VER_HI_MASK, version_hi);
>> -    uc_fw->file_selected.ver.minor = 
>> FIELD_GET(HUC_GSC_MINOR_VER_HI_MASK, version_hi);
>> -    uc_fw->file_selected.ver.patch = 
>> FIELD_GET(HUC_GSC_PATCH_VER_LO_MASK, version_lo);
>> -
>> -    return 0;
>> -}
>> -
>>   static void uc_unpack_css_version(struct intel_uc_fw_ver *ver, u32 
>> css_value)
>>   {
>>       /* Get version numbers from the CSS header */
>> @@ -631,22 +604,22 @@ static void guc_read_css_info(struct 
>> intel_uc_fw *uc_fw, struct uc_css_header *c
>>       uc_fw->private_data_size = css->private_data_size;
>>   }
>>   -static int check_ccs_header(struct intel_gt *gt,
>> -                const struct firmware *fw,
>> -                struct intel_uc_fw *uc_fw)
>> +static int __check_ccs_header(struct intel_gt *gt,
>> +                  const void *fw_data, size_t fw_size,
>> +                  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))) {
>> +    if (unlikely(fw_size < sizeof(struct uc_css_header))) {
>>           gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>>               intel_uc_fw_type_repr(uc_fw->type), 
>> uc_fw->file_selected.path,
>> -            fw->size, sizeof(struct uc_css_header));
>> +            fw_size, sizeof(struct uc_css_header));
>>           return -ENODATA;
>>       }
>>   -    css = (struct uc_css_header *)fw->data;
>> +    css = (struct uc_css_header *)fw_data;
>>         /* Check integrity of size values inside CSS header */
>>       size = (css->header_size_dw - css->key_size_dw - 
>> css->modulus_size_dw -
>> @@ -654,7 +627,7 @@ static int check_ccs_header(struct intel_gt *gt,
>>       if (unlikely(size != sizeof(struct uc_css_header))) {
>>           gt_warn(gt, "%s firmware %s: unexpected header size: %zu != 
>> %zu\n",
>>               intel_uc_fw_type_repr(uc_fw->type), 
>> uc_fw->file_selected.path,
>> -            fw->size, sizeof(struct uc_css_header));
>> +            fw_size, sizeof(struct uc_css_header));
>>           return -EPROTO;
>>       }
>>   @@ -666,10 +639,10 @@ static int check_ccs_header(struct intel_gt *gt,
>>         /* At least, it should have header, uCode and RSA. Size of 
>> all three. */
>>       size = sizeof(struct uc_css_header) + uc_fw->ucode_size + 
>> uc_fw->rsa_size;
>> -    if (unlikely(fw->size < size)) {
>> +    if (unlikely(fw_size < size)) {
>>           gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>>               intel_uc_fw_type_repr(uc_fw->type), 
>> uc_fw->file_selected.path,
>> -            fw->size, size);
>> +            fw_size, size);
>>           return -ENOEXEC;
>>       }
>>   @@ -690,6 +663,32 @@ static int check_ccs_header(struct intel_gt *gt,
>>       return 0;
>>   }
>>   +static int check_gsc_manifest(struct intel_gt *gt,
>> +                  const struct firmware *fw,
>> +                  struct intel_uc_fw *uc_fw)
>> +{
>> +    if (uc_fw->type != INTEL_UC_FW_TYPE_HUC) {
>> +        gt_err(gt, "trying to GSC-parse a non-HuC binary");
>> +        return -EINVAL;
>> +    }
>> +
>> +    intel_huc_fw_get_binary_info(uc_fw, fw->data, fw->size);
>> +
>> +    if (uc_fw->dma_start_offset) {
>> +        u32 delta = uc_fw->dma_start_offset;
>> +        __check_ccs_header(gt, fw->data + delta, fw->size - delta, 
>> uc_fw);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int check_ccs_header(struct intel_gt *gt,
>> +                const struct firmware *fw,
>> +                struct intel_uc_fw *uc_fw)
>> +{
>> +    return __check_ccs_header(gt, fw->data, fw->size, uc_fw);
>> +}
>> +
>>   static bool is_ver_8bit(struct intel_uc_fw_ver *ver)
>>   {
>>       return ver->major < 0xFF && ver->minor < 0xFF && ver->patch < 
>> 0xFF;
>> 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 2be9470eb712..b3daba9526eb 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>> @@ -118,6 +118,8 @@ struct intel_uc_fw {
>>       u32 ucode_size;
>>       u32 private_data_size;
>>   +    u32 dma_start_offset;
>> +
>>       bool loaded_via_gsc;
>>   };
>>   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 646fa8aa6cf1..7fe405126249 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
>> @@ -84,10 +84,4 @@ struct uc_css_header {
>>   } __packed;
>>   static_assert(sizeof(struct uc_css_header) == 128);
>>   -#define HUC_GSC_VERSION_HI_DW        44
>> -#define   HUC_GSC_MAJOR_VER_HI_MASK    (0xFF << 0)
>> -#define   HUC_GSC_MINOR_VER_HI_MASK    (0xFF << 16)
>> -#define HUC_GSC_VERSION_LO_DW        45
>> -#define   HUC_GSC_PATCH_VER_LO_MASK    (0xFF << 0)
>> -
>>   #endif /* _INTEL_UC_FW_ABI_H */
>
John Harrison May 31, 2023, 12:11 a.m. UTC | #3
On 5/30/2023 17:06, Ceraolo Spurio, Daniele wrote:
> On 5/30/2023 4:30 PM, John Harrison wrote:
>> On 5/26/2023 17:52, Daniele Ceraolo Spurio wrote:
>>> The new binaries that support the 2-step authentication contain the
>>> legacy-style binary, which we can use for loading the HuC via DMA. To
>>> find out where this is located in the image, we need to parse the
>>> manifest of the GSC-enabled HuC binary. The manifest consist of a
>>> partition header followed by entries, one of which contains the offset
>>> we're looking for.
>>> Note that the DG2 GSC binary contains entries with the same names, but
>>> it doesn't contain a full legacy binary, so we need to skip assigning
>>> the dma offset in that case (which we can do by checking the ccs).
>>> Also, since we're now parsing the entries, we can extract the HuC
>>> version that way instead of using hardcoded offsets.
>>>
>>> Note that the GSC binary uses the same structures in its binary header,
>>> so they've been added in their own header file.
>>>
>>> v2: fix structure names to match meu defines (s/CPT/CPD/), update 
>>> commit
>>>      message, check ccs validity, drop old version location defines.
>>>
>>> v3: drop references to the MEU tool to reduce confusion, fix log (John)
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #v2
>>> ---
>>>   .../drm/i915/gt/uc/intel_gsc_binary_headers.h |  74 ++++++++++
>>>   drivers/gpu/drm/i915/gt/uc/intel_huc.c        |  11 +-
>>>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     | 135 
>>> ++++++++++++++++++
>>>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h     |   5 +-
>>>   drivers/gpu/drm/i915/gt/uc/intel_huc_print.h  |  21 +++
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  71 +++++----
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h      |   2 +
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |   6 -
>>>   8 files changed, 272 insertions(+), 53 deletions(-)
>>>   create mode 100644 
>>> drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
>>>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
>>> new file mode 100644
>>> index 000000000000..714f0c256118
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
>>> @@ -0,0 +1,74 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
>>> + */
>>> +
>>> +#ifndef _INTEL_GSC_BINARY_HEADERS_H_
>>> +#define _INTEL_GSC_BINARY_HEADERS_H_
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +/* Code partition directory (CPD) structures */
>>> +struct intel_gsc_cpd_header_v2 {
>>> +    u32 header_marker;
>>> +#define INTEL_GSC_CPD_HEADER_MARKER 0x44504324
>>> +
>>> +    u32 num_of_entries;
>>> +    u8 header_version;
>>> +    u8 entry_version;
>>> +    u8 header_length; /* in bytes */
>>> +    u8 flags;
>>> +    u32 partition_name;
>>> +    u32 crc32;
>>> +} __packed;
>>> +
>>> +struct intel_gsc_cpd_entry {
>>> +    u8 name[12];
>>> +
>>> +    /*
>>> +     * Bits 0-24: offset from the beginning of the code partition
>>> +     * Bit 25: huffman compressed
>>> +     * Bits 26-31: reserved
>>> +     */
>>> +    u32 offset;
>>> +#define INTEL_GSC_CPD_ENTRY_OFFSET_MASK GENMASK(24, 0)
>>> +#define INTEL_GSC_CPD_ENTRY_HUFFMAN_COMP BIT(25)
>>> +
>>> +    /*
>>> +     * Module/Item length, in bytes. For Huffman-compressed 
>>> modules, this
>>> +     * refers to the uncompressed size. For software-compressed 
>>> modules,
>>> +     * this refers to the compressed size.
>>> +     */
>>> +    u32 length;
>>> +
>>> +    u8 reserved[4];
>>> +} __packed;
>>> +
>>> +struct intel_gsc_version {
>>> +    u16 major;
>>> +    u16 minor;
>>> +    u16 hotfix;
>>> +    u16 build;
>>> +} __packed;
>>> +
>>> +struct intel_gsc_manifest_header {
>>> +    u32 header_type; /* 0x4 for manifest type */
>>> +    u32 header_length; /* in dwords */
>>> +    u32 header_version;
>>> +    u32 flags;
>>> +    u32 vendor;
>>> +    u32 date;
>>> +    u32 size; /* In dwords, size of entire manifest (header + 
>>> extensions) */
>>> +    u32 header_id;
>>> +    u32 internal_data;
>>> +    struct intel_gsc_version fw_version;
>>> +    u32 security_version;
>>> +    struct intel_gsc_version meu_kit_version;
>>> +    u32 meu_manifest_version;
>>> +    u8 general_data[4];
>>> +    u8 reserved3[56];
>>> +    u32 modulus_size; /* in dwords */
>>> +    u32 exponent_size; /* in dwords */
>>> +} __packed;
>>> +
>>> +#endif
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>>> index 268e036f8f28..6d795438b3e4 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>>> @@ -6,23 +6,14 @@
>>>   #include <linux/types.h>
>>>     #include "gt/intel_gt.h"
>>> -#include "gt/intel_gt_print.h"
>>>   #include "intel_guc_reg.h"
>>>   #include "intel_huc.h"
>>> +#include "intel_huc_print.h"
>>>   #include "i915_drv.h"
>>>     #include <linux/device/bus.h>
>>>   #include <linux/mei_aux.h>
>>>   -#define huc_printk(_huc, _level, _fmt, ...) \
>>> -    gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
>>> -#define huc_err(_huc, _fmt, ...)    huc_printk((_huc), err, _fmt, 
>>> ##__VA_ARGS__)
>>> -#define huc_warn(_huc, _fmt, ...)    huc_printk((_huc), warn, _fmt, 
>>> ##__VA_ARGS__)
>>> -#define huc_notice(_huc, _fmt, ...)    huc_printk((_huc), notice, 
>>> _fmt, ##__VA_ARGS__)
>>> -#define huc_info(_huc, _fmt, ...)    huc_printk((_huc), info, _fmt, 
>>> ##__VA_ARGS__)
>>> -#define huc_dbg(_huc, _fmt, ...)    huc_printk((_huc), dbg, _fmt, 
>>> ##__VA_ARGS__)
>>> -#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), 
>>> probe_error, _fmt, ##__VA_ARGS__)
>>> -
>>>   /**
>>>    * DOC: HuC
>>>    *
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>>> index 534b0aa43316..037d2ad4879d 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>>> @@ -5,11 +5,146 @@
>>>     #include "gt/intel_gsc.h"
>>>   #include "gt/intel_gt.h"
>>> +#include "intel_gsc_binary_headers.h"
>>>   #include "intel_huc.h"
>>>   #include "intel_huc_fw.h"
>>> +#include "intel_huc_print.h"
>>>   #include "i915_drv.h"
>>>   #include "pxp/intel_pxp_huc.h"
>>>   +static void get_version_from_gsc_manifest(struct intel_uc_fw_ver 
>>> *ver, const void *data)
>>> +{
>>> +    const struct intel_gsc_manifest_header *manifest = data;
>>> +
>>> +    ver->major = manifest->fw_version.major;
>>> +    ver->minor = manifest->fw_version.minor;
>>> +    ver->patch = manifest->fw_version.hotfix;
>>> +}
>>> +
>>> +static bool css_valid(const void *data, size_t size)
>>> +{
>>> +    const struct uc_css_header *css = data;
>>> +
>>> +    if (unlikely(size < sizeof(struct uc_css_header)))
>>> +        return false;
>>> +
>>> +    if (css->module_type != 0x6)
>>> +        return false;
>>> +
>>> +    if (css->module_vendor != PCI_VENDOR_ID_INTEL)
>>> +        return false;
>>> +
>>> +    return true;
>>> +}
>>> +
>>> +static inline u32 entry_offset(const struct intel_gsc_cpd_entry 
>>> *entry)
>>> +{
>>> +    return entry->offset & INTEL_GSC_CPD_ENTRY_OFFSET_MASK;
>>> +}
>>> +
>>> +int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const 
>>> void *data, size_t size)
>>> +{
>>> +    struct intel_huc *huc = container_of(huc_fw, struct intel_huc, 
>>> fw);
>>> +    const struct intel_gsc_cpd_header_v2 *header = data;
>>> +    const struct intel_gsc_cpd_entry *entry;
>>> +    size_t min_size = sizeof(*header);
>>> +    int i;
>>> +
>>> +    if (!huc_fw->loaded_via_gsc) {
>>> +        huc_err(huc, "Invalid FW type GSC header parsing!\n");
>> I'm still not understanding what this error means. Is it meant to say 
>> 'Invalid FW type *for* GSC header parsing'?
>
> yes, sorry that was the idea. I'll add the "for"
:)

>
>>
>> 'fw->loaded_via_gsc' is set from the blob table, yes? And 
>> intel_huc_fw_get_binary_info() is only called from 
>> check_gsc_manifest() which is called from check_fw_header() iff 
>> fw->loaded_via_gsc is set. So it should be impossible for this test 
>> to ever fail, yes?
>
> Yes, this should never fail, but with GSC FW the code changes again so 
> I wanted to make sure I had a check in there in case I got things 
> wrong. There would be other failures anyway (because the parsing 
> wouldn't work, so I can drop this check if you think it is redundant.
Was more just wanting to verify my understanding and maybe wondering if 
it should be a BUG rather than just a if. But yeah, the code paths above 
change even within this patch set. So keeping the test as is seems sensible.

John.

>
> Daniele
>
>>
>> John.
>>
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if (size < sizeof(*header)) {
>>> +        huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
>>> +        return -ENODATA;
>>> +    }
>>> +
>>> +    /*
>>> +     * The GSC-enabled HuC binary starts with a directory header, 
>>> followed
>>> +     * by a series of entries. Each entry is identified by a name and
>>> +     * points to a specific section of the binary containing the 
>>> relevant
>>> +     * data. The entries we're interested in are:
>>> +     * - "HUCP.man": points to the GSC manifest header for the HuC, 
>>> which
>>> +     *               contains the version info.
>>> +     * - "huc_fw": points to the legacy-style binary that can be 
>>> used for
>>> +     *             load via the DMA. This entry only contains a 
>>> valid CSS
>>> +     *             on binaries for platforms that support 2-step 
>>> HuC load
>>> +     *             via dma and auth via GSC (like MTL).
>>> +     *
>>> +     * --------------------------------------------------
>>> +     * [  intel_gsc_cpd_header_v2                       ]
>>> +     * --------------------------------------------------
>>> +     * [  intel_gsc_cpd_entry[]                         ]
>>> +     * [      entry1                                    ]
>>> +     * [      ...                                       ]
>>> +     * [      entryX                                    ]
>>> +     * [          "HUCP.man"                            ]
>>> +     * [           ...                                  ]
>>> +     * [           offset >----------------------------]------o
>>> +     * [      ...                                       ] |
>>> +     * [      entryY                                    ] |
>>> +     * [          "huc_fw"                              ] |
>>> +     * [           ...                                  ] |
>>> +     * [           offset >----------------------------]----------o
>>> +     * -------------------------------------------------- | |
>>> +     * |   |
>>> +     * -------------------------------------------------- | |
>>> +     * [ intel_gsc_manifest_header ]<-----o   |
>>> +     * [  ... ]          |
>>> +     * [  intel_gsc_version fw_version ]          |
>>> +     * [  ... ]          |
>>> +     * --------------------------------------------------          |
>>> + * |
>>> +     * --------------------------------------------------          |
>>> +     * [ data[] ]<---------o
>>> +     * [  ...                                           ]
>>> +     * [  ...                                           ]
>>> +     * --------------------------------------------------
>>> +     */
>>> +
>>> +    if (header->header_marker != INTEL_GSC_CPD_HEADER_MARKER) {
>>> +        huc_err(huc, "invalid marker for CPD header: 0x%08x!\n",
>>> +            header->header_marker);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    /* we only have binaries with header v2 and entry v1 for now */
>>> +    if (header->header_version != 2 || header->entry_version != 1) {
>>> +        huc_err(huc, "invalid CPD header/entry version %u:%u!\n",
>>> +            header->header_version, header->entry_version);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if (header->header_length < sizeof(struct 
>>> intel_gsc_cpd_header_v2)) {
>>> +        huc_err(huc, "invalid CPD header length %u!\n",
>>> +            header->header_length);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    min_size = header->header_length + sizeof(*entry) * 
>>> header->num_of_entries;
>>> +    if (size < min_size) {
>>> +        huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
>>> +        return -ENODATA;
>>> +    }
>>> +
>>> +    entry = data + header->header_length;
>>> +
>>> +    for (i = 0; i < header->num_of_entries; i++, entry++) {
>>> +        if (strcmp(entry->name, "HUCP.man") == 0)
>>> + get_version_from_gsc_manifest(&huc_fw->file_selected.ver,
>>> +                              data + entry_offset(entry));
>>> +
>>> +        if (strcmp(entry->name, "huc_fw") == 0) {
>>> +            u32 offset = entry_offset(entry);
>>> +            if (offset < size && css_valid(data + offset, size - 
>>> offset))
>>> +                huc_fw->dma_start_offset = offset;
>>> +        }
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
>>>   {
>>>       int ret;
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
>>> index db42e238b45f..0999ffe6f962 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
>>> @@ -7,8 +7,11 @@
>>>   #define _INTEL_HUC_FW_H_
>>>     struct intel_huc;
>>> +struct intel_uc_fw;
>>> +
>>> +#include <linux/types.h>
>>>     int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc);
>>>   int intel_huc_fw_upload(struct intel_huc *huc);
>>> -
>>> +int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const 
>>> void *data, size_t size);
>>>   #endif
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
>>> new file mode 100644
>>> index 000000000000..915d310ee1df
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
>>> @@ -0,0 +1,21 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
>>> + */
>>> +
>>> +#ifndef __INTEL_HUC_PRINT__
>>> +#define __INTEL_HUC_PRINT__
>>> +
>>> +#include "gt/intel_gt.h"
>>> +#include "gt/intel_gt_print.h"
>>> +
>>> +#define huc_printk(_huc, _level, _fmt, ...) \
>>> +    gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
>>> +#define huc_err(_huc, _fmt, ...)    huc_printk((_huc), err, _fmt, 
>>> ##__VA_ARGS__)
>>> +#define huc_warn(_huc, _fmt, ...)    huc_printk((_huc), warn, _fmt, 
>>> ##__VA_ARGS__)
>>> +#define huc_notice(_huc, _fmt, ...)    huc_printk((_huc), notice, 
>>> _fmt, ##__VA_ARGS__)
>>> +#define huc_info(_huc, _fmt, ...)    huc_printk((_huc), info, _fmt, 
>>> ##__VA_ARGS__)
>>> +#define huc_dbg(_huc, _fmt, ...)    huc_printk((_huc), dbg, _fmt, 
>>> ##__VA_ARGS__)
>>> +#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), 
>>> probe_error, _fmt, ##__VA_ARGS__)
>>> +
>>> +#endif /* __INTEL_HUC_PRINT__ */
>>> 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 31776c279f32..9ced8dbf1253 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> @@ -548,33 +548,6 @@ static void __force_fw_fetch_failures(struct 
>>> intel_uc_fw *uc_fw, int e)
>>>       }
>>>   }
>>>   -static int check_gsc_manifest(struct intel_gt *gt,
>>> -                  const struct firmware *fw,
>>> -                  struct intel_uc_fw *uc_fw)
>>> -{
>>> -    u32 *dw = (u32 *)fw->data;
>>> -    u32 version_hi, version_lo;
>>> -    size_t min_size;
>>> -
>>> -    /* Check the size of the blob before examining buffer contents */
>>> -    min_size = sizeof(u32) * (HUC_GSC_VERSION_LO_DW + 1);
>>> -    if (unlikely(fw->size < min_size)) {
>>> -        gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>>> -            intel_uc_fw_type_repr(uc_fw->type), 
>>> uc_fw->file_selected.path,
>>> -            fw->size, min_size);
>>> -        return -ENODATA;
>>> -    }
>>> -
>>> -    version_hi = dw[HUC_GSC_VERSION_HI_DW];
>>> -    version_lo = dw[HUC_GSC_VERSION_LO_DW];
>>> -
>>> -    uc_fw->file_selected.ver.major = 
>>> FIELD_GET(HUC_GSC_MAJOR_VER_HI_MASK, version_hi);
>>> -    uc_fw->file_selected.ver.minor = 
>>> FIELD_GET(HUC_GSC_MINOR_VER_HI_MASK, version_hi);
>>> -    uc_fw->file_selected.ver.patch = 
>>> FIELD_GET(HUC_GSC_PATCH_VER_LO_MASK, version_lo);
>>> -
>>> -    return 0;
>>> -}
>>> -
>>>   static void uc_unpack_css_version(struct intel_uc_fw_ver *ver, u32 
>>> css_value)
>>>   {
>>>       /* Get version numbers from the CSS header */
>>> @@ -631,22 +604,22 @@ static void guc_read_css_info(struct 
>>> intel_uc_fw *uc_fw, struct uc_css_header *c
>>>       uc_fw->private_data_size = css->private_data_size;
>>>   }
>>>   -static int check_ccs_header(struct intel_gt *gt,
>>> -                const struct firmware *fw,
>>> -                struct intel_uc_fw *uc_fw)
>>> +static int __check_ccs_header(struct intel_gt *gt,
>>> +                  const void *fw_data, size_t fw_size,
>>> +                  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))) {
>>> +    if (unlikely(fw_size < sizeof(struct uc_css_header))) {
>>>           gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>>>               intel_uc_fw_type_repr(uc_fw->type), 
>>> uc_fw->file_selected.path,
>>> -            fw->size, sizeof(struct uc_css_header));
>>> +            fw_size, sizeof(struct uc_css_header));
>>>           return -ENODATA;
>>>       }
>>>   -    css = (struct uc_css_header *)fw->data;
>>> +    css = (struct uc_css_header *)fw_data;
>>>         /* Check integrity of size values inside CSS header */
>>>       size = (css->header_size_dw - css->key_size_dw - 
>>> css->modulus_size_dw -
>>> @@ -654,7 +627,7 @@ static int check_ccs_header(struct intel_gt *gt,
>>>       if (unlikely(size != sizeof(struct uc_css_header))) {
>>>           gt_warn(gt, "%s firmware %s: unexpected header size: %zu 
>>> != %zu\n",
>>>               intel_uc_fw_type_repr(uc_fw->type), 
>>> uc_fw->file_selected.path,
>>> -            fw->size, sizeof(struct uc_css_header));
>>> +            fw_size, sizeof(struct uc_css_header));
>>>           return -EPROTO;
>>>       }
>>>   @@ -666,10 +639,10 @@ static int check_ccs_header(struct intel_gt 
>>> *gt,
>>>         /* At least, it should have header, uCode and RSA. Size of 
>>> all three. */
>>>       size = sizeof(struct uc_css_header) + uc_fw->ucode_size + 
>>> uc_fw->rsa_size;
>>> -    if (unlikely(fw->size < size)) {
>>> +    if (unlikely(fw_size < size)) {
>>>           gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
>>>               intel_uc_fw_type_repr(uc_fw->type), 
>>> uc_fw->file_selected.path,
>>> -            fw->size, size);
>>> +            fw_size, size);
>>>           return -ENOEXEC;
>>>       }
>>>   @@ -690,6 +663,32 @@ static int check_ccs_header(struct intel_gt *gt,
>>>       return 0;
>>>   }
>>>   +static int check_gsc_manifest(struct intel_gt *gt,
>>> +                  const struct firmware *fw,
>>> +                  struct intel_uc_fw *uc_fw)
>>> +{
>>> +    if (uc_fw->type != INTEL_UC_FW_TYPE_HUC) {
>>> +        gt_err(gt, "trying to GSC-parse a non-HuC binary");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    intel_huc_fw_get_binary_info(uc_fw, fw->data, fw->size);
>>> +
>>> +    if (uc_fw->dma_start_offset) {
>>> +        u32 delta = uc_fw->dma_start_offset;
>>> +        __check_ccs_header(gt, fw->data + delta, fw->size - delta, 
>>> uc_fw);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int check_ccs_header(struct intel_gt *gt,
>>> +                const struct firmware *fw,
>>> +                struct intel_uc_fw *uc_fw)
>>> +{
>>> +    return __check_ccs_header(gt, fw->data, fw->size, uc_fw);
>>> +}
>>> +
>>>   static bool is_ver_8bit(struct intel_uc_fw_ver *ver)
>>>   {
>>>       return ver->major < 0xFF && ver->minor < 0xFF && ver->patch < 
>>> 0xFF;
>>> 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 2be9470eb712..b3daba9526eb 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>>> @@ -118,6 +118,8 @@ struct intel_uc_fw {
>>>       u32 ucode_size;
>>>       u32 private_data_size;
>>>   +    u32 dma_start_offset;
>>> +
>>>       bool loaded_via_gsc;
>>>   };
>>>   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 646fa8aa6cf1..7fe405126249 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
>>> @@ -84,10 +84,4 @@ struct uc_css_header {
>>>   } __packed;
>>>   static_assert(sizeof(struct uc_css_header) == 128);
>>>   -#define HUC_GSC_VERSION_HI_DW        44
>>> -#define   HUC_GSC_MAJOR_VER_HI_MASK    (0xFF << 0)
>>> -#define   HUC_GSC_MINOR_VER_HI_MASK    (0xFF << 16)
>>> -#define HUC_GSC_VERSION_LO_DW        45
>>> -#define   HUC_GSC_PATCH_VER_LO_MASK    (0xFF << 0)
>>> -
>>>   #endif /* _INTEL_UC_FW_ABI_H */
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
new file mode 100644
index 000000000000..714f0c256118
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_binary_headers.h
@@ -0,0 +1,74 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef _INTEL_GSC_BINARY_HEADERS_H_
+#define _INTEL_GSC_BINARY_HEADERS_H_
+
+#include <linux/types.h>
+
+/* Code partition directory (CPD) structures */
+struct intel_gsc_cpd_header_v2 {
+	u32 header_marker;
+#define INTEL_GSC_CPD_HEADER_MARKER 0x44504324
+
+	u32 num_of_entries;
+	u8 header_version;
+	u8 entry_version;
+	u8 header_length; /* in bytes */
+	u8 flags;
+	u32 partition_name;
+	u32 crc32;
+} __packed;
+
+struct intel_gsc_cpd_entry {
+	u8 name[12];
+
+	/*
+	 * Bits 0-24: offset from the beginning of the code partition
+	 * Bit 25: huffman compressed
+	 * Bits 26-31: reserved
+	 */
+	u32 offset;
+#define INTEL_GSC_CPD_ENTRY_OFFSET_MASK GENMASK(24, 0)
+#define INTEL_GSC_CPD_ENTRY_HUFFMAN_COMP BIT(25)
+
+	/*
+	 * Module/Item length, in bytes. For Huffman-compressed modules, this
+	 * refers to the uncompressed size. For software-compressed modules,
+	 * this refers to the compressed size.
+	 */
+	u32 length;
+
+	u8 reserved[4];
+} __packed;
+
+struct intel_gsc_version {
+	u16 major;
+	u16 minor;
+	u16 hotfix;
+	u16 build;
+} __packed;
+
+struct intel_gsc_manifest_header {
+	u32 header_type; /* 0x4 for manifest type */
+	u32 header_length; /* in dwords */
+	u32 header_version;
+	u32 flags;
+	u32 vendor;
+	u32 date;
+	u32 size; /* In dwords, size of entire manifest (header + extensions) */
+	u32 header_id;
+	u32 internal_data;
+	struct intel_gsc_version fw_version;
+	u32 security_version;
+	struct intel_gsc_version meu_kit_version;
+	u32 meu_manifest_version;
+	u8 general_data[4];
+	u8 reserved3[56];
+	u32 modulus_size; /* in dwords */
+	u32 exponent_size; /* in dwords */
+} __packed;
+
+#endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 268e036f8f28..6d795438b3e4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -6,23 +6,14 @@ 
 #include <linux/types.h>
 
 #include "gt/intel_gt.h"
-#include "gt/intel_gt_print.h"
 #include "intel_guc_reg.h"
 #include "intel_huc.h"
+#include "intel_huc_print.h"
 #include "i915_drv.h"
 
 #include <linux/device/bus.h>
 #include <linux/mei_aux.h>
 
-#define huc_printk(_huc, _level, _fmt, ...) \
-	gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
-#define huc_err(_huc, _fmt, ...)	huc_printk((_huc), err, _fmt, ##__VA_ARGS__)
-#define huc_warn(_huc, _fmt, ...)	huc_printk((_huc), warn, _fmt, ##__VA_ARGS__)
-#define huc_notice(_huc, _fmt, ...)	huc_printk((_huc), notice, _fmt, ##__VA_ARGS__)
-#define huc_info(_huc, _fmt, ...)	huc_printk((_huc), info, _fmt, ##__VA_ARGS__)
-#define huc_dbg(_huc, _fmt, ...)	huc_printk((_huc), dbg, _fmt, ##__VA_ARGS__)
-#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), probe_error, _fmt, ##__VA_ARGS__)
-
 /**
  * DOC: HuC
  *
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 534b0aa43316..037d2ad4879d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -5,11 +5,146 @@ 
 
 #include "gt/intel_gsc.h"
 #include "gt/intel_gt.h"
+#include "intel_gsc_binary_headers.h"
 #include "intel_huc.h"
 #include "intel_huc_fw.h"
+#include "intel_huc_print.h"
 #include "i915_drv.h"
 #include "pxp/intel_pxp_huc.h"
 
+static void get_version_from_gsc_manifest(struct intel_uc_fw_ver *ver, const void *data)
+{
+	const struct intel_gsc_manifest_header *manifest = data;
+
+	ver->major = manifest->fw_version.major;
+	ver->minor = manifest->fw_version.minor;
+	ver->patch = manifest->fw_version.hotfix;
+}
+
+static bool css_valid(const void *data, size_t size)
+{
+	const struct uc_css_header *css = data;
+
+	if (unlikely(size < sizeof(struct uc_css_header)))
+		return false;
+
+	if (css->module_type != 0x6)
+		return false;
+
+	if (css->module_vendor != PCI_VENDOR_ID_INTEL)
+		return false;
+
+	return true;
+}
+
+static inline u32 entry_offset(const struct intel_gsc_cpd_entry *entry)
+{
+	return entry->offset & INTEL_GSC_CPD_ENTRY_OFFSET_MASK;
+}
+
+int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const void *data, size_t size)
+{
+	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
+	const struct intel_gsc_cpd_header_v2 *header = data;
+	const struct intel_gsc_cpd_entry *entry;
+	size_t min_size = sizeof(*header);
+	int i;
+
+	if (!huc_fw->loaded_via_gsc) {
+		huc_err(huc, "Invalid FW type GSC header parsing!\n");
+		return -EINVAL;
+	}
+
+	if (size < sizeof(*header)) {
+		huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
+		return -ENODATA;
+	}
+
+	/*
+	 * The GSC-enabled HuC binary starts with a directory header, followed
+	 * by a series of entries. Each entry is identified by a name and
+	 * points to a specific section of the binary containing the relevant
+	 * data. The entries we're interested in are:
+	 * - "HUCP.man": points to the GSC manifest header for the HuC, which
+	 *               contains the version info.
+	 * - "huc_fw": points to the legacy-style binary that can be used for
+	 *             load via the DMA. This entry only contains a valid CSS
+	 *             on binaries for platforms that support 2-step HuC load
+	 *             via dma and auth via GSC (like MTL).
+	 *
+	 * --------------------------------------------------
+	 * [  intel_gsc_cpd_header_v2                       ]
+	 * --------------------------------------------------
+	 * [  intel_gsc_cpd_entry[]                         ]
+	 * [      entry1                                    ]
+	 * [      ...                                       ]
+	 * [      entryX                                    ]
+	 * [          "HUCP.man"                            ]
+	 * [           ...                                  ]
+	 * [           offset  >----------------------------]------o
+	 * [      ...                                       ]      |
+	 * [      entryY                                    ]      |
+	 * [          "huc_fw"                              ]      |
+	 * [           ...                                  ]      |
+	 * [           offset  >----------------------------]----------o
+	 * --------------------------------------------------      |   |
+	 *                                                         |   |
+	 * --------------------------------------------------      |   |
+	 * [ intel_gsc_manifest_header                      ]<-----o   |
+	 * [  ...                                           ]          |
+	 * [  intel_gsc_version fw_version                  ]          |
+	 * [  ...                                           ]          |
+	 * --------------------------------------------------          |
+	 *                                                             |
+	 * --------------------------------------------------          |
+	 * [ data[]                                         ]<---------o
+	 * [  ...                                           ]
+	 * [  ...                                           ]
+	 * --------------------------------------------------
+	 */
+
+	if (header->header_marker != INTEL_GSC_CPD_HEADER_MARKER) {
+		huc_err(huc, "invalid marker for CPD header: 0x%08x!\n",
+			header->header_marker);
+		return -EINVAL;
+	}
+
+	/* we only have binaries with header v2 and entry v1 for now */
+	if (header->header_version != 2 || header->entry_version != 1) {
+		huc_err(huc, "invalid CPD header/entry version %u:%u!\n",
+			header->header_version, header->entry_version);
+		return -EINVAL;
+	}
+
+	if (header->header_length < sizeof(struct intel_gsc_cpd_header_v2)) {
+		huc_err(huc, "invalid CPD header length %u!\n",
+			header->header_length);
+		return -EINVAL;
+	}
+
+	min_size = header->header_length + sizeof(*entry) * header->num_of_entries;
+	if (size < min_size) {
+		huc_err(huc, "FW too small! %zu < %zu\n", size, min_size);
+		return -ENODATA;
+	}
+
+	entry = data + header->header_length;
+
+	for (i = 0; i < header->num_of_entries; i++, entry++) {
+		if (strcmp(entry->name, "HUCP.man") == 0)
+			get_version_from_gsc_manifest(&huc_fw->file_selected.ver,
+						      data + entry_offset(entry));
+
+		if (strcmp(entry->name, "huc_fw") == 0) {
+			u32 offset = entry_offset(entry);
+			if (offset < size && css_valid(data + offset, size - offset))
+				huc_fw->dma_start_offset = offset;
+		}
+	}
+
+	return 0;
+}
+
 int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
 {
 	int ret;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
index db42e238b45f..0999ffe6f962 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
@@ -7,8 +7,11 @@ 
 #define _INTEL_HUC_FW_H_
 
 struct intel_huc;
+struct intel_uc_fw;
+
+#include <linux/types.h>
 
 int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc);
 int intel_huc_fw_upload(struct intel_huc *huc);
-
+int intel_huc_fw_get_binary_info(struct intel_uc_fw *huc_fw, const void *data, size_t size);
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
new file mode 100644
index 000000000000..915d310ee1df
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_print.h
@@ -0,0 +1,21 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef __INTEL_HUC_PRINT__
+#define __INTEL_HUC_PRINT__
+
+#include "gt/intel_gt.h"
+#include "gt/intel_gt_print.h"
+
+#define huc_printk(_huc, _level, _fmt, ...) \
+	gt_##_level(huc_to_gt(_huc), "HuC: " _fmt, ##__VA_ARGS__)
+#define huc_err(_huc, _fmt, ...)	huc_printk((_huc), err, _fmt, ##__VA_ARGS__)
+#define huc_warn(_huc, _fmt, ...)	huc_printk((_huc), warn, _fmt, ##__VA_ARGS__)
+#define huc_notice(_huc, _fmt, ...)	huc_printk((_huc), notice, _fmt, ##__VA_ARGS__)
+#define huc_info(_huc, _fmt, ...)	huc_printk((_huc), info, _fmt, ##__VA_ARGS__)
+#define huc_dbg(_huc, _fmt, ...)	huc_printk((_huc), dbg, _fmt, ##__VA_ARGS__)
+#define huc_probe_error(_huc, _fmt, ...) huc_printk((_huc), probe_error, _fmt, ##__VA_ARGS__)
+
+#endif /* __INTEL_HUC_PRINT__ */
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 31776c279f32..9ced8dbf1253 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -548,33 +548,6 @@  static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
 	}
 }
 
-static int check_gsc_manifest(struct intel_gt *gt,
-			      const struct firmware *fw,
-			      struct intel_uc_fw *uc_fw)
-{
-	u32 *dw = (u32 *)fw->data;
-	u32 version_hi, version_lo;
-	size_t min_size;
-
-	/* Check the size of the blob before examining buffer contents */
-	min_size = sizeof(u32) * (HUC_GSC_VERSION_LO_DW + 1);
-	if (unlikely(fw->size < min_size)) {
-		gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
-			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
-			fw->size, min_size);
-		return -ENODATA;
-	}
-
-	version_hi = dw[HUC_GSC_VERSION_HI_DW];
-	version_lo = dw[HUC_GSC_VERSION_LO_DW];
-
-	uc_fw->file_selected.ver.major = FIELD_GET(HUC_GSC_MAJOR_VER_HI_MASK, version_hi);
-	uc_fw->file_selected.ver.minor = FIELD_GET(HUC_GSC_MINOR_VER_HI_MASK, version_hi);
-	uc_fw->file_selected.ver.patch = FIELD_GET(HUC_GSC_PATCH_VER_LO_MASK, version_lo);
-
-	return 0;
-}
-
 static void uc_unpack_css_version(struct intel_uc_fw_ver *ver, u32 css_value)
 {
 	/* Get version numbers from the CSS header */
@@ -631,22 +604,22 @@  static void guc_read_css_info(struct intel_uc_fw *uc_fw, struct uc_css_header *c
 	uc_fw->private_data_size = css->private_data_size;
 }
 
-static int check_ccs_header(struct intel_gt *gt,
-			    const struct firmware *fw,
-			    struct intel_uc_fw *uc_fw)
+static int __check_ccs_header(struct intel_gt *gt,
+			      const void *fw_data, size_t fw_size,
+			      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))) {
+	if (unlikely(fw_size < sizeof(struct uc_css_header))) {
 		gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
 			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
-			fw->size, sizeof(struct uc_css_header));
+			fw_size, sizeof(struct uc_css_header));
 		return -ENODATA;
 	}
 
-	css = (struct uc_css_header *)fw->data;
+	css = (struct uc_css_header *)fw_data;
 
 	/* Check integrity of size values inside CSS header */
 	size = (css->header_size_dw - css->key_size_dw - css->modulus_size_dw -
@@ -654,7 +627,7 @@  static int check_ccs_header(struct intel_gt *gt,
 	if (unlikely(size != sizeof(struct uc_css_header))) {
 		gt_warn(gt, "%s firmware %s: unexpected header size: %zu != %zu\n",
 			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
-			fw->size, sizeof(struct uc_css_header));
+			fw_size, sizeof(struct uc_css_header));
 		return -EPROTO;
 	}
 
@@ -666,10 +639,10 @@  static int check_ccs_header(struct intel_gt *gt,
 
 	/* At least, it should have header, uCode and RSA. Size of all three. */
 	size = sizeof(struct uc_css_header) + uc_fw->ucode_size + uc_fw->rsa_size;
-	if (unlikely(fw->size < size)) {
+	if (unlikely(fw_size < size)) {
 		gt_warn(gt, "%s firmware %s: invalid size: %zu < %zu\n",
 			intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
-			fw->size, size);
+			fw_size, size);
 		return -ENOEXEC;
 	}
 
@@ -690,6 +663,32 @@  static int check_ccs_header(struct intel_gt *gt,
 	return 0;
 }
 
+static int check_gsc_manifest(struct intel_gt *gt,
+			      const struct firmware *fw,
+			      struct intel_uc_fw *uc_fw)
+{
+	if (uc_fw->type != INTEL_UC_FW_TYPE_HUC) {
+		gt_err(gt, "trying to GSC-parse a non-HuC binary");
+		return -EINVAL;
+	}
+
+	intel_huc_fw_get_binary_info(uc_fw, fw->data, fw->size);
+
+	if (uc_fw->dma_start_offset) {
+		u32 delta = uc_fw->dma_start_offset;
+		__check_ccs_header(gt, fw->data + delta, fw->size - delta, uc_fw);
+	}
+
+	return 0;
+}
+
+static int check_ccs_header(struct intel_gt *gt,
+			    const struct firmware *fw,
+			    struct intel_uc_fw *uc_fw)
+{
+	return __check_ccs_header(gt, fw->data, fw->size, uc_fw);
+}
+
 static bool is_ver_8bit(struct intel_uc_fw_ver *ver)
 {
 	return ver->major < 0xFF && ver->minor < 0xFF && ver->patch < 0xFF;
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 2be9470eb712..b3daba9526eb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -118,6 +118,8 @@  struct intel_uc_fw {
 	u32 ucode_size;
 	u32 private_data_size;
 
+	u32 dma_start_offset;
+
 	bool loaded_via_gsc;
 };
 
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 646fa8aa6cf1..7fe405126249 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
@@ -84,10 +84,4 @@  struct uc_css_header {
 } __packed;
 static_assert(sizeof(struct uc_css_header) == 128);
 
-#define HUC_GSC_VERSION_HI_DW		44
-#define   HUC_GSC_MAJOR_VER_HI_MASK	(0xFF << 0)
-#define   HUC_GSC_MINOR_VER_HI_MASK	(0xFF << 16)
-#define HUC_GSC_VERSION_LO_DW		45
-#define   HUC_GSC_PATCH_VER_LO_MASK	(0xFF << 0)
-
 #endif /* _INTEL_UC_FW_ABI_H */