[PATCH 2/2] drm/xe/guc: Add support for NPK as a GuC log target

[Lucas De Marchi]

On Wed, Jul 23, 2025 at 02:20:20PM -0700, John.C.Harrison at Intel.com wrote:
>From: John Harrison <John.C.Harrison at Intel.com>
>
>The GuC has an option to write log data via NPK. This is basically a
>magic IO address that GuC writes arbitrary data to and which can be
>logged by a suitable hardware logger. This can allow retrieval of the
>GuC log in hardware debug environments even when the system as a whole
>dies horribly.
>
>Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>---
> drivers/gpu/drm/xe/xe_configfs.c | 61 ++++++++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_configfs.h |  3 ++
> drivers/gpu/drm/xe/xe_guc.c      |  5 +++
> 3 files changed, 69 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
>index 8ec1ff1e4e80..9fbd7e2ab152 100644
>--- a/drivers/gpu/drm/xe/xe_configfs.c
>+++ b/drivers/gpu/drm/xe/xe_configfs.c
>@@ -77,6 +77,17 @@
>  * available for migrations, but it's disabled. This is intended for debugging
>  * purposes only.
>  *
>+ * GuC log target:
>+ * ---------------
>+ *
>+ * Specify the target for GuC firmware logging:
>+ *   0 = memory
>+ *   1 = NPK
>+ *   2 = memory + NPK
>+ *
>+ * Note that logging via NPK (North Peak) is only useful if some kind of hardware
>+ * debugger is attached that can save out the NPK stream externally.
>+ *
>  * Remove devices
>  * ==============
>  *
>@@ -90,6 +101,7 @@ struct xe_config_device {
>
> 	bool survivability_mode;
> 	u64 engines_allowed;
>+	u32 guc_log_target;

could be u8, right?

>
> 	/* protects attributes */
> 	struct mutex lock;
>@@ -226,12 +238,38 @@ static ssize_t engines_allowed_store(struct config_item *item, const char *page,
> 	return len;
> }
>
>+static ssize_t guc_log_target_show(struct config_item *item, char *page)
>+{
>+	struct xe_config_device *dev = to_xe_config_device(item);
>+
>+	return sprintf(page, "%d\n", dev->guc_log_target);
>+}
>+
>+static ssize_t guc_log_target_store(struct config_item *item, const char *page, size_t len)
>+{
>+	struct xe_config_device *dev = to_xe_config_device(item);
>+	u32 guc_log_target;
>+	int ret;
>+
>+	ret = kstrtou32(page, 0, &guc_log_target);
>+	if (ret)
>+		return ret;
>+
>+	mutex_lock(&dev->lock);
>+	dev->guc_log_target = guc_log_target;

nit: we are copy and pasting this function for each attribute and adjusting
the type/var. In https://patchwork.freedesktop.org/patch/665377/?series=151729&rev=2
I decided to name it `var` so at least it's easier to copy and paste and
an eventual consolidation later more obvious.

>+	mutex_unlock(&dev->lock);
>+
>+	return len;
>+}
>+
> CONFIGFS_ATTR(, survivability_mode);
> CONFIGFS_ATTR(, engines_allowed);
>+CONFIGFS_ATTR(, guc_log_target);
>
> static struct configfs_attribute *xe_config_device_attrs[] = {
> 	&attr_survivability_mode,
> 	&attr_engines_allowed,
>+	&attr_guc_log_target,

There's also this patch that will conflict here,
https://patchwork.freedesktop.org/patch/665141/?series=151773&rev=4
particularly if the the suggestion for adding a defaults is done.

> 	NULL,
> };
>
>@@ -386,6 +424,29 @@ u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev)
> 	return engines_allowed;
> }
>
>+/**
>+ * xe_configfs_get_guc_log_target - get configfs GuC log target attribute
>+ * @pdev: pci device
>+ *
>+ * find the configfs group that belongs to the pci device and return
>+ * the GuC log target attribute

There's also this in my patch series:
https://patchwork.freedesktop.org/patch/665376/?series=151729&rev=2
Any opinion?

>+ *
>+ * Return: GuC log target if config group is found, false otherwise
>+ */
>+u32 xe_configfs_get_guc_log_target(struct pci_dev *pdev)
>+{
>+	struct xe_config_device *dev = configfs_find_group(pdev);
>+	u32 guc_log_target;
>+
>+	if (!dev)
>+		return false;
>+
>+	guc_log_target = dev->guc_log_target;
>+	config_item_put(&dev->group.cg_item);
>+
>+	return guc_log_target;
>+}
>+
> int __init xe_configfs_init(void)
> {
> 	struct config_group *root = &xe_configfs.su_group;
>diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h
>index fb8764008089..97f9ad0f9495 100644
>--- a/drivers/gpu/drm/xe/xe_configfs.h
>+++ b/drivers/gpu/drm/xe/xe_configfs.h
>@@ -16,12 +16,15 @@ void xe_configfs_exit(void);
> bool xe_configfs_get_survivability_mode(struct pci_dev *pdev);
> void xe_configfs_clear_survivability_mode(struct pci_dev *pdev);
> u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev);
>+u32 xe_configfs_get_guc_log_target(struct pci_dev *pdev);
> #else
> static inline int xe_configfs_init(void) { return 0; }
> static inline void xe_configfs_exit(void) { }
> static inline bool xe_configfs_get_survivability_mode(struct pci_dev *pdev) { return false; }
> static inline void xe_configfs_clear_survivability_mode(struct pci_dev *pdev) { }
> static inline u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev) { return U64_MAX; }
>+static inline u32 xe_configfs_get_guc_log_target(struct pci_dev *pdev) { return 0; }
>+

trailing newline here that can be fixed while pushing.

I'm ok pushing this and I and Michal rebase on top after the reviews are
finished.

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

> #endif
>
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>index 8fac3c518975..53ef93e162d2 100644
>--- a/drivers/gpu/drm/xe/xe_guc.c
>+++ b/drivers/gpu/drm/xe/xe_guc.c
>@@ -16,6 +16,7 @@
> #include "regs/xe_guc_regs.h"
> #include "regs/xe_irq_regs.h"
> #include "xe_bo.h"
>+#include "xe_configfs.h"
> #include "xe_device.h"
> #include "xe_force_wake.h"
> #include "xe_gt.h"
>@@ -67,6 +68,7 @@ static u32 guc_bo_ggtt_addr(struct xe_guc *guc,
>
> static u32 guc_ctl_debug_flags(struct xe_guc *guc)
> {
>+	u32 target = xe_configfs_get_guc_log_target(to_pci_dev(guc_to_xe(guc)->drm.dev));
> 	u32 level = xe_guc_log_get_level(&guc->log);
> 	u32 flags = 0;
>
>@@ -75,6 +77,9 @@ static u32 guc_ctl_debug_flags(struct xe_guc *guc)
> 	else
> 		flags |= FIELD_PREP(GUC_LOG_VERBOSITY, GUC_LOG_LEVEL_TO_VERBOSITY(level));
>
>+	if (target)
>+		flags |= FIELD_PREP(GUC_LOG_DESTINATION, target);
>+
> 	return flags;
> }
>
>-- 
>2.49.0
>