[PATCH 1/3] drm/xe: Fix and re-enable xe_print_blob_ascii85()
Lucas De Marchi
lucas.demarchi at intel.com
Thu Jan 23 19:37:33 UTC 2025
On Thu, Jan 23, 2025 at 09:59:41AM -0800, Jose Souza wrote:
>From: Lucas De Marchi <lucas.demarchi at intel.com>
>
>Commit 70fb86a85dc9 ("drm/xe: Revert some changes that break a mesa
>debug tool") partially reverted some changes to workaround breakage
>caused to mesa tools. However, in doing so it also broke fetching the
>GuC log via debugfs since xe_print_blob_ascii85() simply bails out.
>
>The fix is to avoid the extra newlines: the devcoredump interface is
>line-oriented and adding random newlines in the middle breaks it. If a
>tool is able to parse it by looking at the data and checking for chars
>that are out of the ascii85 space, it can still do so. A format change
>that breaks the line-oriented output on devcoredump however needs better
>coordination with existing tools.
>
>Reviewed-by: José Roberto de Souza <jose.souza at intel.com>
>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")
>Fixes: ec1455ce7e35 ("drm/xe/devcoredump: Add ASCII85 dump helper function")
>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>---
> drivers/gpu/drm/xe/xe_devcoredump.c | 30 +++++++++--------------------
> drivers/gpu/drm/xe/xe_devcoredump.h | 2 +-
> drivers/gpu/drm/xe/xe_guc_ct.c | 3 ++-
> drivers/gpu/drm/xe/xe_guc_log.c | 4 +++-
> 4 files changed, 15 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
>index 81dc7795c0651..1c86e6456d60f 100644
>--- a/drivers/gpu/drm/xe/xe_devcoredump.c
>+++ b/drivers/gpu/drm/xe/xe_devcoredump.c
>@@ -395,42 +395,30 @@ int xe_devcoredump_init(struct xe_device *xe)
> /**
> * 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.
>+ * The output is split to multiple print calls because some print targets, e.g.
>+ * dmesg cannot handle arbitrarily long lines. These targets may add newline
>+ * between calls.
> *
> * 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
> * @prefix: optional prefix to add to output string
let's just make sure to document suffix
* @suffix: optional suffix to add at the end. 0 disables it and is
* not added to the output, which is useful when using multiple calls
* to dump data to @p
Lucas De Marchi
> * @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 char *prefix,
>+void xe_print_blob_ascii85(struct drm_printer *p, const char *prefix, char suffix,
> 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;
>
>- /*
>- * Splitting blobs across multiple lines is not compatible with the mesa
>- * debug decoder tool. Note that even dropping the explicit '\n' below
>- * doesn't help because the GuC log is so big some underlying implementation
>- * still splits the lines at 512K characters. So just bail completely for
>- * the moment.
>- */
>- return;
>-
> #define DMESG_MAX_LINE_LEN 800
>-#define MIN_SPACE (ASCII85_BUFSZ + 2) /* 85 + "\n\0" */
>+ /* Always leave space for the suffix char and the \0 */
>+#define MIN_SPACE (ASCII85_BUFSZ + 2) /* 85 + "<suffix>\0" */
>
> if (size & 3)
> drm_printf(p, "Size not word aligned: %zu", size);
>@@ -462,7 +450,6 @@ void xe_print_blob_ascii85(struct drm_printer *p, const char *prefix,
> 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);
>@@ -474,10 +461,11 @@ void xe_print_blob_ascii85(struct drm_printer *p, const char *prefix,
> }
> }
>
>+ if (suffix)
>+ line_buff[line_pos++] = suffix;
>+
> if (line_pos) {
>- line_buff[line_pos++] = '\n';
> line_buff[line_pos++] = 0;
>-
> drm_puts(p, line_buff);
> }
>
>diff --git a/drivers/gpu/drm/xe/xe_devcoredump.h b/drivers/gpu/drm/xe/xe_devcoredump.h
>index 6a17e6d601022..5391a80a4d1ba 100644
>--- a/drivers/gpu/drm/xe/xe_devcoredump.h
>+++ b/drivers/gpu/drm/xe/xe_devcoredump.h
>@@ -29,7 +29,7 @@ static inline int xe_devcoredump_init(struct xe_device *xe)
> }
> #endif
>
>-void xe_print_blob_ascii85(struct drm_printer *p, const char *prefix,
>+void xe_print_blob_ascii85(struct drm_printer *p, const char *prefix, char suffix,
> const void *blob, size_t offset, size_t size);
>
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
>index 8b65c5e959cc2..50c8076b51585 100644
>--- a/drivers/gpu/drm/xe/xe_guc_ct.c
>+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
>@@ -1724,7 +1724,8 @@ void xe_guc_ct_snapshot_print(struct xe_guc_ct_snapshot *snapshot,
> snapshot->g2h_outstanding);
>
> if (snapshot->ctb)
>- xe_print_blob_ascii85(p, "CTB data", snapshot->ctb, 0, snapshot->ctb_size);
>+ xe_print_blob_ascii85(p, "CTB data", '\n',
>+ snapshot->ctb, 0, snapshot->ctb_size);
> } else {
> drm_puts(p, "CT disabled\n");
> }
>diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
>index 80151ff6a71f8..44482ea919924 100644
>--- a/drivers/gpu/drm/xe/xe_guc_log.c
>+++ b/drivers/gpu/drm/xe/xe_guc_log.c
>@@ -207,8 +207,10 @@ void xe_guc_log_snapshot_print(struct xe_guc_log_snapshot *snapshot, struct drm_
> remain = snapshot->size;
> for (i = 0; i < snapshot->num_chunks; i++) {
> size_t size = min(GUC_LOG_CHUNK_SIZE, remain);
>+ const char *prefix = i ? NULL : "Log data";
>+ char suffix = i == snapshot->num_chunks - 1 ? '\n' : 0;
>
>- xe_print_blob_ascii85(p, i ? NULL : "Log data", snapshot->copy[i], 0, size);
>+ xe_print_blob_ascii85(p, prefix, suffix, snapshot->copy[i], 0, size);
> remain -= size;
> }
> }
>--
>2.48.1
>
More information about the Intel-xe
mailing list