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

[John Harrison]

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.

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);
>   }
>   
>   /**