[PATCH 1/4] drm/xe/guc: Update CSS header structures

John Harrison john.c.harrison at intel.com
Fri Aug 1 17:10:28 UTC 2025


On 7/31/2025 4:11 PM, Daniele Ceraolo Spurio wrote:
> On 7/24/2025 5:21 PM, John.C.Harrison at Intel.com wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> Rework the CSS header structure according to recent updates to the GuC
>> API spec. Also include more field definitions.
>>
>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_uc_fw.c     | 20 +++++-----
>>   drivers/gpu/drm/xe/xe_uc_fw_abi.h | 64 ++++++++++++++++++++-----------
>>   2 files changed, 51 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c 
>> b/drivers/gpu/drm/xe/xe_uc_fw.c
>> index a236f1d37248..c8378f3e0032 100644
>> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
>> @@ -344,11 +344,11 @@ static int guc_read_css_info(struct xe_uc_fw 
>> *uc_fw, struct uc_css_header *css)
>
> Since we now have a specific type for it, it might be worth changing 
> this function to accept struct uc_css_guc_info instead of struct 
> uc_css_header.
That works.

>
>>           return -EINVAL;
>>       }
>>   -    compatibility->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, 
>> css->submission_version);
>> -    compatibility->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, 
>> css->submission_version);
>> -    compatibility->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, 
>> css->submission_version);
>> +    compatibility->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, 
>> css->guc_info.submission_version);
>> +    compatibility->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, 
>> css->guc_info.submission_version);
>> +    compatibility->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, 
>> css->guc_info.submission_version);
>>   -    uc_fw->private_data_size = css->private_data_size;
>> +    uc_fw->private_data_size = css->guc_info.private_data_size;
>>         return 0;
>>   }
>> @@ -417,8 +417,8 @@ static int parse_css_header(struct xe_uc_fw 
>> *uc_fw, const void *fw_data, size_t
>>       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 -
>> -        css->exponent_size_dw) * sizeof(u32);
>> +    size = (css->header_size_dw - css->rsa_info.key_size_dw - 
>> css->rsa_info.modulus_size_dw -
>> +        css->rsa_info.exponent_size_dw) * sizeof(u32);
>>       if (unlikely(size != sizeof(struct uc_css_header))) {
>>           drm_warn(&xe->drm,
>>                "%s firmware %s: unexpected header size: %zu != %zu\n",
>> @@ -431,7 +431,7 @@ static int parse_css_header(struct xe_uc_fw 
>> *uc_fw, const void *fw_data, size_t
>>       uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * 
>> sizeof(u32);
>>         /* now RSA */
>> -    uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
>> +    uc_fw->rsa_size = css->rsa_info.key_size_dw * sizeof(u32);
>>         /* At least, it should have header, uCode and RSA. Size of 
>> all three. */
>>       size = sizeof(struct uc_css_header) + uc_fw->ucode_size +
>> @@ -444,9 +444,9 @@ static int parse_css_header(struct xe_uc_fw 
>> *uc_fw, const void *fw_data, size_t
>>       }
>>         /* Get version numbers from the CSS header */
>> -    release->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, 
>> css->sw_version);
>> -    release->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, 
>> css->sw_version);
>> -    release->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, 
>> css->sw_version);
>> +    release->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, 
>> css->guc_info.sw_version);
>> +    release->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, 
>> css->guc_info.sw_version);
>> +    release->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, 
>> css->guc_info.sw_version);
>>         if (uc_fw->type == XE_UC_FW_TYPE_GUC)
>>           return guc_read_css_info(uc_fw, css);
>> diff --git a/drivers/gpu/drm/xe/xe_uc_fw_abi.h 
>> b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> index 87ade41209d0..44d529dbce3b 100644
>> --- a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> +++ b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> @@ -44,6 +44,39 @@
>>    *    in fw. So driver will load a truncated firmware in this case.
>>    */
>>   +struct uc_css_rsa_info {
>> +    u32 key_size_dw;
>> +    u32 modulus_size_dw;
>> +    u32 exponent_size_dw;
>> +} __packed;
>> +
>> +struct uc_css_guc_info {
>> +    u32 time;
>> +#define CSS_TIME_HOUR                (0xFF << 0)
>> +#define CSS_DATE_MIN                (0xFF << 8)
>> +#define CSS_DATE_SEC                (0xFFFF << 16)
>> +    u32 reserved0[5];
>> +    u32 sw_version;
>> +#define CSS_SW_VERSION_UC_MAJOR            (0xFF << 16)
>> +#define CSS_SW_VERSION_UC_MINOR            (0xFF << 8)
>> +#define CSS_SW_VERSION_UC_PATCH            (0xFF << 0)
>> +    u32 submission_version;
>> +    u32 reserved1[11];
>> +    u32 header_info;
>> +#define CSS_HEADER_INFO_SVN            (0xFF)
>> +#define CSS_HEADER_INFO_COPY_VALID        (0x1 << 31)
>> +    u32 private_data_size;
>> +    u32 ukernel_info;
>> +#define CSS_UKERNEL_INFO_DEVICEID        (0xFFFF << 16)
>> +#define CSS_UKERNEL_INFO_PRODKEY        (0xFF << 8)
>> +#define CSS_UKERNEL_INFO_BUILDTYPE        (0x3 << 2)
>> +#define CSS_UKERNEL_INFO_BUILDTYPE_PROD        0
>> +#define CSS_UKERNEL_INFO_BUILDTYPE_PREPROD    1
>> +#define CSS_UKERNEL_INFO_BUILDTYPE_DEBUG    2
>> +#define CSS_UKERNEL_INFO_ENCSTATUS        (0x1 << 1)
>> +#define CSS_UKERNEL_INFO_COPY_VALID        (0x1 << 0)
>
> I'd skip defining CSS_UKERNEL_INFO_COPY_VALID and 
> CSS_HEADER_INFO_COPY_VALID. They're only set in the HW's copy of those 
> fields, which are only consumed by bootrom; we'll never see them set 
> from the driver POV (or even from the POV of someone hexdumping the 
> binary), so having the define has no use.

It's more about not having holes in the definition leaving people to 
wonder if that is a bug in the driver, a deliberate obfuscation, etc. 
One could call it _RESERVED but that seems inaccurate when it does 
actually have a definition.

>
>> +} __packed;
>> +
>>   struct uc_css_header {
>>       u32 module_type;
>>       /*
>> @@ -52,36 +85,21 @@ struct uc_css_header {
>>        */
>>       u32 header_size_dw;
>>       u32 header_version;
>> -    u32 module_id;
>> +    u32 reserved0;
>>       u32 module_vendor;
>>       u32 date;
>> -#define CSS_DATE_DAY            (0xFF << 0)
>> -#define CSS_DATE_MONTH            (0xFF << 8)
>> -#define CSS_DATE_YEAR            (0xFFFF << 16)
>> +#define CSS_DATE_DAY                (0xFF << 0)
>> +#define CSS_DATE_MONTH                (0xFF << 8)
>> +#define CSS_DATE_YEAR                (0xFFFF << 16)
>>       u32 size_dw; /* uCode plus header_size_dw */
>> -    u32 key_size_dw;
>> -    u32 modulus_size_dw;
>> -    u32 exponent_size_dw;
>> -    u32 time;
>> -#define CSS_TIME_HOUR            (0xFF << 0)
>> -#define CSS_DATE_MIN            (0xFF << 8)
>> -#define CSS_DATE_SEC            (0xFFFF << 16)
>> -    char username[8];
>> -    char buildnumber[12];
>> -    u32 sw_version;
>> -#define CSS_SW_VERSION_UC_MAJOR        (0xFF << 16)
>> -#define CSS_SW_VERSION_UC_MINOR        (0xFF << 8)
>> -#define CSS_SW_VERSION_UC_PATCH        (0xFF << 0)
>>       union {
>> -        u32 submission_version; /* only applies to GuC */
>> -        u32 reserved2;
>> +        u32 reserved1[3];
>> +        struct uc_css_rsa_info rsa_info;
>
> Do we actually support any binary where this block is reserved instead 
> of containing the rsa_info?
Not yet, but there are plans...

John.

>
> Daniele
>
>>       };
>> -    u32 reserved0[12];
>>       union {
>> -        u32 private_data_size; /* only applies to GuC */
>> -        u32 reserved1;
>> +        u32 reserved2[22];
>> +        struct uc_css_guc_info guc_info;
>>       };
>> -    u32 header_info;
>>   } __packed;
>>   static_assert(sizeof(struct uc_css_header) == 128);
>



More information about the Intel-xe mailing list