* [PATCH v2 1/2] drm/xe: add a new debugfs file - mmio
@ 2023-12-20 18:34 Koby Elbaz
2023-12-20 18:34 ` [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file Koby Elbaz
0 siblings, 1 reply; 5+ messages in thread
From: Koby Elbaz @ 2023-12-20 18:34 UTC (permalink / raw
To: intel-xe; +Cc: daniel, rodrigo.vivi
Now that the former mmio handlers are fully removed, this
debugfs file will be used for mmio access (read/write)
through the debugfs ioctl file operation.
Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
---
drivers/gpu/drm/xe/xe_debugfs.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
index c56fd7d59f05..c24637ff09d8 100644
--- a/drivers/gpu/drm/xe/xe_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_debugfs.c
@@ -97,12 +97,22 @@ static int forcewake_release(struct inode *inode, struct file *file)
return 0;
}
+static long xe_debugfs_mmio_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+ return 0;
+}
+
static const struct file_operations forcewake_all_fops = {
.owner = THIS_MODULE,
.open = forcewake_open,
.release = forcewake_release,
};
+static const struct file_operations mmio_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = xe_debugfs_mmio_ioctl,
+};
+
void xe_debugfs_register(struct xe_device *xe)
{
struct ttm_device *bdev = &xe->ttm;
@@ -141,6 +151,8 @@ void xe_debugfs_register(struct xe_device *xe)
for_each_gt(gt, xe, id)
xe_gt_debugfs_register(gt);
+ debugfs_create_file("mmio", 0644, root, xe, &mmio_fops);
+
#ifdef CONFIG_FAULT_INJECTION
fault_create_debugfs_attr("fail_gt_reset", root, >_reset_failure);
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file
2023-12-20 18:34 [PATCH v2 1/2] drm/xe: add a new debugfs file - mmio Koby Elbaz
@ 2023-12-20 18:34 ` Koby Elbaz
2023-12-23 11:24 ` Ofir Bitton
2023-12-24 0:26 ` Souza, Jose
0 siblings, 2 replies; 5+ messages in thread
From: Koby Elbaz @ 2023-12-20 18:34 UTC (permalink / raw
To: intel-xe; +Cc: daniel, rodrigo.vivi
The drm mmio ioctl handler (xe_mmio_ioctl) wasn't needed anymore,
and therefore, recently removed.
Nevertheless, it is now restored for debugging purposes to function
as the ioctl handler of the 'mmio' debugfs file.
Note, that the non-admin user's limited mmio access (aka, whitelist),
was removed being irrelevant anymore, now that a user using debugfs
must anyway have root permission to use it.
Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
---
Changes in v2: removed whitelist support for non-admin users
drivers/gpu/drm/xe/xe_debugfs.c | 107 ++++++++++++++++++++++++++-
drivers/gpu/drm/xe/xe_debugfs_mmio.h | 44 +++++++++++
2 files changed, 150 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/xe/xe_debugfs_mmio.h
diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
index c24637ff09d8..a4381e3d7d37 100644
--- a/drivers/gpu/drm/xe/xe_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_debugfs.c
@@ -13,6 +13,10 @@
#include "xe_device.h"
#include "xe_gt_debugfs.h"
#include "xe_step.h"
+#include "xe_debugfs_mmio.h"
+#include "xe_mmio.h"
+#include "regs/xe_regs.h"
+#include "regs/xe_engine_regs.h"
#ifdef CONFIG_DRM_XE_DEBUG
#include "xe_bo_evict.h"
@@ -97,8 +101,109 @@ static int forcewake_release(struct inode *inode, struct file *file)
return 0;
}
-static long xe_debugfs_mmio_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+static int _xe_debugfs_mmio_ioctl(struct xe_device *xe, struct xe_debugfs_mmio_ioctl_data *args)
{
+ struct xe_gt *gt = xe_root_mmio_gt(xe);
+ unsigned int bits_flag, bytes;
+ struct xe_reg reg;
+ int ret = 0;
+
+ if (XE_IOCTL_DBG(xe, args->extensions) ||
+ XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
+ return -EINVAL;
+
+ if (XE_IOCTL_DBG(xe, args->flags & ~XE_DEBUGFS_MMIO_VALID_FLAGS))
+ return -EINVAL;
+
+ if (XE_IOCTL_DBG(xe, !(args->flags & XE_DEBUGFS_MMIO_WRITE) && args->value))
+ return -EINVAL;
+
+ bits_flag = args->flags & XE_DEBUGFS_MMIO_BITS_MASK;
+ bytes = 1 << bits_flag;
+ if (XE_IOCTL_DBG(xe, args->addr + bytes > xe->mmio.size))
+ return -EINVAL;
+
+ /*
+ * TODO: allow distinction between XE_REG / XE_REG_EXT to
+ * handle registers from both MMIO & MMIO_EXT space.
+ */
+
+ /*
+ * TODO: migrate to xe_gt_mcr to lookup the mmio range and handle
+ * multicast registers. Steering would need uapi extension.
+ */
+ reg = XE_REG(args->addr);
+
+ xe_device_mem_access_get(xe);
+ xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+
+ if (args->flags & XE_DEBUGFS_MMIO_WRITE) {
+ switch (bits_flag) {
+ case XE_DEBUGFS_MMIO_32BIT:
+ if (XE_IOCTL_DBG(xe, args->value > U32_MAX)) {
+ ret = -EINVAL;
+ goto exit;
+ }
+ xe_mmio_write32(gt, reg, args->value);
+ break;
+ default:
+ drm_dbg(&xe->drm, "Invalid MMIO bit size");
+ fallthrough;
+ case XE_DEBUGFS_MMIO_8BIT: /* TODO */
+ case XE_DEBUGFS_MMIO_16BIT: /* TODO */
+ ret = -EOPNOTSUPP;
+ goto exit;
+ }
+ }
+
+ if (args->flags & XE_DEBUGFS_MMIO_READ) {
+ switch (bits_flag) {
+ case XE_DEBUGFS_MMIO_32BIT:
+ args->value = xe_mmio_read32(gt, reg);
+ break;
+ case XE_DEBUGFS_MMIO_64BIT:
+ args->value = xe_mmio_read64_2x32(gt, reg);
+ break;
+ default:
+ drm_dbg(&xe->drm, "Invalid MMIO bit size");
+ fallthrough;
+ case XE_DEBUGFS_MMIO_8BIT: /* TODO */
+ case XE_DEBUGFS_MMIO_16BIT: /* TODO */
+ ret = -EOPNOTSUPP;
+ }
+ }
+
+exit:
+ xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+ xe_device_mem_access_put(xe);
+
+ return ret;
+}
+
+static long xe_debugfs_mmio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ struct xe_device *xe = file_inode(filp)->i_private;
+ struct xe_debugfs_mmio_ioctl_data mmio_data;
+ int ret;
+
+ if (cmd == XE_DEBUGFS_MMIO_IOCTL) {
+ ret = copy_from_user(&mmio_data, (struct xe_debugfs_mmio_ioctl_data __user *)arg,
+ sizeof(mmio_data));
+ if (ret)
+ return -EFAULT;
+
+ ret = _xe_debugfs_mmio_ioctl(xe, &mmio_data);
+ if (ret)
+ return ret;
+
+ ret = copy_to_user((struct xe_debugfs_mmio_ioctl_data __user *)arg,
+ &mmio_data, sizeof(mmio_data));
+ if (ret)
+ return -EFAULT;
+ } else {
+ return -EINVAL;
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/xe/xe_debugfs_mmio.h b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
new file mode 100644
index 000000000000..52590b5aca0e
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef _XE_DEBUGFS_MMIO_H_
+#define _XE_DEBUGFS_MMIO_H_
+
+#include <drm/drm.h>
+
+#define XE_DEBUGFS_MMIO_8BIT 0x0
+#define XE_DEBUGFS_MMIO_16BIT 0x1
+#define XE_DEBUGFS_MMIO_32BIT 0x2
+#define XE_DEBUGFS_MMIO_64BIT 0x3
+#define XE_DEBUGFS_MMIO_BITS_MASK 0x3
+#define XE_DEBUGFS_MMIO_READ 0x4
+#define XE_DEBUGFS_MMIO_WRITE 0x8
+
+#define XE_DEBUGFS_MMIO_VALID_FLAGS (\
+ XE_DEBUGFS_MMIO_BITS_MASK |\
+ XE_DEBUGFS_MMIO_READ |\
+ XE_DEBUGFS_MMIO_WRITE)
+
+struct xe_debugfs_mmio_ioctl_data {
+ /** @extensions: Pointer to the first extension struct, if any */
+ __u64 extensions;
+
+ __u32 addr;
+
+ __u32 flags;
+
+ __u64 value;
+
+ /** @reserved: Reserved */
+ __u64 reserved[2];
+};
+
+#define XE_DEBUGFS_MMIO_IOCTL_NR 0x00
+
+#define XE_DEBUGFS_MMIO_IOCTL \
+ _IOWR(0x20, XE_DEBUGFS_MMIO_IOCTL_NR, struct xe_debugfs_mmio_ioctl_data)
+
+
+#endif /* _XE_DEBUGFS_MMIO_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file
2023-12-20 18:34 ` [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file Koby Elbaz
@ 2023-12-23 11:24 ` Ofir Bitton
2023-12-24 0:26 ` Souza, Jose
1 sibling, 0 replies; 5+ messages in thread
From: Ofir Bitton @ 2023-12-23 11:24 UTC (permalink / raw
To: Koby Elbaz, intel-xe@lists.freedesktop.org
Cc: daniel@ffwll.ch, rodrigo.vivi@intel.com
On 20/12/2023 20:34, Koby Elbaz wrote:
> The drm mmio ioctl handler (xe_mmio_ioctl) wasn't needed anymore,
> and therefore, recently removed.
> Nevertheless, it is now restored for debugging purposes to function
> as the ioctl handler of the 'mmio' debugfs file.
> Note, that the non-admin user's limited mmio access (aka, whitelist),
> was removed being irrelevant anymore, now that a user using debugfs
> must anyway have root permission to use it.
>
> Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
> ---
> Changes in v2: removed whitelist support for non-admin users
>
> drivers/gpu/drm/xe/xe_debugfs.c | 107 ++++++++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_debugfs_mmio.h | 44 +++++++++++
> 2 files changed, 150 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/xe/xe_debugfs_mmio.h
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index c24637ff09d8..a4381e3d7d37 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -13,6 +13,10 @@
> #include "xe_device.h"
> #include "xe_gt_debugfs.h"
> #include "xe_step.h"
> +#include "xe_debugfs_mmio.h"
> +#include "xe_mmio.h"
> +#include "regs/xe_regs.h"
> +#include "regs/xe_engine_regs.h"
>
> #ifdef CONFIG_DRM_XE_DEBUG
> #include "xe_bo_evict.h"
> @@ -97,8 +101,109 @@ static int forcewake_release(struct inode *inode, struct file *file)
> return 0;
> }
>
> -static long xe_debugfs_mmio_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> +static int _xe_debugfs_mmio_ioctl(struct xe_device *xe, struct xe_debugfs_mmio_ioctl_data *args)
> {
> + struct xe_gt *gt = xe_root_mmio_gt(xe);
> + unsigned int bits_flag, bytes;
> + struct xe_reg reg;
> + int ret = 0;
> +
> + if (XE_IOCTL_DBG(xe, args->extensions) ||
> + XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
> + return -EINVAL;
> +
> + if (XE_IOCTL_DBG(xe, args->flags & ~XE_DEBUGFS_MMIO_VALID_FLAGS))
> + return -EINVAL;
> +
> + if (XE_IOCTL_DBG(xe, !(args->flags & XE_DEBUGFS_MMIO_WRITE) && args->value))
> + return -EINVAL;
> +
> + bits_flag = args->flags & XE_DEBUGFS_MMIO_BITS_MASK;
> + bytes = 1 << bits_flag;
> + if (XE_IOCTL_DBG(xe, args->addr + bytes > xe->mmio.size))
> + return -EINVAL;
> +
> + /*
> + * TODO: allow distinction between XE_REG / XE_REG_EXT to
> + * handle registers from both MMIO & MMIO_EXT space.
> + */
> +
> + /*
> + * TODO: migrate to xe_gt_mcr to lookup the mmio range and handle
> + * multicast registers. Steering would need uapi extension.
> + */
> + reg = XE_REG(args->addr);
> +
> + xe_device_mem_access_get(xe);
> + xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +
> + if (args->flags & XE_DEBUGFS_MMIO_WRITE) {
> + switch (bits_flag) {
> + case XE_DEBUGFS_MMIO_32BIT:
> + if (XE_IOCTL_DBG(xe, args->value > U32_MAX)) {
> + ret = -EINVAL;
> + goto exit;
> + }
> + xe_mmio_write32(gt, reg, args->value);
> + break;
> + default:
> + drm_dbg(&xe->drm, "Invalid MMIO bit size");
> + fallthrough;
> + case XE_DEBUGFS_MMIO_8BIT: /* TODO */
> + case XE_DEBUGFS_MMIO_16BIT: /* TODO */
> + ret = -EOPNOTSUPP;
> + goto exit;
> + }
> + }
> +
> + if (args->flags & XE_DEBUGFS_MMIO_READ) {
> + switch (bits_flag) {
> + case XE_DEBUGFS_MMIO_32BIT:
> + args->value = xe_mmio_read32(gt, reg);
> + break;
> + case XE_DEBUGFS_MMIO_64BIT:
> + args->value = xe_mmio_read64_2x32(gt, reg);
> + break;
> + default:
> + drm_dbg(&xe->drm, "Invalid MMIO bit size");
> + fallthrough;
> + case XE_DEBUGFS_MMIO_8BIT: /* TODO */
> + case XE_DEBUGFS_MMIO_16BIT: /* TODO */
> + ret = -EOPNOTSUPP;
> + }
> + }
> +
> +exit:
> + xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> + xe_device_mem_access_put(xe);
> +
> + return ret;
> +}
> +
> +static long xe_debugfs_mmio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> + struct xe_device *xe = file_inode(filp)->i_private;
> + struct xe_debugfs_mmio_ioctl_data mmio_data;
> + int ret;
> +
> + if (cmd == XE_DEBUGFS_MMIO_IOCTL) {
> + ret = copy_from_user(&mmio_data, (struct xe_debugfs_mmio_ioctl_data __user *)arg,
> + sizeof(mmio_data));
> + if (ret)
> + return -EFAULT;
> +
> + ret = _xe_debugfs_mmio_ioctl(xe, &mmio_data);
> + if (ret)
> + return ret;
> +
> + ret = copy_to_user((struct xe_debugfs_mmio_ioctl_data __user *)arg,
> + &mmio_data, sizeof(mmio_data));
> + if (ret)
> + return -EFAULT;
> + } else {
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs_mmio.h b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
> new file mode 100644
> index 000000000000..52590b5aca0e
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_DEBUGFS_MMIO_H_
> +#define _XE_DEBUGFS_MMIO_H_
> +
> +#include <drm/drm.h>
> +
> +#define XE_DEBUGFS_MMIO_8BIT 0x0
> +#define XE_DEBUGFS_MMIO_16BIT 0x1
> +#define XE_DEBUGFS_MMIO_32BIT 0x2
> +#define XE_DEBUGFS_MMIO_64BIT 0x3
> +#define XE_DEBUGFS_MMIO_BITS_MASK 0x3
> +#define XE_DEBUGFS_MMIO_READ 0x4
> +#define XE_DEBUGFS_MMIO_WRITE 0x8
> +
> +#define XE_DEBUGFS_MMIO_VALID_FLAGS (\
> + XE_DEBUGFS_MMIO_BITS_MASK |\
> + XE_DEBUGFS_MMIO_READ |\
> + XE_DEBUGFS_MMIO_WRITE)
> +
> +struct xe_debugfs_mmio_ioctl_data {
> + /** @extensions: Pointer to the first extension struct, if any */
> + __u64 extensions;
> +
> + __u32 addr;
> +
> + __u32 flags;
> +
> + __u64 value;
> +
> + /** @reserved: Reserved */
> + __u64 reserved[2];
> +};
> +
> +#define XE_DEBUGFS_MMIO_IOCTL_NR 0x00
> +
> +#define XE_DEBUGFS_MMIO_IOCTL \
> + _IOWR(0x20, XE_DEBUGFS_MMIO_IOCTL_NR, struct xe_debugfs_mmio_ioctl_data)
> +
> +
> +#endif /* _XE_DEBUGFS_MMIO_H_ */
For both patches.
Reviewed-by: Ofir Bitton <obitton@habana.ai>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file
2023-12-20 18:34 ` [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file Koby Elbaz
2023-12-23 11:24 ` Ofir Bitton
@ 2023-12-24 0:26 ` Souza, Jose
2023-12-25 12:33 ` Koby Elbaz
1 sibling, 1 reply; 5+ messages in thread
From: Souza, Jose @ 2023-12-24 0:26 UTC (permalink / raw
To: intel-xe@lists.freedesktop.org, Elbaz, Koby (Habana)
Cc: daniel@ffwll.ch, Vivi, Rodrigo
On Wed, 2023-12-20 at 20:34 +0200, Koby Elbaz wrote:
> The drm mmio ioctl handler (xe_mmio_ioctl) wasn't needed anymore,
> and therefore, recently removed.
> Nevertheless, it is now restored for debugging purposes to function
> as the ioctl handler of the 'mmio' debugfs file.
> Note, that the non-admin user's limited mmio access (aka, whitelist),
> was removed being irrelevant anymore, now that a user using debugfs
> must anyway have root permission to use it.
you can use a IGT tool called intel_reg that does what you want without adding any line to Xe driver.
>
> Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
> ---
> Changes in v2: removed whitelist support for non-admin users
>
> drivers/gpu/drm/xe/xe_debugfs.c | 107 ++++++++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_debugfs_mmio.h | 44 +++++++++++
> 2 files changed, 150 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/xe/xe_debugfs_mmio.h
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index c24637ff09d8..a4381e3d7d37 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -13,6 +13,10 @@
> #include "xe_device.h"
> #include "xe_gt_debugfs.h"
> #include "xe_step.h"
> +#include "xe_debugfs_mmio.h"
> +#include "xe_mmio.h"
> +#include "regs/xe_regs.h"
> +#include "regs/xe_engine_regs.h"
>
> #ifdef CONFIG_DRM_XE_DEBUG
> #include "xe_bo_evict.h"
> @@ -97,8 +101,109 @@ static int forcewake_release(struct inode *inode, struct file *file)
> return 0;
> }
>
> -static long xe_debugfs_mmio_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> +static int _xe_debugfs_mmio_ioctl(struct xe_device *xe, struct xe_debugfs_mmio_ioctl_data *args)
> {
> + struct xe_gt *gt = xe_root_mmio_gt(xe);
> + unsigned int bits_flag, bytes;
> + struct xe_reg reg;
> + int ret = 0;
> +
> + if (XE_IOCTL_DBG(xe, args->extensions) ||
> + XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
> + return -EINVAL;
> +
> + if (XE_IOCTL_DBG(xe, args->flags & ~XE_DEBUGFS_MMIO_VALID_FLAGS))
> + return -EINVAL;
> +
> + if (XE_IOCTL_DBG(xe, !(args->flags & XE_DEBUGFS_MMIO_WRITE) && args->value))
> + return -EINVAL;
> +
> + bits_flag = args->flags & XE_DEBUGFS_MMIO_BITS_MASK;
> + bytes = 1 << bits_flag;
> + if (XE_IOCTL_DBG(xe, args->addr + bytes > xe->mmio.size))
> + return -EINVAL;
> +
> + /*
> + * TODO: allow distinction between XE_REG / XE_REG_EXT to
> + * handle registers from both MMIO & MMIO_EXT space.
> + */
> +
> + /*
> + * TODO: migrate to xe_gt_mcr to lookup the mmio range and handle
> + * multicast registers. Steering would need uapi extension.
> + */
> + reg = XE_REG(args->addr);
> +
> + xe_device_mem_access_get(xe);
> + xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +
> + if (args->flags & XE_DEBUGFS_MMIO_WRITE) {
> + switch (bits_flag) {
> + case XE_DEBUGFS_MMIO_32BIT:
> + if (XE_IOCTL_DBG(xe, args->value > U32_MAX)) {
> + ret = -EINVAL;
> + goto exit;
> + }
> + xe_mmio_write32(gt, reg, args->value);
> + break;
> + default:
> + drm_dbg(&xe->drm, "Invalid MMIO bit size");
> + fallthrough;
> + case XE_DEBUGFS_MMIO_8BIT: /* TODO */
> + case XE_DEBUGFS_MMIO_16BIT: /* TODO */
> + ret = -EOPNOTSUPP;
> + goto exit;
> + }
> + }
> +
> + if (args->flags & XE_DEBUGFS_MMIO_READ) {
> + switch (bits_flag) {
> + case XE_DEBUGFS_MMIO_32BIT:
> + args->value = xe_mmio_read32(gt, reg);
> + break;
> + case XE_DEBUGFS_MMIO_64BIT:
> + args->value = xe_mmio_read64_2x32(gt, reg);
> + break;
> + default:
> + drm_dbg(&xe->drm, "Invalid MMIO bit size");
> + fallthrough;
> + case XE_DEBUGFS_MMIO_8BIT: /* TODO */
> + case XE_DEBUGFS_MMIO_16BIT: /* TODO */
> + ret = -EOPNOTSUPP;
> + }
> + }
> +
> +exit:
> + xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> + xe_device_mem_access_put(xe);
> +
> + return ret;
> +}
> +
> +static long xe_debugfs_mmio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> + struct xe_device *xe = file_inode(filp)->i_private;
> + struct xe_debugfs_mmio_ioctl_data mmio_data;
> + int ret;
> +
> + if (cmd == XE_DEBUGFS_MMIO_IOCTL) {
> + ret = copy_from_user(&mmio_data, (struct xe_debugfs_mmio_ioctl_data __user *)arg,
> + sizeof(mmio_data));
> + if (ret)
> + return -EFAULT;
> +
> + ret = _xe_debugfs_mmio_ioctl(xe, &mmio_data);
> + if (ret)
> + return ret;
> +
> + ret = copy_to_user((struct xe_debugfs_mmio_ioctl_data __user *)arg,
> + &mmio_data, sizeof(mmio_data));
> + if (ret)
> + return -EFAULT;
> + } else {
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs_mmio.h b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
> new file mode 100644
> index 000000000000..52590b5aca0e
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_DEBUGFS_MMIO_H_
> +#define _XE_DEBUGFS_MMIO_H_
> +
> +#include <drm/drm.h>
> +
> +#define XE_DEBUGFS_MMIO_8BIT 0x0
> +#define XE_DEBUGFS_MMIO_16BIT 0x1
> +#define XE_DEBUGFS_MMIO_32BIT 0x2
> +#define XE_DEBUGFS_MMIO_64BIT 0x3
> +#define XE_DEBUGFS_MMIO_BITS_MASK 0x3
> +#define XE_DEBUGFS_MMIO_READ 0x4
> +#define XE_DEBUGFS_MMIO_WRITE 0x8
> +
> +#define XE_DEBUGFS_MMIO_VALID_FLAGS (\
> + XE_DEBUGFS_MMIO_BITS_MASK |\
> + XE_DEBUGFS_MMIO_READ |\
> + XE_DEBUGFS_MMIO_WRITE)
> +
> +struct xe_debugfs_mmio_ioctl_data {
> + /** @extensions: Pointer to the first extension struct, if any */
> + __u64 extensions;
> +
> + __u32 addr;
> +
> + __u32 flags;
> +
> + __u64 value;
> +
> + /** @reserved: Reserved */
> + __u64 reserved[2];
> +};
> +
> +#define XE_DEBUGFS_MMIO_IOCTL_NR 0x00
> +
> +#define XE_DEBUGFS_MMIO_IOCTL \
> + _IOWR(0x20, XE_DEBUGFS_MMIO_IOCTL_NR, struct xe_debugfs_mmio_ioctl_data)
> +
> +
> +#endif /* _XE_DEBUGFS_MMIO_H_ */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file
2023-12-24 0:26 ` Souza, Jose
@ 2023-12-25 12:33 ` Koby Elbaz
0 siblings, 0 replies; 5+ messages in thread
From: Koby Elbaz @ 2023-12-25 12:33 UTC (permalink / raw
To: Souza, Jose, intel-xe@lists.freedesktop.org
Cc: Bitton, Ofir1, daniel@ffwll.ch, Vivi, Rodrigo
On 24/12/2023 2:26, Souza, Jose wrote:
> On Wed, 2023-12-20 at 20:34 +0200, Koby Elbaz wrote:
>> The drm mmio ioctl handler (xe_mmio_ioctl) wasn't needed anymore,
>> and therefore, recently removed.
>> Nevertheless, it is now restored for debugging purposes to function
>> as the ioctl handler of the 'mmio' debugfs file.
>> Note, that the non-admin user's limited mmio access (aka, whitelist),
>> was removed being irrelevant anymore, now that a user using debugfs
>> must anyway have root permission to use it.
> you can use a IGT tool called intel_reg that does what you want without adding any line to Xe driver.
>
To some extent we can, however, it does not satisfy our development needs.
Moreover, this change was recently agreed upon throughout the removal of
the former mmio-ioctl code:
https://patchwork.freedesktop.org/series/123403/
Koby Elbaz
>> Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
>> ---
>> Changes in v2: removed whitelist support for non-admin users
>>
>> drivers/gpu/drm/xe/xe_debugfs.c | 107 ++++++++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_debugfs_mmio.h | 44 +++++++++++
>> 2 files changed, 150 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/xe/xe_debugfs_mmio.h
>>
>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
>> index c24637ff09d8..a4381e3d7d37 100644
>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>> @@ -13,6 +13,10 @@
>> #include "xe_device.h"
>> #include "xe_gt_debugfs.h"
>> #include "xe_step.h"
>> +#include "xe_debugfs_mmio.h"
>> +#include "xe_mmio.h"
>> +#include "regs/xe_regs.h"
>> +#include "regs/xe_engine_regs.h"
>>
>> #ifdef CONFIG_DRM_XE_DEBUG
>> #include "xe_bo_evict.h"
>> @@ -97,8 +101,109 @@ static int forcewake_release(struct inode *inode, struct file *file)
>> return 0;
>> }
>>
>> -static long xe_debugfs_mmio_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>> +static int _xe_debugfs_mmio_ioctl(struct xe_device *xe, struct xe_debugfs_mmio_ioctl_data *args)
>> {
>> + struct xe_gt *gt = xe_root_mmio_gt(xe);
>> + unsigned int bits_flag, bytes;
>> + struct xe_reg reg;
>> + int ret = 0;
>> +
>> + if (XE_IOCTL_DBG(xe, args->extensions) ||
>> + XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
>> + return -EINVAL;
>> +
>> + if (XE_IOCTL_DBG(xe, args->flags & ~XE_DEBUGFS_MMIO_VALID_FLAGS))
>> + return -EINVAL;
>> +
>> + if (XE_IOCTL_DBG(xe, !(args->flags & XE_DEBUGFS_MMIO_WRITE) && args->value))
>> + return -EINVAL;
>> +
>> + bits_flag = args->flags & XE_DEBUGFS_MMIO_BITS_MASK;
>> + bytes = 1 << bits_flag;
>> + if (XE_IOCTL_DBG(xe, args->addr + bytes > xe->mmio.size))
>> + return -EINVAL;
>> +
>> + /*
>> + * TODO: allow distinction between XE_REG / XE_REG_EXT to
>> + * handle registers from both MMIO & MMIO_EXT space.
>> + */
>> +
>> + /*
>> + * TODO: migrate to xe_gt_mcr to lookup the mmio range and handle
>> + * multicast registers. Steering would need uapi extension.
>> + */
>> + reg = XE_REG(args->addr);
>> +
>> + xe_device_mem_access_get(xe);
>> + xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> +
>> + if (args->flags & XE_DEBUGFS_MMIO_WRITE) {
>> + switch (bits_flag) {
>> + case XE_DEBUGFS_MMIO_32BIT:
>> + if (XE_IOCTL_DBG(xe, args->value > U32_MAX)) {
>> + ret = -EINVAL;
>> + goto exit;
>> + }
>> + xe_mmio_write32(gt, reg, args->value);
>> + break;
>> + default:
>> + drm_dbg(&xe->drm, "Invalid MMIO bit size");
>> + fallthrough;
>> + case XE_DEBUGFS_MMIO_8BIT: /* TODO */
>> + case XE_DEBUGFS_MMIO_16BIT: /* TODO */
>> + ret = -EOPNOTSUPP;
>> + goto exit;
>> + }
>> + }
>> +
>> + if (args->flags & XE_DEBUGFS_MMIO_READ) {
>> + switch (bits_flag) {
>> + case XE_DEBUGFS_MMIO_32BIT:
>> + args->value = xe_mmio_read32(gt, reg);
>> + break;
>> + case XE_DEBUGFS_MMIO_64BIT:
>> + args->value = xe_mmio_read64_2x32(gt, reg);
>> + break;
>> + default:
>> + drm_dbg(&xe->drm, "Invalid MMIO bit size");
>> + fallthrough;
>> + case XE_DEBUGFS_MMIO_8BIT: /* TODO */
>> + case XE_DEBUGFS_MMIO_16BIT: /* TODO */
>> + ret = -EOPNOTSUPP;
>> + }
>> + }
>> +
>> +exit:
>> + xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> + xe_device_mem_access_put(xe);
>> +
>> + return ret;
>> +}
>> +
>> +static long xe_debugfs_mmio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>> +{
>> + struct xe_device *xe = file_inode(filp)->i_private;
>> + struct xe_debugfs_mmio_ioctl_data mmio_data;
>> + int ret;
>> +
>> + if (cmd == XE_DEBUGFS_MMIO_IOCTL) {
>> + ret = copy_from_user(&mmio_data, (struct xe_debugfs_mmio_ioctl_data __user *)arg,
>> + sizeof(mmio_data));
>> + if (ret)
>> + return -EFAULT;
>> +
>> + ret = _xe_debugfs_mmio_ioctl(xe, &mmio_data);
>> + if (ret)
>> + return ret;
>> +
>> + ret = copy_to_user((struct xe_debugfs_mmio_ioctl_data __user *)arg,
>> + &mmio_data, sizeof(mmio_data));
>> + if (ret)
>> + return -EFAULT;
>> + } else {
>> + return -EINVAL;
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/xe/xe_debugfs_mmio.h b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
>> new file mode 100644
>> index 000000000000..52590b5aca0e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_debugfs_mmio.h
>> @@ -0,0 +1,44 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_DEBUGFS_MMIO_H_
>> +#define _XE_DEBUGFS_MMIO_H_
>> +
>> +#include <drm/drm.h>
>> +
>> +#define XE_DEBUGFS_MMIO_8BIT 0x0
>> +#define XE_DEBUGFS_MMIO_16BIT 0x1
>> +#define XE_DEBUGFS_MMIO_32BIT 0x2
>> +#define XE_DEBUGFS_MMIO_64BIT 0x3
>> +#define XE_DEBUGFS_MMIO_BITS_MASK 0x3
>> +#define XE_DEBUGFS_MMIO_READ 0x4
>> +#define XE_DEBUGFS_MMIO_WRITE 0x8
>> +
>> +#define XE_DEBUGFS_MMIO_VALID_FLAGS (\
>> + XE_DEBUGFS_MMIO_BITS_MASK |\
>> + XE_DEBUGFS_MMIO_READ |\
>> + XE_DEBUGFS_MMIO_WRITE)
>> +
>> +struct xe_debugfs_mmio_ioctl_data {
>> + /** @extensions: Pointer to the first extension struct, if any */
>> + __u64 extensions;
>> +
>> + __u32 addr;
>> +
>> + __u32 flags;
>> +
>> + __u64 value;
>> +
>> + /** @reserved: Reserved */
>> + __u64 reserved[2];
>> +};
>> +
>> +#define XE_DEBUGFS_MMIO_IOCTL_NR 0x00
>> +
>> +#define XE_DEBUGFS_MMIO_IOCTL \
>> + _IOWR(0x20, XE_DEBUGFS_MMIO_IOCTL_NR, struct xe_debugfs_mmio_ioctl_data)
>> +
>> +
>> +#endif /* _XE_DEBUGFS_MMIO_H_ */
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-25 13:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-20 18:34 [PATCH v2 1/2] drm/xe: add a new debugfs file - mmio Koby Elbaz
2023-12-20 18:34 ` [PATCH v2 2/2] drm/xe: restore xe_mmio_ioctl as the ioctl handler of the mmio debugfs file Koby Elbaz
2023-12-23 11:24 ` Ofir Bitton
2023-12-24 0:26 ` Souza, Jose
2023-12-25 12:33 ` Koby Elbaz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.