[PATCH 01/10] drm/xe: Correct BMG VSEC header sizing

[Ruhl, Michael J]

>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen at linux.intel.com>
>Sent: Saturday, May 31, 2025 1:18 AM
>To: Ruhl, Michael J <michael.j.ruhl at intel.com>
>Cc: platform-driver-x86 at vger.kernel.org; intel-xe at lists.freedesktop.org; Hans
>de Goede <hdegoede at redhat.com>; De Marchi, Lucas
><lucas.demarchi at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>
>Subject: Re: [PATCH 01/10] drm/xe: Correct BMG VSEC header sizing
>
>On Fri, 30 May 2025, Michael J. Ruhl wrote:
>
>> The intel_vsec_header information for the crashlog feature
>> is incorrect.
>>
>> Update the VSEC header with correct sizing and count.
>
>Does this warrant a Fixes tag?

Since the crashlog support doesn't exist yet (for this instance), I don't think a fixes
is warranted.

>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_vsec.c | 20 +++++---------------
>>  1 file changed, 5 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
>> index 3e573b0b7ebd..67238fc57a4d 100644
>> --- a/drivers/gpu/drm/xe/xe_vsec.c
>> +++ b/drivers/gpu/drm/xe/xe_vsec.c
>> @@ -32,28 +32,18 @@ static struct intel_vsec_header bmg_telemetry = {
>>  	.offset = BMG_DISCOVERY_OFFSET,
>>  };
>>
>> -static struct intel_vsec_header bmg_punit_crashlog = {
>> -	.length = 0x10,
>> +static struct intel_vsec_header bmg_crashlog = {
>> +	.length = 0x18,
>>  	.id = VSEC_ID_CRASHLOG,
>> -	.num_entries = 1,
>> -	.entry_size = 4,
>> +	.num_entries = 2,
>> +	.entry_size = 6,
>>  	.tbir = 0,
>>  	.offset = BMG_DISCOVERY_OFFSET + 0x60,
>>  };
>>
>> -static struct intel_vsec_header bmg_oobmsm_crashlog = {
>> -	.length = 0x10,
>> -	.id = VSEC_ID_CRASHLOG,
>> -	.num_entries = 1,
>> -	.entry_size = 4,
>> -	.tbir = 0,
>> -	.offset = BMG_DISCOVERY_OFFSET + 0x78,
>> -};
>> -
>>  static struct intel_vsec_header *bmg_capabilities[] = {
>>  	&bmg_telemetry,
>> -	&bmg_punit_crashlog,
>> -	&bmg_oobmsm_crashlog,
>> +	&bmg_crashlog,
>
>Eh, this change goes way beyond what you said in the changelog, was that
>intentional? If yes, please describe and justify all the changes (and
>consider if some of them belong to a separate patch as it sounds like
>there are two or more changes mixed up into this patch).

With num_entries (the count) being update, this is in line with the change.  Originally they were
separate entries (punit and oobsms), but once I sized things correctly (0x18 vs 0x10), only one
capability entry was needed (similar to the telemetry entry).

I will expand the commit message.

M


>>  	NULL
>>  };
>>
>>
>
>--
> i.