All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Harrison <john.c.harrison@intel.com>
To: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>,
	<intel-gfx@lists.freedesktop.org>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3 2/7] drm/i915/huc: Parse the GSC-enabled HuC binary
Date: Tue, 30 May 2023 17:11:36 -0700	[thread overview]
Message-ID: <ebce68b1-5b91-63ee-9844-df86b3d71344@intel.com> (raw)
In-Reply-To: <ed576336-7b56-efa2-5355-f49001995182@intel.com>

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 */
>>
>


WARNING: multiple messages have this Message-ID (diff)
From: John Harrison <john.c.harrison@intel.com>
To: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>,
	<intel-gfx@lists.freedesktop.org>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 2/7] drm/i915/huc: Parse the GSC-enabled HuC binary
Date: Tue, 30 May 2023 17:11:36 -0700	[thread overview]
Message-ID: <ebce68b1-5b91-63ee-9844-df86b3d71344@intel.com> (raw)
In-Reply-To: <ed576336-7b56-efa2-5355-f49001995182@intel.com>

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 */
>>
>


  reply	other threads:[~2023-05-31  0:11 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-27  0:52 [PATCH v3 0/7] drm/i915: HuC loading and authentication for MTL Daniele Ceraolo Spurio
2023-05-27  0:52 ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-27  0:52 ` [PATCH v3 1/7] drm/i915/uc: perma-pin firmwares Daniele Ceraolo Spurio
2023-05-27  0:52   ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-30 22:52   ` John Harrison
2023-05-30 22:52     ` [Intel-gfx] " John Harrison
2023-05-27  0:52 ` [PATCH v3 2/7] drm/i915/huc: Parse the GSC-enabled HuC binary Daniele Ceraolo Spurio
2023-05-27  0:52   ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-30 23:30   ` John Harrison
2023-05-30 23:30     ` [Intel-gfx] " John Harrison
2023-05-31  0:06     ` Ceraolo Spurio, Daniele
2023-05-31  0:06       ` [Intel-gfx] " Ceraolo Spurio, Daniele
2023-05-31  0:11       ` John Harrison [this message]
2023-05-31  0:11         ` John Harrison
2023-05-27  0:52 ` [PATCH v3 3/7] drm/i915/huc: Load GSC-enabled HuC via DMA xfer if the fuse says so Daniele Ceraolo Spurio
2023-05-27  0:52   ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-30 23:51   ` John Harrison
2023-05-30 23:51     ` [Intel-gfx] " John Harrison
2023-05-31  0:11     ` Ceraolo Spurio, Daniele
2023-05-31  0:11       ` [Intel-gfx] " Ceraolo Spurio, Daniele
2023-05-31  0:20       ` John Harrison
2023-05-31  0:20         ` [Intel-gfx] " John Harrison
2023-05-31  0:26         ` Ceraolo Spurio, Daniele
2023-05-31  0:26           ` [Intel-gfx] " Ceraolo Spurio, Daniele
2023-05-27  0:52 ` [PATCH v3 4/7] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow Daniele Ceraolo Spurio
2023-05-27  0:52   ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-27  5:07   ` kernel test robot
2023-05-27  8:59   ` kernel test robot
2023-05-30 15:29   ` [PATCH v4] " Daniele Ceraolo Spurio
2023-05-30 15:29     ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-31  0:01     ` John Harrison
2023-05-31  0:01       ` [Intel-gfx] " John Harrison
2023-05-31  0:14       ` Ceraolo Spurio, Daniele
2023-05-31  0:14         ` [Intel-gfx] " Ceraolo Spurio, Daniele
2023-05-31  0:24         ` John Harrison
2023-05-31  0:24           ` [Intel-gfx] " John Harrison
2023-05-31  0:53   ` [PATCH v3 4/7] " kernel test robot
2023-05-27  0:52 ` [PATCH v3 5/7] drm/i915/mtl/huc: auth HuC via GSC Daniele Ceraolo Spurio
2023-05-27  0:52   ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-31  0:33   ` Teres Alexis, Alan Previn
2023-05-31  0:33     ` [Intel-gfx] " Teres Alexis, Alan Previn
2023-05-27  0:52 ` [PATCH v3 6/7] drm/i915/mtl/huc: Use the media gt for the HuC getparam Daniele Ceraolo Spurio
2023-05-27  0:52   ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-27  0:52 ` [PATCH v3 7/7] drm/i915/huc: define HuC FW version for MTL Daniele Ceraolo Spurio
2023-05-27  0:52   ` [Intel-gfx] " Daniele Ceraolo Spurio
2023-05-27  1:22 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: HuC loading and authentication for MTL (rev5) Patchwork
2023-05-27  1:22 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-05-27  1:35 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-05-28  1:07 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-05-31  9:20 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: HuC loading and authentication for MTL (rev6) Patchwork
2023-05-31  9:20 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-05-31  9:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ebce68b1-5b91-63ee-9844-df86b3d71344@intel.com \
    --to=john.c.harrison@intel.com \
    --cc=alan.previn.teres.alexis@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.