Re: [PATCH v5 2/3] drm/xe/sysctrl: Add RAS error injection debugfs interface

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Sep 10, 2026 at 10:56:21AM -0700, Anoop, Vijay wrote:
> From: Anoop Vijay <anoop.c.vijay@intel.com>
> 
> Add debugfs interface for exercising System Controller's RAS error
> injection command, used to validate RAS error detection and recovery
> paths.
> 
> Command details:
> - Group ID: 0x02 (diag group)
> - Command ID: 0x7E (XE_SYSCTRL_CMD_DIAG_RAS_ERR_INJECT)
> - Usage: echo "<ras_block_id> <ras_sub_block_id> <err_type> [params]" \
>              > /sys/kernel/debug/dri/0/sc/ras_error_inject
>          cat /sys/kernel/debug/dri/0/sc/ras_error_inject
> 
> This command requires the diag application to have completed firmware
> boot and initialization (late-bind loaded). Both cat and echo are
> rejected with -ENODEV until xe_sysctrl_is_diag_fw_ready() reports the
> diag firmware as ready: the readiness check is done in .open(), so the
> file stays visible under sc/ but is inaccessible for both read and
> write until the diag firmware becomes ready.
> 
> Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
> ---
> v5:
> - Add per-entry locking for debugfs accesses
> - Add xe_pm_runtime guards for ras_error_inject readiness checks
> - Simplify ras_error_inject write-path flow
> 
> v4 (Rodrigo, Anshuman):
> - Gated ras_error_inject on diag firmware readiness in .open()
> ---
>  drivers/gpu/drm/xe/xe_sysctrl_debugfs.c       | 139 ++++++++++++++++++
>  drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h |  31 ++++
>  drivers/gpu/drm/xe/xe_sysctrl_types.h         |   3 +
>  3 files changed, 173 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_debugfs.c b/drivers/gpu/drm/xe/xe_sysctrl_debugfs.c
> index c0454c4c0ae0..0c537248b30d 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_debugfs.c
> @@ -10,6 +10,7 @@
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
> +#include <linux/string_choices.h>
>  #include <linux/uaccess.h>
>  
>  #include "xe_pm.h"
> @@ -127,6 +128,140 @@ static const struct file_operations xe_sysctrl_loopback_fops = {
>  	.release = single_release,
>  };
>  
> +static ssize_t xe_sysctrl_ras_error_inject_write(struct file *file, const char __user *ubuf,
> +						 size_t len, loff_t *offp)
> +{
> +	char *kbuf __free(kfree) = NULL;
> +	struct seq_file *m = file->private_data;
> +	struct xe_sysctrl_debugfs_entry *entry = m->private;
> +	struct xe_device *xe = sc_to_xe(entry->sc);
> +	struct xe_sysctrl_diag_ras_err_inj_req req = {};
> +	struct xe_sysctrl_mailbox_command cmd = {};
> +	u8 resp_hdr_only[sizeof(u32)];
> +	unsigned int nfields = 0;
> +	char *token, *tmp;
> +	unsigned long val;
> +	size_t out_len = 0;
> +	int status;
> +
> +	if (len == 0 || len >= PAGE_SIZE)
> +		return -EINVAL;
> +
> +	kbuf = kmalloc(len + 1, GFP_KERNEL);
> +	if (!kbuf)
> +		return -ENOMEM;
> +
> +	if (copy_from_user(kbuf, ubuf, len))
> +		return -EFAULT;
> +	kbuf[len] = '\0';
> +
> +	tmp = kbuf;
> +	while ((token = strsep(&tmp, " \t\n")) != NULL) {
> +		if (*token == '\0')
> +			continue;
> +
> +		if (kstrtoul(token, 0, &val))
> +			goto inval;
> +
> +		switch (nfields) {
> +		case 0:
> +			if (val > U16_MAX)
> +				goto inval;
> +			req.ras_block_id = val;
> +			break;
> +		case 1:
> +			if (val > U16_MAX)
> +				goto inval;
> +			req.ras_sub_block_id = val;
> +			break;
> +		case 2:
> +			if (val > U16_MAX)
> +				goto inval;
> +			req.err_type = val;
> +			break;
> +		case 3:
> +			if (val > U32_MAX)
> +				goto inval;
> +			req.params = val;
> +			break;
> +		default:
> +			xe_err(xe, "sysctrl: too many ras_error_inject arguments\n");
> +			return -EINVAL;
> +		}
> +		nfields++;
> +	}
> +
> +	if (nfields < 3) {
> +		xe_err(xe,
> +		       "sysctrl: usage: <ras_block_id> <ras_sub_block_id> <err_type> [params]\n");
> +		return -EINVAL;
> +	}
> +
> +	xe_sysctrl_create_command(&cmd, entry->group, entry->command,
> +				  &req, sizeof(req), resp_hdr_only,
> +				  sizeof(resp_hdr_only));
> +
> +	scoped_guard(mutex, &entry->lock) {
> +		guard(xe_pm_runtime)(xe);
> +		status = xe_sysctrl_send_command(entry->sc, &cmd, &out_len);
> +		entry->status = status;
> +	}
> +
> +	return status ? status : len;
> +
> +inval:
> +	xe_err(xe, "sysctrl: invalid ras_error_inject token '%s'\n", token);

I understand and agree with your goal of avoiding duplicating this msg above,
but at the same time I agree with Sashiko this mixed style is bad.

Please consider splitting the token parsing in a separate function so you
can better organize this.

probably Claude Opus or Sonnet here can help a bit.

> +	return -EINVAL;
> +}
> +
> +static int xe_sysctrl_ras_error_inject_show(struct seq_file *m, void *data)
> +{
> +	struct xe_sysctrl_debugfs_entry *entry = m->private;
> +	struct xe_device *xe = sc_to_xe(entry->sc);
> +	bool fw_ready;
> +
> +	scoped_guard(xe_pm_runtime, xe)
> +		fw_ready = xe_sysctrl_is_diag_fw_ready(xe);
> +
> +	guard(mutex)(&entry->lock);
> +
> +	seq_printf(m, "Command: group=0x%02x cmd=0x%02x\n", entry->group, entry->command);
> +	seq_printf(m, "Diag firmware ready: %s\n", str_yes_no(fw_ready));
> +	seq_printf(m, "Status: %d (%s)\n", entry->status, entry->status ? "FAILED" : "SUCCESS");
> +
> +	seq_puts(m, "\nUsage:\n");
> +	seq_puts(m, "  echo \"<ras_block_id> <ras_sub_block_id> <err_type> [params]\" > ras_error_inject\n");
> +	seq_puts(m, "  cat ras_error_inject\n");
> +
> +	return 0;
> +}
> +
> +static int xe_sysctrl_ras_error_inject_open(struct inode *inode, struct file *file)
> +{
> +	struct xe_sysctrl_debugfs_entry *entry = inode->i_private;
> +	struct xe_device *xe = sc_to_xe(entry->sc);
> +	bool fw_ready;
> +
> +	scoped_guard(xe_pm_runtime, xe)
> +		fw_ready = xe_sysctrl_is_diag_fw_ready(xe);
> +
> +	if (!fw_ready) {
> +		xe_err(xe, "sysctrl: diag firmware not ready, ras_error_inject unavailable\n");
> +		return -ENODEV;
> +	}
> +
> +	return single_open(file, xe_sysctrl_ras_error_inject_show, inode->i_private);
> +}
> +
> +static const struct file_operations xe_sysctrl_ras_error_inject_fops = {
> +	.owner = THIS_MODULE,
> +	.open = xe_sysctrl_ras_error_inject_open,
> +	.read = seq_read,
> +	.write = xe_sysctrl_ras_error_inject_write,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +};
> +
>  static void xe_sysctrl_register_entry(struct dentry *root, struct xe_sysctrl_debugfs_entry *entry,
>  				      struct xe_sysctrl *sc, const char *name,
>  				      u8 group, u8 command,
> @@ -164,4 +299,8 @@ void xe_sysctrl_debugfs_register(struct xe_sysctrl *sc, struct dentry *parent)
>  	xe_sysctrl_register_entry(root, &sc->debugfs.loopback, sc, "loopback",
>  				  XE_SYSCTRL_GROUP_CORE, XE_SYSCTRL_CMD_LOOPBACK,
>  				  &xe_sysctrl_loopback_fops);
> +
> +	xe_sysctrl_register_entry(root, &sc->debugfs.ras_error_inject, sc, "ras_error_inject",
> +				  XE_SYSCTRL_GROUP_DIAG, XE_SYSCTRL_CMD_DIAG_RAS_ERR_INJECT,
> +				  &xe_sysctrl_ras_error_inject_fops);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> index 501a4a4c16ff..0f65bef42399 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> @@ -14,10 +14,12 @@
>   * enum xe_sysctrl_group - System Controller command groups
>   *
>   * @XE_SYSCTRL_GROUP_GFSP: GFSP group
> + * @XE_SYSCTRL_GROUP_DIAG: Diag group
>   * @XE_SYSCTRL_GROUP_CORE: Core group
>   */
>  enum xe_sysctrl_group {
>  	XE_SYSCTRL_GROUP_GFSP			= 0x01,
> +	XE_SYSCTRL_GROUP_DIAG			= 0x02,
>  	XE_SYSCTRL_GROUP_CORE			= 0xFF,
>  };
>  
> @@ -55,6 +57,35 @@ enum xe_sysctrl_core_cmd {
>  	XE_SYSCTRL_CMD_GET_APP_STATUS_BY_ID		= 0x05,
>  };
>  
> +/**
> + * enum xe_sysctrl_diag_cmd - Commands supported by Diag group
> + *
> + * @XE_SYSCTRL_CMD_DIAG_RAS_ERR_INJECT: RAS error injection
> + */
> +enum xe_sysctrl_diag_cmd {
> +	XE_SYSCTRL_CMD_DIAG_RAS_ERR_INJECT		= 0x7E,
> +};
> +
> +/**
> + * struct xe_sysctrl_diag_ras_err_inj_req - DIAG_RAS_ERR_INJECT request payload
> + *
> + * Request payload for XE_SYSCTRL_CMD_DIAG_RAS_ERR_INJECT. The mailbox layer
> + * prepends the application message header before sending.
> + *
> + * @ras_block_id: RAS block (subsystem) to inject the error into
> + * @ras_sub_block_id: RAS sub-block (IP) within @ras_block_id
> + * @err_type: Type of test error to inject
> + * @reserved: Must be zero
> + * @params: Optional injection parameters (default 0)
> + */
> +struct xe_sysctrl_diag_ras_err_inj_req {
> +	u16 ras_block_id;
> +	u16 ras_sub_block_id;
> +	u16 err_type;
> +	u16 reserved;
> +	u32 params;
> +} __packed;
> +
>  /**
>   * struct xe_sysctrl_app_status_req - Get application status request
>   *
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_types.h b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> index 9ad3c40de97a..8ea6e1f29ddd 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_types.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> @@ -68,6 +68,9 @@ struct xe_sysctrl {
>  
>  		/** @debugfs.loopback: Loopback test entry */
>  		struct xe_sysctrl_debugfs_entry loopback;
> +
> +		/** @debugfs.ras_error_inject: RAS error injection test entry */
> +		struct xe_sysctrl_debugfs_entry ras_error_inject;
>  	} debugfs;
>  };
>  
> -- 
> 2.43.0
>