Re: [PATCH v6 2/9] drm/xe: Add ASCII85 dump helper function

[John Harrison <john.c.harrison@intel.com>]

On 8/29/2024 23:23, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> There is a need to include the GuC log and other large binary objects
> in core dumps and via dmesg. So add a helper for dumping to a printer
> function via conversion to ASCII85 encoding.
>
> Another issue with dumping such a large buffer is that it can be slow,
> especially if dumping to dmesg over a serial port. So add a yield to
> prevent the 'task has been stuck for 120s' kernel hang check feature
> from firing.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
The intent would be to use this helper for the existing ascii85 dumps in 
xe_lrc.c and xe_vm.c. Otherwise those will hit problems with maximum 
line lengths when spewing a devcoredump to dmesg. However, that will 
also add line wrapping to the regular devcoredump via sysfs output. And 
I'm not sure if that would confuse the decoding tool? White space should 
just be ignored when decoding ASCII85 but the tool might well not be 
aware that the data spans multiple lines.

One thought is to move the line wrapping out of the encoding helper and 
in to the drm line printer helper. However that would require putting 
memory allocation operations in the line printer and that seems more 
problematic as that could be used in atomic context. It seems much 
simpler/safer to just allow for line wrapping in the sysfs output.

John.

> ---
>   drivers/gpu/drm/xe/xe_devcoredump.c | 76 +++++++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_devcoredump.h |  5 ++
>   2 files changed, 81 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index bdb76e834e4c..eec7b89ab48b 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -6,6 +6,7 @@
>   #include "xe_devcoredump.h"
>   #include "xe_devcoredump_types.h"
>   
> +#include <linux/ascii85.h>
>   #include <linux/devcoredump.h>
>   #include <generated/utsrelease.h>
>   
> @@ -310,3 +311,78 @@ int xe_devcoredump_init(struct xe_device *xe)
>   }
>   
>   #endif
> +
> +/**
> + * xe_print_blob_ascii85 - print a BLOB to some useful location in ASCII85
> + *
> + * The output is split to multiple lines because some print targets, e.g. dmesg
> + * cannot handle arbitrarily long lines. Note also that printing to dmesg in
> + * piece-meal fashion is not possible, each separate call to drm_puts() has a
> + * line-feed automatically added! Therefore, the entire output line must be
> + * constructed in a local buffer first, then printed in one atomic output call.
> + *
> + * There is also a scheduler yield call to prevent the 'task has been stuck for
> + * 120s' kernel hang check feature from firing when printing to a slow target
> + * such as dmesg over a serial port.
> + *
> + * TODO: Add compression prior to the ASCII85 encoding to shrink huge buffers down.
> + *
> + * @p: the printer object to output to
> + * @blob: the Binary Large OBject to dump out
> + * @offset: offset in bytes to skip from the front of the BLOB, must be a multiple of sizeof(u32)
> + * @size: the size in bytes of the BLOB, must be a multiple of sizeof(u32)
> + */
> +void xe_print_blob_ascii85(struct drm_printer *p, const void *blob, size_t offset, size_t size)
> +{
> +	const u32 *blob32 = (const u32 *)blob;
> +	char buff[ASCII85_BUFSZ], *line_buff;
> +	size_t line_pos = 0;
> +
> +#define DMESG_MAX_LINE_LEN	800
> +#define MIN_SPACE		(ASCII85_BUFSZ + 2)		/* 85 + "\n\0" */
> +
> +	if (size & 3)
> +		drm_printf(p, "Size not word aligned: %zu", size);
> +	if (offset & 3)
> +		drm_printf(p, "Offset not word aligned: %zu", size);
> +
> +	line_buff = kzalloc(sizeof(DMESG_MAX_LINE_LEN), GFP_KERNEL);
> +	if (IS_ERR(line_buff)) {
> +		drm_printf(p, "Failed to allocate line buffer: %pe", line_buff);
> +		return;
> +	}
> +
> +	blob32 += offset / sizeof(*blob32);
> +	size /= sizeof(*blob32);
> +
> +	while (size--) {
> +		u32 val = *(blob32++);
> +
> +		strscpy(line_buff + line_pos, ascii85_encode(val, buff),
> +			DMESG_MAX_LINE_LEN - line_pos);
> +		line_pos += strlen(line_buff + line_pos);
> +
> +		if ((line_pos + MIN_SPACE) >= DMESG_MAX_LINE_LEN) {
> +			line_buff[line_pos++] = '\n';
> +			line_buff[line_pos++] = 0;
> +
> +			drm_puts(p, line_buff);
> +
> +			line_pos = 0;
> +
> +			/* Prevent 'stuck thread' time out errors */
> +			cond_resched();
> +		}
> +	}
> +
> +	if (line_pos) {
> +		line_buff[line_pos++] = '\n';
> +		line_buff[line_pos++] = 0;
> +
> +		drm_puts(p, line_buff);
> +	}
> +
> +	kfree(line_buff);
> +
> +#undef MIN_SPACE
> +}
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.h b/drivers/gpu/drm/xe/xe_devcoredump.h
> index e2fa65ce0932..3f82188590ac 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.h
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.h
> @@ -6,6 +6,9 @@
>   #ifndef _XE_DEVCOREDUMP_H_
>   #define _XE_DEVCOREDUMP_H_
>   
> +#include <linux/types.h>
> +
> +struct drm_printer;
>   struct xe_device;
>   struct xe_sched_job;
>   
> @@ -23,4 +26,6 @@ static inline int xe_devcoredump_init(struct xe_device *xe)
>   }
>   #endif
>   
> +void xe_print_blob_ascii85(struct drm_printer *p, const void *blob, size_t offset, size_t size);
> +
>   #endif