[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <af6e1d5b-84e2-4ad7-b7a4-9c3ba3bf00f5@intel.com>
Date: Thu, 19 Jun 2025 15:02:54 +0530
From: "Nilawar, Badal" <badal.nilawar@...el.com>
To: Daniele Ceraolo Spurio <daniele.ceraolospurio@...el.com>,
<intel-xe@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>
CC: <anshuman.gupta@...el.com>, <rodrigo.vivi@...el.com>,
<alexander.usyskin@...el.com>, <gregkh@...uxfoundation.org>, <jgg@...dia.com>
Subject: Re: [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print
version info
On 19-06-2025 03:26, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>> Extract and print version info of the late binding binary.
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@...el.com>
>> ---
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 132 ++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 +
>> drivers/gpu/drm/xe/xe_uc_fw_abi.h | 69 +++++++++++
>> 3 files changed, 203 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> index 001e526e569a..f71d5825ac5b 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -45,6 +45,129 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
>> return container_of(late_bind, struct xe_device, late_bind);
>> }
>> +/* Refer to the "Late Bind based Firmware Layout" documentation
>> entry for details */
>> +static int parse_cpd_header(struct xe_late_bind *late_bind, u32 fw_id,
>> + const void *data, size_t size, const char
>> *manifest_entry)
>
> We'll need to try and make this common between the uc_fw code and this
> code to reduce duplication, but we can do that as a follow up.
I agree, we should do this as follow up.
>
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + const struct gsc_cpd_header_v2 *header = data;
>> + const struct gsc_manifest_header *manifest;
>> + const struct gsc_cpd_entry *entry;
>> + size_t min_size = sizeof(*header);
>> + struct xe_late_bind_fw *lb_fw;
>> + u32 offset;
>> + int i;
>> +
>> + if (fw_id >= MAX_FW_ID)
>> + return -EINVAL;
>> + lb_fw = &late_bind->late_bind_fw[fw_id];
>> +
>> + /* manifest_entry is mandatory */
>> + xe_assert(xe, manifest_entry);
>> +
>> + if (size < min_size || header->header_marker !=
>> GSC_CPD_HEADER_MARKER)
>> + return -ENOENT;
>> +
>> + if (header->header_length < sizeof(struct gsc_cpd_header_v2)) {
>> + drm_err(&xe->drm, "%s late binding fw: Invalid CPD header
>> length %u!\n",
>> + fw_id_to_name[lb_fw->id], header->header_length);
>> + return -EINVAL;
>> + }
>> +
>> + min_size = header->header_length + sizeof(struct gsc_cpd_entry)
>> * header->num_of_entries;
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + /* Look for the manifest first */
>> + entry = (void *)header + header->header_length;
>> + for (i = 0; i < header->num_of_entries; i++, entry++)
>> + if (strcmp(entry->name, manifest_entry) == 0)
>> + offset = entry->offset & GSC_CPD_ENTRY_OFFSET_MASK;
>> +
>> + if (!offset) {
>> + drm_err(&xe->drm, "%s late binding fw: Failed to find
>> manifest_entry\n",
>> + fw_id_to_name[lb_fw->id]);
>> + return -ENODATA;
>> + }
>> +
>> + min_size = offset + sizeof(struct gsc_manifest_header);
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + manifest = data + offset;
>> +
>> + lb_fw->version.major = manifest->fw_version.major;
>> + lb_fw->version.minor = manifest->fw_version.minor;
>> + lb_fw->version.hotfix = manifest->fw_version.hotfix;
>> + lb_fw->version.build = manifest->fw_version.build;
>
> not: here you can just do:
>
> lb_fw->version = manifest->fw_version;
>
> since both variables are of type struct gsc_version.
Ok
>
>> +
>> + return 0;
>> +}
>> +
>> +/* Refer to the "Late Bind based Firmware Layout" documentation
>> entry for details */
>> +static int parse_lb_layout(struct xe_late_bind *late_bind, u32 fw_id,
>
> IMO it'd be cleaner to just pass xe and xe_late_bind_fw, instead of
> xe_late_bind and fw_id.
> You should also be able to do a lb_fw_to_xe() call if you want with
> something like:
>
> container_of(lb_fw, struct xe_device, late_bind.late_bind_fw[lb_fw->id])
Sure.
>
>> + const void *data, size_t size, const char *fpt_entry)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + const struct csc_fpt_header *header = data;
>> + const struct csc_fpt_entry *entry;
>> + size_t min_size = sizeof(*header);
>> + struct xe_late_bind_fw *lb_fw;
>> + u32 offset;
>> + int i;
>> +
>> + if (fw_id >= MAX_FW_ID)
>> + return -EINVAL;
>> +
>> + lb_fw = &late_bind->late_bind_fw[fw_id];
>> +
>> + /* fpt_entry is mandatory */
>> + xe_assert(xe, fpt_entry);
>> +
>> + if (size < min_size || header->header_marker !=
>> CSC_FPT_HEADER_MARKER)
>> + return -ENOENT;
>> +
>> + if (header->header_length < sizeof(struct csc_fpt_header)) {
>> + drm_err(&xe->drm, "%s late binding fw: Invalid FPT header
>> length %u!\n",
>> + fw_id_to_name[lb_fw->id], header->header_length);
>> + return -EINVAL;
>> + }
>> +
>> + min_size = header->header_length + sizeof(struct csc_fpt_entry)
>> * header->num_of_entries;
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + /* Look for the manifest first */
>
> Here you're looking for the cpd header, not the manifest.
Ok.
>
>> + entry = (void *)header + header->header_length;
>> + for (i = 0; i < header->num_of_entries; i++, entry++)
>> + if (strcmp(entry->name, fpt_entry) == 0)
>> + offset = entry->offset;
>> +
>> + if (!offset) {
>> + drm_err(&xe->drm, "%s late binding fw: Failed to find
>> fpt_entry\n",
>> + fw_id_to_name[lb_fw->id]);
>> + return -ENODATA;
>> + }
>> +
>> + min_size = offset + sizeof(struct gsc_cpd_header_v2);
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + return parse_cpd_header(late_bind, fw_id, data + offset, size -
>> offset, "LTES.man");
>> +}
>> +
>> static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
>> {
>> struct xe_device *xe = late_bind_to_xe(late_bind);
>> @@ -185,8 +308,15 @@ static int __xe_late_bind_fw_init(struct
>> xe_late_bind *late_bind, u32 fw_id)
>> return -ENODATA;
>> }
>> - lb_fw->payload_size = fw->size;
>> + ret = parse_lb_layout(late_bind, fw_id, fw->data, fw->size,
>> "LTES");
>> + if (ret)
>> + return ret;
>> +
>> + drm_info(&xe->drm, "Using %s firmware from %s version %d.%d.%d\n",
>> + fw_id_to_name[lb_fw->id], lb_fw->blob_path,
>> + lb_fw->version.major, lb_fw->version.minor,
>> lb_fw->version.hotfix);
>
> You need to log the build number as well, as that needs to be relevant
> for this type of headers (we do log it for GSC for example).
I will log build number too.
>
>> + lb_fw->payload_size = fw->size;
>> memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
>> release_firmware(fw);
>> INIT_WORK(&lb_fw->work, late_bind_work);
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> index f79f0c0b2c4a..3fc4f350c81f 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> @@ -10,6 +10,7 @@
>> #include <linux/mutex.h>
>> #include <linux/types.h>
>> #include <linux/workqueue.h>
>> +#include "xe_uc_fw_abi.h"
>> #define MAX_PAYLOAD_SIZE (1024 * 4)
>> @@ -41,6 +42,8 @@ struct xe_late_bind_fw {
>> size_t payload_size;
>> /** @late_bind_fw.work: worker to upload latebind blob */
>> struct work_struct work;
>> + /** @late_bind_fw.version: late binding blob manifest version */
>> + struct gsc_version version;
>> };
>> /**
>> diff --git a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> index 87ade41209d0..13da2ca96817 100644
>> --- a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> +++ b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> @@ -318,4 +318,73 @@ struct gsc_manifest_header {
>> u32 exponent_size; /* in dwords */
>> } __packed;
>> +/**
>> + * DOC: Late binding Firmware Layout
>> + *
>> + * The Late binding binary starts with FPT header, which contains
>> locations
>> + * of various partitions of the binary. Here we're interested in
>> finding out
>> + * manifest version. To the manifest version, we need to locate CPD
>> header
>> + * one of the entry in CPD header points to manifest header.
>> Manifest header
>> + * contains the version.
>> + *
>> + * +================================================+
>> + * | FPT Header |
>> + * +================================================+
>> + * | FPT entries[] |
>> + * | entry1 |
>> + * | ... |
>> + * | entryX |
>> + * | "LTES" |
>> + * | ... |
>> + * | offset >-----------------------------|------o
>> + * +================================================+ |
>> + * |
>> + * +================================================+ |
>> + * | CPD Header |<-----o
>> + * +================================================+
>> + * | CPD entries[] |
>> + * | entry1 |
>> + * | ... |
>> + * | entryX |
>> + * | "LTES.man" |
>> + * | ... |
>> + * | offset >----------------------------|------o
>> + * +================================================+ |
>> + * |
>> + * +================================================+ |
>> + * | Manifest Header |<-----o
>> + * | ... |
>> + * | FW version |
>> + * | ... |
>> + * +================================================+
>> + */
>> +
>> +/* FPT Headers */
>> +struct csc_fpt_header {
>> + u32 header_marker;
>> +#define CSC_FPT_HEADER_MARKER 0x54504624
>> + u32 num_of_entries;
>> + u8 header_version;
>> + u8 entry_version;
>> + u8 header_length; /* in bytes */
>> + u8 flags;
>> + u16 ticks_to_add;
>> + u16 tokens_to_add;
>> + u32 uma_size;
>> + u32 crc32;
>> + u16 fitc_major;
>> + u16 fitc_minor;
>> + u16 fitc_hotfix;
>> + u16 fitc_build;
>
> For other headers we grouped the version values in a gsc_version
> struct. So here instead of the 4 separate versions you could have:
>
> struct gsc_version fitc_version;
>
> Which makes it easier to read as all headers have the same type for
> the version. We don't read this one though, so not a blocker.
Fine, I will take care of this.
Badal
>
> Daniele
>
>> +} __packed;
>> +
>> +struct csc_fpt_entry {
>> + u8 name[4]; /* partition name */
>> + u32 reserved1;
>> + u32 offset; /* offset from beginning of CSE region */
>> + u32 length; /* partition length in bytes */
>> + u32 reserved2[3];
>> + u32 partition_flags;
>> +} __packed;
>> +
>> #endif
>
Powered by blists - more mailing lists