lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ