[PATCH 1/2] drm/xe/devcoredump: Move exec queue snapshot to Contexts section

[Lucas De Marchi]

On Thu, Jan 23, 2025 at 08:14:11AM -0600, Jose Souza wrote:
>On Wed, 2025-01-22 at 21:11 -0800, Lucas De Marchi wrote:
>> Having the exec queue snapshot inside a "GuC CT" section was always
>> wrong.  Commit c28fd6c358db ("drm/xe/devcoredump: Improve section
>> headings and add tile info") tried to fix that bug, but with that also
>> broke the mesa tool that parses the devcoredump, hence it was reverted
>> in commit 70fb86a85dc9 ("drm/xe: Revert some changes that break a mesa
>> debug tool").
>>
>> With the mesa tool also fixed, this can propagate as a fix on both
>> kernel and userspace side to avoid unnecessary headache for a debug
>> feature.
>
>This will break older versions of the Mesa parser. Is this really necessary?

See cover letter with the mesa MR that would fix the tool to follow the
kernel fix and work with both newer and older format. Linking it here
anyway: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33177

It's a fix so simple that IMO it's better than carrying the cruft ad
infinitum on all the tools that may possibly parse the devcoredump.


>Is it worth breaking the tool? In my opinion, it is not.
>
>Also, do we need to discuss this now? Wouldn't it be better to focus on bringing the GuC log in first?

That's what the second patch does. We need to discuss both now and
decide, otherwise we can't re-enable it and have either the guc log
parser or mesa's aubinator_error_decode_xe broken.

Lucas De Marchi

>
>>
>> Cc: John Harrison <John.C.Harrison at Intel.com>
>> Cc: Julia Filipchuk <julia.filipchuk at intel.com>
>> Cc: José Roberto de Souza <jose.souza at intel.com>
>> Cc: stable at vger.kernel.org
>> Fixes: 70fb86a85dc9 ("drm/xe: Revert some changes that break a mesa debug tool")
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_devcoredump.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
>> index 81dc7795c0651..a7946a76777e7 100644
>> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
>> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
>> @@ -119,11 +119,7 @@ static ssize_t __xe_devcoredump_read(char *buffer, size_t count,
>>  	drm_puts(&p, "\n**** GuC CT ****\n");
>>  	xe_guc_ct_snapshot_print(ss->guc.ct, &p);
>>
>> -	/*
>> -	 * Don't add a new section header here because the mesa debug decoder
>> -	 * tool expects the context information to be in the 'GuC CT' section.
>> -	 */
>> -	/* drm_puts(&p, "\n**** Contexts ****\n"); */
>> +	drm_puts(&p, "\n**** Contexts ****\n");
>>  	xe_guc_exec_queue_snapshot_print(ss->ge, &p);
>>
>>  	drm_puts(&p, "\n**** Job ****\n");
>