[PATCH v6 6/6] drm/xe/guc: Only add GuC crash dump if available

[John Harrison]

On 8/4/2025 1:14 PM, John Harrison wrote:
> On 7/21/2025 6:35 PM, Zhanjun Dong wrote:
>> Add GuC crash dump data empty check. LFD will only include crash dump
>> section when data is not empty.
>>
>> Signed-off-by: Zhanjun Dong<zhanjun.dong at intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_guc_log.c | 60 +++++++++++++++++++++++++++++++++
>>   1 file changed, 60 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c 
>> b/drivers/gpu/drm/xe/xe_guc_log.c
>> index a8910211c579..f5f31f15e7e3 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_log.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
>> @@ -502,6 +502,65 @@ xe_guc_log_add_log_event(struct drm_printer *p, 
>> struct xe_guc_log_snapshot *snap
>>       return size;
>>   }
>>   +static int
>> +xe_guc_log_add_crash_dump(struct drm_printer *p, struct 
>> xe_guc_log_snapshot *snapshot,
>> +              struct guc_lic_save *config)
>> +{
>> +    struct guc_log_buffer_entry_list *entry;
>> +    int chunk_from, chunk_id, to_chunk_id;
>> +    int pos, from, to;
>> +    size_t size = 0;
>> +    char buf[GUC_LFD_BUFFER_SIZE] = {0};
>> +
>> +    entry = &config->entry[GUC_LOG_BUFFER_STATE_HEADER_ENTRY_CRASH];
>> +
>> +    /* Skip zero sized crash dump */
>> +    if (!entry->buf_size)
>> +        return 0;
>> +
>> +    /* Check if crash dump section are all zero */
>> +    from = entry->offset;
>> +    to = entry->offset + entry->buf_size;
>> +    chunk_from = from % GUC_LOG_CHUNK_SIZE;
>> +    chunk_id = from / GUC_LOG_CHUNK_SIZE;
>> +    to_chunk_id = to / GUC_LOG_CHUNK_SIZE;
>> +    pos = from;
>> +
>> +    do {
>> +        size_t size = (to_chunk_id > chunk_id ? GUC_LOG_CHUNK_SIZE : 
>> to) - chunk_from;
>> +        u32 *buf32 = snapshot->copy[chunk_id] + chunk_from;
>> +        int i;
>> +
>> +        for (i = 0; i < size / sizeof(u32); i++)
>> +            if (buf32[i])
>> +                break;
>> +        if (i < size / sizeof(u32)) {
>> +            pos += i * sizeof(u32) - chunk_from;
>> +            break;
>> +        }
>> +        pos += size;
>> +        chunk_id++;
>> +        chunk_from = 0;
>> +    } while (pos < to);
>> +
>> +    /* Buffer has non-zero data? */
>> +    if (pos < to) {
>> +        struct guc_lfd_data *lfd = (void *)buf;
>> +
>> +        size = xe_guc_log_add_lfd_header(buf, GUC_LFD_BUFFER_SIZE);
>> +        lfd->dw0 |= FIELD_PREP(GUC_LFD_DATA_DESC_TYPE, 
>> GUC_LFD_TYPE_FW_CRASH_DUMP);
>> +        /* Calculate data length */
>> +        lfd->dw_size = DIV_ROUND_UP(entry->buf_size, sizeof(u32));
>> +        /* Output GUC_LFD_TYPE_FW_CRASH_DUMP header */
>> +        xe_print_blob_ascii85(p, NULL, 0, buf, 0, size);
>> +
>> +        /* rd/wr ptr is not used for crash dump */
>> +        xe_guc_log_print_over_chunks(p, snapshot, entry->offset,
>> +                         entry->offset + entry->buf_size);
>> +    }
>> +    return size;
> Can't help but think that this is all way too complicated.
>
> The crash dump buffer is never going to be more than 1MB. And that 
> size is the smallest we can allocate when using large units in order 
> to get a larger log buffer (because the units are shared). My 
> understanding is that the actual size used is only a few pages 
> (currently working on getting a definitive answer from someone who 
> knows). So it would be far simpler to just save the crash buffer to 
> its own single, contiguous allocation in the snapshot with no chunking 
> required.
Just got an answer back. The size is needed is 12KB currently but will 
shortly rise to 16KB. So a private allocation in the snapshot will be 
sufficient and is definitely going to be simpler than going through the 
chunking stuff.

Unfortunately, that also means that our current default of 8KB in 
non-debug builds is too small! Can you post a patch to bump that up to 16KB?

Thanks,
John.

>
> Then all the above complication disappears. You get a simple "for(i = 
> 0; i < size; i++) if(i) break;" to check for empty and a single print 
> buffer call.
>
> John.
>
>> +}
>> +
>>   static void
>>   xe_guc_log_snapshot_print_lfd(struct xe_guc_log_snapshot *snapshot, 
>> struct drm_printer *p)
>>   {
>> @@ -522,6 +581,7 @@ xe_guc_log_snapshot_print_lfd(struct 
>> xe_guc_log_snapshot *snapshot, struct drm_p
>>           return;
>>         xe_guc_log_add_log_event(p, snapshot, &config);
>> +    xe_guc_log_add_crash_dump(p, snapshot, &config);
>>   }
>>     /**
>