Re: [PATCH v2 11/12] drm/xe/pxp: Add PXP debugfs support

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

On 8/16/2024 12:00, Daniele Ceraolo Spurio wrote:
> This patch introduces 2 PXP debugfs entries:
>
> - info: prints the current PXP status and key instance
> - terminate: simulate a termination interrupt
>
> The first one is useful for debug, while the second one can be used for
> testing the termination flow.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/xe/Makefile         |   1 +
>   drivers/gpu/drm/xe/xe_debugfs.c     |   3 +
>   drivers/gpu/drm/xe/xe_pxp_debugfs.c | 120 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_pxp_debugfs.h |  13 +++
>   4 files changed, 137 insertions(+)
>   create mode 100644 drivers/gpu/drm/xe/xe_pxp_debugfs.c
>   create mode 100644 drivers/gpu/drm/xe/xe_pxp_debugfs.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index a508b9166b88..7cc65f419710 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -84,6 +84,7 @@ xe-y += xe_bb.o \
>   	xe_pt.o \
>   	xe_pt_walk.o \
>   	xe_pxp.o \
> +	xe_pxp_debugfs.o \
>   	xe_pxp_submit.o \
>   	xe_query.o \
>   	xe_range_fence.o \
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index 1011e5d281fa..a04f9c2d886b 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -17,6 +17,7 @@
>   #include "xe_gt_printk.h"
>   #include "xe_guc_ads.h"
>   #include "xe_pm.h"
> +#include "xe_pxp_debugfs.h"
>   #include "xe_sriov.h"
>   #include "xe_step.h"
>   
> @@ -214,6 +215,8 @@ void xe_debugfs_register(struct xe_device *xe)
>   	for_each_gt(gt, xe, id)
>   		xe_gt_debugfs_register(gt);
>   
> +	xe_pxp_debugfs_register(xe->pxp);
> +
>   #ifdef CONFIG_FAULT_INJECTION
>   	fault_create_debugfs_attr("fail_gt_reset", root, &gt_reset_failure);
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_pxp_debugfs.c b/drivers/gpu/drm/xe/xe_pxp_debugfs.c
> new file mode 100644
> index 000000000000..00c8179a9f0f
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pxp_debugfs.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include "xe_pxp_debugfs.h"
> +
> +#include <linux/debugfs.h>
> +
> +#include <drm/drm_debugfs.h>
> +#include <drm/drm_managed.h>
> +#include <drm/drm_print.h>
> +
> +#include "xe_device.h"
> +#include "xe_pxp.h"
> +#include "xe_pxp_types.h"
> +#include "regs/xe_gt_regs.h"
> +
> +static struct xe_pxp *node_to_pxp(struct drm_info_node *node)
> +{
> +	return node->info_ent->data;
> +}
> +
> +static const char *pxp_status_to_str(struct xe_pxp *pxp)
> +{
> +	lockdep_assert_held(&pxp->mutex);
> +
> +	switch (pxp->status) {
> +	case XE_PXP_ERROR:
> +		return "error";
> +	case XE_PXP_NEEDS_TERMINATION:
> +		return "needs termination";
> +	case XE_PXP_TERMINATION_IN_PROGRESS:
> +		return "termination in progress";
> +	case XE_PXP_READY_TO_START:
> +		return "ready to start";
> +	case XE_PXP_ACTIVE:
> +		return "active";
> +	case XE_PXP_SUSPENDED:
> +		return "suspended";
> +	default:
> +		return "unknown";
> +	}
> +};
> +
> +static int pxp_info(struct seq_file *m, void *data)
> +{
> +	struct xe_pxp *pxp = node_to_pxp(m->private);
> +	struct drm_printer p = drm_seq_file_printer(m);
> +	const char *status;
> +
> +	if (!xe_pxp_is_enabled(pxp))
> +		return -ENODEV;
> +
> +	mutex_lock(&pxp->mutex);
> +	status = pxp_status_to_str(pxp);
> +	mutex_unlock(&pxp->mutex);
> +
> +	drm_printf(&p, "status: %s\n", status);
> +	drm_printf(&p, "instance counter: %u\n", pxp->key_instance);
Maybe cache this inside the mutex lock as well? So that the instance and 
the status were at least a valid pair at one point even if things might 
have changed since.

John.

> +
> +	return 0;
> +}
> +
> +static int pxp_terminate(struct seq_file *m, void *data)
> +{
> +	struct xe_pxp *pxp = node_to_pxp(m->private);
> +	struct drm_printer p = drm_seq_file_printer(m);
> +
> +	if (!xe_pxp_is_enabled(pxp))
> +		return -ENODEV;
> +
> +	/* simulate a termination interrupt */
> +	spin_lock_irq(&pxp->xe->irq.lock);
> +	xe_pxp_irq_handler(pxp->xe, KCR_PXP_STATE_TERMINATED_INTERRUPT);
> +	spin_unlock_irq(&pxp->xe->irq.lock);
> +
> +	drm_printf(&p, "Termination queued\n");
> +
> +	return 0;
> +}
> +
> +static const struct drm_info_list debugfs_list[] = {
> +	{"info", pxp_info, 0},
> +	{"terminate", pxp_terminate, 0},
> +};
> +
> +void xe_pxp_debugfs_register(struct xe_pxp *pxp)
> +{
> +	struct drm_minor *minor;
> +	struct drm_info_list *local;
> +	struct dentry *root;
> +	int i;
> +
> +	if (!xe_pxp_is_enabled(pxp))
> +		return;
> +
> +	minor = pxp->xe->drm.primary;
> +	if (!minor->debugfs_root)
> +		return;
> +
> +#define DEBUGFS_SIZE	(ARRAY_SIZE(debugfs_list) * sizeof(struct drm_info_list))
> +	local = drmm_kmalloc(&pxp->xe->drm, DEBUGFS_SIZE, GFP_KERNEL);
> +	if (!local)
> +		return;
> +
> +	memcpy(local, debugfs_list, DEBUGFS_SIZE);
> +#undef DEBUGFS_SIZE
> +
> +	for (i = 0; i < ARRAY_SIZE(debugfs_list); ++i)
> +		local[i].data = pxp;
> +
> +	root = debugfs_create_dir("pxp", minor->debugfs_root);
> +	if (IS_ERR(root))
> +		return;
> +
> +	drm_debugfs_create_files(local,
> +				 ARRAY_SIZE(debugfs_list),
> +				 root, minor);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_pxp_debugfs.h b/drivers/gpu/drm/xe/xe_pxp_debugfs.h
> new file mode 100644
> index 000000000000..988466aad50b
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pxp_debugfs.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#ifndef __XE_PXP_DEBUGFS_H__
> +#define __XE_PXP_DEBUGFS_H__
> +
> +struct xe_pxp;
> +
> +void xe_pxp_debugfs_register(struct xe_pxp *pxp);
> +
> +#endif /* __XE_PXP_DEBUGFS_H__ */