All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Add debugfs info for the AMD IOMMU
@ 2018-03-14 23:04 ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:04 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

The following series creates a debugfs directory for AMD IOMMUs,
constructs a framework for additional entries, an online README,
and a method for dumping device table entries. Data is reported
in a default concise mode, but a verbose mode is enabled via a
filesystem entry.

This is the first of three patch series that will expose a number
of IOMMU registers.

Changes since v2:
- Change lock type to a mutex
- Convert a #define to an inline
- Alphabetize #include files
- Remove unnecessary checks for pointers
- Use kstrtoint() instead of sscanf()
- Added comments
- Minor style fixes

Changes since v1:
- Correctly use CONFIG_AMD_IOMMU_DEBUG in Makefile and header file

---

Gary R Hook (5):
      iommu/amd - Add debugfs support
      iommu/amd - Add a 'verbose' switch for IOMMU debugfs
      iommu/amd - Add a README variable for the IOMMU debugfs
      iommu/amd - Expose the active IOMMU device table entries
      iommu/amd - Add a debugfs entry to specify a IOMMU device table entry


 drivers/iommu/Kconfig             |    7 +
 drivers/iommu/Makefile            |    1 
 drivers/iommu/amd_iommu_debugfs.c |  316 +++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c    |    6 -
 drivers/iommu/amd_iommu_proto.h   |    7 +
 drivers/iommu/amd_iommu_types.h   |    3 
 6 files changed, 338 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iommu/amd_iommu_debugfs.c

--

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v3 0/5] Add debugfs info for the AMD IOMMU
@ 2018-03-14 23:04 ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:04 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

The following series creates a debugfs directory for AMD IOMMUs,
constructs a framework for additional entries, an online README,
and a method for dumping device table entries. Data is reported
in a default concise mode, but a verbose mode is enabled via a
filesystem entry.

This is the first of three patch series that will expose a number
of IOMMU registers.

Changes since v2:
- Change lock type to a mutex
- Convert a #define to an inline
- Alphabetize #include files
- Remove unnecessary checks for pointers
- Use kstrtoint() instead of sscanf()
- Added comments
- Minor style fixes

Changes since v1:
- Correctly use CONFIG_AMD_IOMMU_DEBUG in Makefile and header file

---

Gary R Hook (5):
      iommu/amd - Add debugfs support
      iommu/amd - Add a 'verbose' switch for IOMMU debugfs
      iommu/amd - Add a README variable for the IOMMU debugfs
      iommu/amd - Expose the active IOMMU device table entries
      iommu/amd - Add a debugfs entry to specify a IOMMU device table entry


 drivers/iommu/Kconfig             |    7 +
 drivers/iommu/Makefile            |    1 
 drivers/iommu/amd_iommu_debugfs.c |  316 +++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c    |    6 -
 drivers/iommu/amd_iommu_proto.h   |    7 +
 drivers/iommu/amd_iommu_types.h   |    3 
 6 files changed, 338 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iommu/amd_iommu_debugfs.c

--

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v3 1/5] iommu/amd - Add debugfs support
@ 2018-03-14 23:04   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:04 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

Expose the IOMMU MMIO registers and device table

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/iommu/Kconfig             |    7 ++
 drivers/iommu/Makefile            |    1 
 drivers/iommu/amd_iommu_debugfs.c |  122 +++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c    |    6 +-
 drivers/iommu/amd_iommu_proto.h   |    7 ++
 drivers/iommu/amd_iommu_types.h   |    3 +
 6 files changed, 144 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iommu/amd_iommu_debugfs.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index f3a21343e636..8b2a5b8707c6 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -135,6 +135,13 @@ config AMD_IOMMU_V2
 	  hardware. Select this option if you want to use devices that support
 	  the PCI PRI and PASID interface.
 
+config AMD_IOMMU_DEBUG
+	bool "Expose AMD IOMMU internals in DebugFS"
+	depends on AMD_IOMMU && DEBUG_FS
+	help
+	  Provides debugfs access to IOMMU data such as registers and device
+	  table entries.
+
 # Intel IOMMU support
 config DMAR_TABLE
 	bool
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb695854809..64fba8b1ca4f 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
 obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
 obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
+obj-$(CONFIG_AMD_IOMMU_DEBUG) += amd_iommu_debugfs.o
 obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
 obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
new file mode 100644
index 000000000000..4f0f05a89a41
--- /dev/null
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD IOMMU driver
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ *
+ * Author: Gary R Hook <gary.hook@amd.com>
+ */
+
+#ifdef	CONFIG_DEBUG_FS
+
+#include <linux/debugfs.h>
+#include <linux/iommu.h>
+#include <linux/pci.h>
+
+#include "amd_iommu_proto.h"
+#include "amd_iommu_types.h"
+
+/* DebugFS helpers
+ *
+ * This is intended to shrink the scnprintf statements used to produce
+ * debugfs output. Each function that uses OSCNPRINTF will need to declare
+ * local variables:
+ *
+ *	unsigned int obuflen = <max size>;
+ *	unsigned int oboff = 0;
+ *	char *obuf;
+ *
+ * wherein obuflen is the expected buffer length needed for the output text.
+ * Every statement then reduces to
+ *	oboff += OSCNPRINTF(<format>[, <arg1>[, ...]]);
+ */
+#define	OBUFP		(obuf + oboff)
+#define	OBUFLEN		obuflen
+#define	OBUFSPC		(OBUFLEN - oboff)
+#define	OSCNPRINTF(fmt, ...) \
+		scnprintf(OBUFP, OBUFSPC, fmt, ## __VA_ARGS__)
+
+static struct dentry *iommu_debugfs_dir;
+static DEFINE_MUTEX(iommu_debugfs_lock);
+
+#define	MAX_NAME_LEN	20
+
+static unsigned int amd_iommu_count_valid_dtes(int start, int end)
+{
+	unsigned int n = 0;
+	int i;
+
+	/* Scan the DTE table from entry 'start' through entry 'end' for
+	 * active entries
+	 */
+	for (i = start ; i <= end ; i++) {
+		if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+		    || amd_iommu_dev_table[i].data[1])
+			n++;
+	}
+	return n;
+}
+
+static ssize_t amd_iommu_debugfs_dtecount_read(struct file *filp,
+					  char __user *ubuf,
+					  size_t count, loff_t *offp)
+{
+	unsigned int obuflen = 64;
+	unsigned int oboff = 0;
+	unsigned int n;
+	ssize_t ret;
+	char *obuf;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	n = amd_iommu_count_valid_dtes(0, 0xFFFF);
+	oboff += OSCNPRINTF("%d\n", n);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+	kfree(obuf);
+
+	return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_dtecount_read,
+	.write = NULL,
+};
+
+void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
+{
+	char name[MAX_NAME_LEN + 1];
+	struct dentry *d_dte;
+
+	if (!debugfs_initialized())
+		return;
+
+	mutex_lock(&iommu_debugfs_lock);
+	if (!iommu_debugfs_dir)
+		iommu_debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
+	mutex_unlock(&iommu_debugfs_lock);
+	if (!iommu_debugfs_dir)
+		goto err;
+
+	snprintf(name, MAX_NAME_LEN, "iommu%02x", iommu->index);
+	iommu->debugfs_instance = debugfs_create_dir(name, iommu_debugfs_dir);
+	if (!iommu->debugfs_instance)
+		goto err;
+
+	d_dte = debugfs_create_file("count", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_dtecount_ops);
+	if (!d_dte)
+		goto err;
+
+	return;
+
+err:
+	debugfs_remove_recursive(iommu->debugfs_instance);
+}
+
+#endif
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 4e4a615bf13f..a03142fd6179 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2721,6 +2721,7 @@ int __init amd_iommu_enable_faulting(void)
  */
 static int __init amd_iommu_init(void)
 {
+	struct amd_iommu *iommu;
 	int ret;
 
 	ret = iommu_go_to_state(IOMMU_INITIALIZED);
@@ -2730,14 +2731,15 @@ static int __init amd_iommu_init(void)
 			disable_iommus();
 			free_iommu_resources();
 		} else {
-			struct amd_iommu *iommu;
-
 			uninit_device_table_dma();
 			for_each_iommu(iommu)
 				iommu_flush_all_caches(iommu);
 		}
 	}
 
+	for_each_iommu(iommu)
+		amd_iommu_debugfs_setup(iommu);
+
 	return ret;
 }
 
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 640c286a0ab9..db5872d3d40f 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -33,6 +33,13 @@ extern void amd_iommu_uninit_devices(void);
 extern void amd_iommu_init_notifier(void);
 extern int amd_iommu_init_api(void);
 
+#ifdef	CONFIG_AMD_IOMMU_DEBUG
+extern void amd_iommu_debugfs_setup(struct amd_iommu *iommu);
+#else
+static inline void amd_iommu_debugfs_setup(struct amd_iommu *iommu) {}
+#endif
+
+
 /* Needed for interrupt remapping */
 extern int amd_iommu_prepare(void);
 extern int amd_iommu_enable(void);
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 6a877ebd058b..43c008dc7ade 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -593,6 +593,9 @@ struct amd_iommu {
 
 	u32 flags;
 	volatile u64 __aligned(8) cmd_sem;
+
+	/* DebugFS Info */
+	struct dentry *debugfs_instance;
 };
 
 static inline struct amd_iommu *dev_to_amd_iommu(struct device *dev)

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 1/5] iommu/amd - Add debugfs support
@ 2018-03-14 23:04   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:04 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Expose the IOMMU MMIO registers and device table

Signed-off-by: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/Kconfig             |    7 ++
 drivers/iommu/Makefile            |    1 
 drivers/iommu/amd_iommu_debugfs.c |  122 +++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c    |    6 +-
 drivers/iommu/amd_iommu_proto.h   |    7 ++
 drivers/iommu/amd_iommu_types.h   |    3 +
 6 files changed, 144 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iommu/amd_iommu_debugfs.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index f3a21343e636..8b2a5b8707c6 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -135,6 +135,13 @@ config AMD_IOMMU_V2
 	  hardware. Select this option if you want to use devices that support
 	  the PCI PRI and PASID interface.
 
+config AMD_IOMMU_DEBUG
+	bool "Expose AMD IOMMU internals in DebugFS"
+	depends on AMD_IOMMU && DEBUG_FS
+	help
+	  Provides debugfs access to IOMMU data such as registers and device
+	  table entries.
+
 # Intel IOMMU support
 config DMAR_TABLE
 	bool
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb695854809..64fba8b1ca4f 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
 obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
 obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
+obj-$(CONFIG_AMD_IOMMU_DEBUG) += amd_iommu_debugfs.o
 obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
 obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
new file mode 100644
index 000000000000..4f0f05a89a41
--- /dev/null
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD IOMMU driver
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ *
+ * Author: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
+ */
+
+#ifdef	CONFIG_DEBUG_FS
+
+#include <linux/debugfs.h>
+#include <linux/iommu.h>
+#include <linux/pci.h>
+
+#include "amd_iommu_proto.h"
+#include "amd_iommu_types.h"
+
+/* DebugFS helpers
+ *
+ * This is intended to shrink the scnprintf statements used to produce
+ * debugfs output. Each function that uses OSCNPRINTF will need to declare
+ * local variables:
+ *
+ *	unsigned int obuflen = <max size>;
+ *	unsigned int oboff = 0;
+ *	char *obuf;
+ *
+ * wherein obuflen is the expected buffer length needed for the output text.
+ * Every statement then reduces to
+ *	oboff += OSCNPRINTF(<format>[, <arg1>[, ...]]);
+ */
+#define	OBUFP		(obuf + oboff)
+#define	OBUFLEN		obuflen
+#define	OBUFSPC		(OBUFLEN - oboff)
+#define	OSCNPRINTF(fmt, ...) \
+		scnprintf(OBUFP, OBUFSPC, fmt, ## __VA_ARGS__)
+
+static struct dentry *iommu_debugfs_dir;
+static DEFINE_MUTEX(iommu_debugfs_lock);
+
+#define	MAX_NAME_LEN	20
+
+static unsigned int amd_iommu_count_valid_dtes(int start, int end)
+{
+	unsigned int n = 0;
+	int i;
+
+	/* Scan the DTE table from entry 'start' through entry 'end' for
+	 * active entries
+	 */
+	for (i = start ; i <= end ; i++) {
+		if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+		    || amd_iommu_dev_table[i].data[1])
+			n++;
+	}
+	return n;
+}
+
+static ssize_t amd_iommu_debugfs_dtecount_read(struct file *filp,
+					  char __user *ubuf,
+					  size_t count, loff_t *offp)
+{
+	unsigned int obuflen = 64;
+	unsigned int oboff = 0;
+	unsigned int n;
+	ssize_t ret;
+	char *obuf;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	n = amd_iommu_count_valid_dtes(0, 0xFFFF);
+	oboff += OSCNPRINTF("%d\n", n);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+	kfree(obuf);
+
+	return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_dtecount_read,
+	.write = NULL,
+};
+
+void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
+{
+	char name[MAX_NAME_LEN + 1];
+	struct dentry *d_dte;
+
+	if (!debugfs_initialized())
+		return;
+
+	mutex_lock(&iommu_debugfs_lock);
+	if (!iommu_debugfs_dir)
+		iommu_debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
+	mutex_unlock(&iommu_debugfs_lock);
+	if (!iommu_debugfs_dir)
+		goto err;
+
+	snprintf(name, MAX_NAME_LEN, "iommu%02x", iommu->index);
+	iommu->debugfs_instance = debugfs_create_dir(name, iommu_debugfs_dir);
+	if (!iommu->debugfs_instance)
+		goto err;
+
+	d_dte = debugfs_create_file("count", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_dtecount_ops);
+	if (!d_dte)
+		goto err;
+
+	return;
+
+err:
+	debugfs_remove_recursive(iommu->debugfs_instance);
+}
+
+#endif
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 4e4a615bf13f..a03142fd6179 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2721,6 +2721,7 @@ int __init amd_iommu_enable_faulting(void)
  */
 static int __init amd_iommu_init(void)
 {
+	struct amd_iommu *iommu;
 	int ret;
 
 	ret = iommu_go_to_state(IOMMU_INITIALIZED);
@@ -2730,14 +2731,15 @@ static int __init amd_iommu_init(void)
 			disable_iommus();
 			free_iommu_resources();
 		} else {
-			struct amd_iommu *iommu;
-
 			uninit_device_table_dma();
 			for_each_iommu(iommu)
 				iommu_flush_all_caches(iommu);
 		}
 	}
 
+	for_each_iommu(iommu)
+		amd_iommu_debugfs_setup(iommu);
+
 	return ret;
 }
 
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 640c286a0ab9..db5872d3d40f 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -33,6 +33,13 @@ extern void amd_iommu_uninit_devices(void);
 extern void amd_iommu_init_notifier(void);
 extern int amd_iommu_init_api(void);
 
+#ifdef	CONFIG_AMD_IOMMU_DEBUG
+extern void amd_iommu_debugfs_setup(struct amd_iommu *iommu);
+#else
+static inline void amd_iommu_debugfs_setup(struct amd_iommu *iommu) {}
+#endif
+
+
 /* Needed for interrupt remapping */
 extern int amd_iommu_prepare(void);
 extern int amd_iommu_enable(void);
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 6a877ebd058b..43c008dc7ade 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -593,6 +593,9 @@ struct amd_iommu {
 
 	u32 flags;
 	volatile u64 __aligned(8) cmd_sem;
+
+	/* DebugFS Info */
+	struct dentry *debugfs_instance;
 };
 
 static inline struct amd_iommu *dev_to_amd_iommu(struct device *dev)

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 2/5] iommu/amd - Add a 'verbose' switch for IOMMU debugfs
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

Enable more descriptive debugfs output via a 'verbose' variable.

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/iommu/amd_iommu_debugfs.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index 4f0f05a89a41..170863e5e86b 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -41,6 +41,8 @@ static DEFINE_MUTEX(iommu_debugfs_lock);
 
 #define	MAX_NAME_LEN	20
 
+static unsigned int amd_iommu_verbose = 0;
+
 static unsigned int amd_iommu_count_valid_dtes(int start, int end)
 {
 	unsigned int n = 0;
@@ -72,7 +74,10 @@ static ssize_t amd_iommu_debugfs_dtecount_read(struct file *filp,
 		return -ENOMEM;
 
 	n = amd_iommu_count_valid_dtes(0, 0xFFFF);
-	oboff += OSCNPRINTF("%d\n", n);
+	if (amd_iommu_verbose)
+		oboff += OSCNPRINTF("# DTEs:  %d\n", n);
+	else
+		oboff += OSCNPRINTF("%d\n", n);
 
 	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
 	kfree(obuf);
@@ -90,6 +95,7 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 {
 	char name[MAX_NAME_LEN + 1];
+	struct dentry *d_verbose;
 	struct dentry *d_dte;
 
 	if (!debugfs_initialized())
@@ -107,6 +113,12 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!iommu->debugfs_instance)
 		goto err;
 
+	d_verbose = debugfs_create_u32("verbose", 0600,
+				       iommu->debugfs_instance,
+				       &amd_iommu_verbose);
+	if (!d_verbose)
+		goto err;
+
 	d_dte = debugfs_create_file("count", 0400,
 				    iommu->debugfs_instance, iommu,
 				    &amd_iommu_debugfs_dtecount_ops);

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 2/5] iommu/amd - Add a 'verbose' switch for IOMMU debugfs
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Enable more descriptive debugfs output via a 'verbose' variable.

Signed-off-by: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_debugfs.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index 4f0f05a89a41..170863e5e86b 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -41,6 +41,8 @@ static DEFINE_MUTEX(iommu_debugfs_lock);
 
 #define	MAX_NAME_LEN	20
 
+static unsigned int amd_iommu_verbose = 0;
+
 static unsigned int amd_iommu_count_valid_dtes(int start, int end)
 {
 	unsigned int n = 0;
@@ -72,7 +74,10 @@ static ssize_t amd_iommu_debugfs_dtecount_read(struct file *filp,
 		return -ENOMEM;
 
 	n = amd_iommu_count_valid_dtes(0, 0xFFFF);
-	oboff += OSCNPRINTF("%d\n", n);
+	if (amd_iommu_verbose)
+		oboff += OSCNPRINTF("# DTEs:  %d\n", n);
+	else
+		oboff += OSCNPRINTF("%d\n", n);
 
 	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
 	kfree(obuf);
@@ -90,6 +95,7 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 {
 	char name[MAX_NAME_LEN + 1];
+	struct dentry *d_verbose;
 	struct dentry *d_dte;
 
 	if (!debugfs_initialized())
@@ -107,6 +113,12 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!iommu->debugfs_instance)
 		goto err;
 
+	d_verbose = debugfs_create_u32("verbose", 0600,
+				       iommu->debugfs_instance,
+				       &amd_iommu_verbose);
+	if (!d_verbose)
+		goto err;
+
 	d_dte = debugfs_create_file("count", 0400,
 				    iommu->debugfs_instance, iommu,
 				    &amd_iommu_debugfs_dtecount_ops);

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 3/5] iommu/amd - Add a README variable for the IOMMU debugfs
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

Provide help text via a filesystem entry

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/iommu/amd_iommu_debugfs.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index 170863e5e86b..d95428b1ef90 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -92,6 +92,31 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 	.write = NULL,
 };
 
+static char readmetext[] =
+"count                   Count of active devices\n"
+"verbose                 Provide additional descriptive text\n"
+"\n";
+
+static ssize_t amd_iommu_debugfs_readme_read(struct file *filp,
+				      char __user *ubuf,
+				      size_t count, loff_t *offp)
+{
+	ssize_t ret;
+
+	ret = simple_read_from_buffer(ubuf, count, offp,
+				      readmetext, strlen(readmetext));
+
+	return ret;
+}
+
+
+static const struct file_operations amd_iommu_debugfs_readme_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_readme_read,
+	.write = NULL,
+};
+
 void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 {
 	char name[MAX_NAME_LEN + 1];
@@ -125,6 +150,12 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!d_dte)
 		goto err;
 
+	d_dte = debugfs_create_file("README", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_readme_ops);
+	if (!d_dte)
+		goto err;
+
 	return;
 
 err:

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 3/5] iommu/amd - Add a README variable for the IOMMU debugfs
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Provide help text via a filesystem entry

Signed-off-by: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_debugfs.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index 170863e5e86b..d95428b1ef90 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -92,6 +92,31 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 	.write = NULL,
 };
 
+static char readmetext[] =
+"count                   Count of active devices\n"
+"verbose                 Provide additional descriptive text\n"
+"\n";
+
+static ssize_t amd_iommu_debugfs_readme_read(struct file *filp,
+				      char __user *ubuf,
+				      size_t count, loff_t *offp)
+{
+	ssize_t ret;
+
+	ret = simple_read_from_buffer(ubuf, count, offp,
+				      readmetext, strlen(readmetext));
+
+	return ret;
+}
+
+
+static const struct file_operations amd_iommu_debugfs_readme_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_readme_read,
+	.write = NULL,
+};
+
 void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 {
 	char name[MAX_NAME_LEN + 1];
@@ -125,6 +150,12 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!d_dte)
 		goto err;
 
+	d_dte = debugfs_create_file("README", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_readme_ops);
+	if (!d_dte)
+		goto err;
+
 	return;
 
 err:

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 4/5] iommu/amd - Expose the active IOMMU device table entries
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

Add a debugfs entry to dump the active device table entries from
the IOMMU's table. Active is determined by non-default values
in the first and second long words of the DTE. Aside from IOMMU
devices, this output should list every device reported by lspci.

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/iommu/amd_iommu_debugfs.c |   60 +++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index d95428b1ef90..1d941c5329be 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -92,9 +92,62 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 	.write = NULL,
 };
 
+#define	MAX_PCI_ID	0xFFFF
+
+#define	PRINTDTE(i)	OSCNPRINTF("%02x:%02x:%x - %016llx %016llx %016llx %016llx\n", \
+				   PCI_BUS_NUM(i), PCI_SLOT(i), PCI_FUNC(i), \
+				   amd_iommu_dev_table[i].data[0], \
+				   amd_iommu_dev_table[i].data[1], \
+				   amd_iommu_dev_table[i].data[2], \
+				   amd_iommu_dev_table[i].data[3]);
+
+static ssize_t amd_iommu_debugfs_dte_read(struct file *filp,
+					  char __user *ubuf,
+					  size_t count, loff_t *offp)
+{
+	unsigned int obuflen;
+	unsigned int oboff = 0;
+	unsigned int istart, iend;
+	ssize_t ret;
+	u32 i, n;
+	char *obuf;
+
+	/* Count the number of valid entries in the device table */
+	istart = 0;
+	iend = MAX_PCI_ID;
+	n = amd_iommu_count_valid_dtes(istart, iend);
+	obuflen = n * 80;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	for (i = istart ; i <= iend ; i++)
+		if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+		     || amd_iommu_dev_table[i].data[1])
+			oboff += PRINTDTE(i);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+	kfree(obuf);
+
+	return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_dte_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_dte_read,
+	.write = NULL,
+};
+
 static char readmetext[] =
+"devicetable             Print active entries in the device table\n"
 "count                   Count of active devices\n"
 "verbose                 Provide additional descriptive text\n"
+"\n"
+"                        Dumping the Device Table\n"
+"The device table is scanned for entries that appear to be active. The\n"
+"default range is from 0 to 0xFFFF, and only active entries will be reported\n"
 "\n";
 
 static ssize_t amd_iommu_debugfs_readme_read(struct file *filp,
@@ -144,6 +197,13 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!d_verbose)
 		goto err;
 
+	/* Device Table Entries */
+	d_dte = debugfs_create_file("devicetable", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_dte_ops);
+	if (!d_dte)
+		goto err;
+
 	d_dte = debugfs_create_file("count", 0400,
 				    iommu->debugfs_instance, iommu,
 				    &amd_iommu_debugfs_dtecount_ops);

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 4/5] iommu/amd - Expose the active IOMMU device table entries
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Add a debugfs entry to dump the active device table entries from
the IOMMU's table. Active is determined by non-default values
in the first and second long words of the DTE. Aside from IOMMU
devices, this output should list every device reported by lspci.

Signed-off-by: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_debugfs.c |   60 +++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index d95428b1ef90..1d941c5329be 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -92,9 +92,62 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 	.write = NULL,
 };
 
+#define	MAX_PCI_ID	0xFFFF
+
+#define	PRINTDTE(i)	OSCNPRINTF("%02x:%02x:%x - %016llx %016llx %016llx %016llx\n", \
+				   PCI_BUS_NUM(i), PCI_SLOT(i), PCI_FUNC(i), \
+				   amd_iommu_dev_table[i].data[0], \
+				   amd_iommu_dev_table[i].data[1], \
+				   amd_iommu_dev_table[i].data[2], \
+				   amd_iommu_dev_table[i].data[3]);
+
+static ssize_t amd_iommu_debugfs_dte_read(struct file *filp,
+					  char __user *ubuf,
+					  size_t count, loff_t *offp)
+{
+	unsigned int obuflen;
+	unsigned int oboff = 0;
+	unsigned int istart, iend;
+	ssize_t ret;
+	u32 i, n;
+	char *obuf;
+
+	/* Count the number of valid entries in the device table */
+	istart = 0;
+	iend = MAX_PCI_ID;
+	n = amd_iommu_count_valid_dtes(istart, iend);
+	obuflen = n * 80;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	for (i = istart ; i <= iend ; i++)
+		if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+		     || amd_iommu_dev_table[i].data[1])
+			oboff += PRINTDTE(i);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+	kfree(obuf);
+
+	return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_dte_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_dte_read,
+	.write = NULL,
+};
+
 static char readmetext[] =
+"devicetable             Print active entries in the device table\n"
 "count                   Count of active devices\n"
 "verbose                 Provide additional descriptive text\n"
+"\n"
+"                        Dumping the Device Table\n"
+"The device table is scanned for entries that appear to be active. The\n"
+"default range is from 0 to 0xFFFF, and only active entries will be reported\n"
 "\n";
 
 static ssize_t amd_iommu_debugfs_readme_read(struct file *filp,
@@ -144,6 +197,13 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!d_verbose)
 		goto err;
 
+	/* Device Table Entries */
+	d_dte = debugfs_create_file("devicetable", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_dte_ops);
+	if (!d_dte)
+		goto err;
+
 	d_dte = debugfs_create_file("count", 0400,
 				    iommu->debugfs_instance, iommu,
 				    &amd_iommu_debugfs_dtecount_ops);

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 5/5] iommu/amd - Add a debugfs entry to specify a IOMMU device table entry
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

Initially (at boot) the device table values dumped are all of the
active devices.  Add a devid debugfs file to allow the user to select a
single device table entry to dump (active or not). Let any devid value
greater than the maximum allowable PCI ID (0xFFFF) restore the
behavior to that effective at boot.

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/iommu/amd_iommu_debugfs.c |  121 ++++++++++++++++++++++++++++++++-----
 1 file changed, 106 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index 1d941c5329be..47bf718f6178 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -42,6 +42,7 @@ static DEFINE_MUTEX(iommu_debugfs_lock);
 #define	MAX_NAME_LEN	20
 
 static unsigned int amd_iommu_verbose = 0;
+static unsigned int amd_iommu_devid = ~0;
 
 static unsigned int amd_iommu_count_valid_dtes(int start, int end)
 {
@@ -92,14 +93,84 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 	.write = NULL,
 };
 
+static ssize_t amd_iommu_debugfs_devid_read(struct file *filp,
+					    char __user *ubuf,
+					    size_t count, loff_t *offp)
+{
+	unsigned int obuflen = 64;
+	unsigned int oboff = 0;
+	ssize_t ret;
+	char *obuf;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	if (amd_iommu_verbose)
+		oboff += OSCNPRINTF("%02x:%02x:%x (%u / %04x)\n",
+				    PCI_BUS_NUM(amd_iommu_devid),
+				    PCI_SLOT(amd_iommu_devid),
+				    PCI_FUNC(amd_iommu_devid),
+				    amd_iommu_devid, amd_iommu_devid);
+	else
+		oboff += OSCNPRINTF("%u\n", amd_iommu_devid);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+	kfree(obuf);
+
+	return ret;
+}
+
+static ssize_t amd_iommu_debugfs_devid_write(struct file *filp,
+					    const char __user *ubuf,
+					    size_t count, loff_t *offp)
+{
+	unsigned int pci_id, pci_slot, pci_func;
+	unsigned int obuflen = 80;
+	ssize_t ret;
+	char *obuf;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	ret = simple_write_to_buffer(obuf, OBUFLEN, offp, ubuf, count);
+
+	if (strnchr(obuf, OBUFLEN, ':')) {
+		int n;
+		n = sscanf(obuf, "%x:%x.%x", &pci_id, &pci_slot, &pci_func);
+		if (n == 3)
+			amd_iommu_devid = PCI_DEVID(pci_id, PCI_DEVFN(pci_slot, pci_func));
+	} else {
+		kstrtoint(obuf, 0, &amd_iommu_devid);
+	}
+
+	kfree(obuf);
+
+	return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_devid_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_devid_read,
+	.write = amd_iommu_debugfs_devid_write,
+};
+
 #define	MAX_PCI_ID	0xFFFF
 
-#define	PRINTDTE(i)	OSCNPRINTF("%02x:%02x:%x - %016llx %016llx %016llx %016llx\n", \
-				   PCI_BUS_NUM(i), PCI_SLOT(i), PCI_FUNC(i), \
-				   amd_iommu_dev_table[i].data[0], \
-				   amd_iommu_dev_table[i].data[1], \
-				   amd_iommu_dev_table[i].data[2], \
-				   amd_iommu_dev_table[i].data[3]);
+static inline int amd_iommu_debugfs_printdte(int i, char *obuf, unsigned int obuflen, unsigned int oboff)
+{
+	int rc;
+
+	rc = OSCNPRINTF("%02x:%02x:%x - %016llx %016llx %016llx %016llx\n",
+			PCI_BUS_NUM(i), PCI_SLOT(i), PCI_FUNC(i),
+			amd_iommu_dev_table[i].data[0],
+			amd_iommu_dev_table[i].data[1],
+			amd_iommu_dev_table[i].data[2],
+			amd_iommu_dev_table[i].data[3]);
+	return rc;
+}
 
 static ssize_t amd_iommu_debugfs_dte_read(struct file *filp,
 					  char __user *ubuf,
@@ -113,19 +184,28 @@ static ssize_t amd_iommu_debugfs_dte_read(struct file *filp,
 	char *obuf;
 
 	/* Count the number of valid entries in the device table */
-	istart = 0;
-	iend = MAX_PCI_ID;
-	n = amd_iommu_count_valid_dtes(istart, iend);
+	if (amd_iommu_devid > MAX_PCI_ID) {
+		istart = 0;
+		iend = MAX_PCI_ID;
+		n = amd_iommu_count_valid_dtes(istart, iend);
+	} else {
+		n = 1;
+	}
 	obuflen = n * 80;
 
 	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
 	if (!obuf)
 		return -ENOMEM;
 
-	for (i = istart ; i <= iend ; i++)
-		if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
-		     || amd_iommu_dev_table[i].data[1])
-			oboff += PRINTDTE(i);
+	if (amd_iommu_devid > MAX_PCI_ID) {
+		for (i = istart ; i <= iend ; i++)
+			if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+			     || amd_iommu_dev_table[i].data[1])
+				oboff += amd_iommu_debugfs_printdte(i, obuf, obuflen, oboff);
+	} else {
+		i = amd_iommu_devid;
+		oboff += amd_iommu_debugfs_printdte(i, obuf, obuflen, oboff);
+	}
 
 	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
 	kfree(obuf);
@@ -142,12 +222,17 @@ static const struct file_operations amd_iommu_debugfs_dte_ops = {
 
 static char readmetext[] =
 "devicetable             Print active entries in the device table\n"
+"devid                   Controls which device IDs are printed\n"
 "count                   Count of active devices\n"
 "verbose                 Provide additional descriptive text\n"
 "\n"
 "                        Dumping the Device Table\n"
-"The device table is scanned for entries that appear to be active. The\n"
-"default range is from 0 to 0xFFFF, and only active entries will be reported\n"
+"The device table is scanned for entries that appear to be active.\n"
+"The default (initial) range is from 0 to 0xFFFF, represented by a devid\n"
+"value greater than 0xFFFF, and only active entries will be reported.\n"
+"If devid is set to a specific value, only that device entry will be\n"
+"displayed (active or not). devid may be specified as ##:##:#, a decimal\n"
+"value, or a hex value.\n"
 "\n";
 
 static ssize_t amd_iommu_debugfs_readme_read(struct file *filp,
@@ -210,6 +295,12 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!d_dte)
 		goto err;
 
+	d_dte = debugfs_create_file("devid", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_devid_ops);
+	if (!d_dte)
+		goto err;
+
 	d_dte = debugfs_create_file("README", 0400,
 				    iommu->debugfs_instance, iommu,
 				    &amd_iommu_debugfs_readme_ops);

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v3 5/5] iommu/amd - Add a debugfs entry to specify a IOMMU device table entry
@ 2018-03-14 23:05   ` Gary R Hook
  0 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-14 23:05 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Initially (at boot) the device table values dumped are all of the
active devices.  Add a devid debugfs file to allow the user to select a
single device table entry to dump (active or not). Let any devid value
greater than the maximum allowable PCI ID (0xFFFF) restore the
behavior to that effective at boot.

Signed-off-by: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_debugfs.c |  121 ++++++++++++++++++++++++++++++++-----
 1 file changed, 106 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
index 1d941c5329be..47bf718f6178 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -42,6 +42,7 @@ static DEFINE_MUTEX(iommu_debugfs_lock);
 #define	MAX_NAME_LEN	20
 
 static unsigned int amd_iommu_verbose = 0;
+static unsigned int amd_iommu_devid = ~0;
 
 static unsigned int amd_iommu_count_valid_dtes(int start, int end)
 {
@@ -92,14 +93,84 @@ static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
 	.write = NULL,
 };
 
+static ssize_t amd_iommu_debugfs_devid_read(struct file *filp,
+					    char __user *ubuf,
+					    size_t count, loff_t *offp)
+{
+	unsigned int obuflen = 64;
+	unsigned int oboff = 0;
+	ssize_t ret;
+	char *obuf;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	if (amd_iommu_verbose)
+		oboff += OSCNPRINTF("%02x:%02x:%x (%u / %04x)\n",
+				    PCI_BUS_NUM(amd_iommu_devid),
+				    PCI_SLOT(amd_iommu_devid),
+				    PCI_FUNC(amd_iommu_devid),
+				    amd_iommu_devid, amd_iommu_devid);
+	else
+		oboff += OSCNPRINTF("%u\n", amd_iommu_devid);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+	kfree(obuf);
+
+	return ret;
+}
+
+static ssize_t amd_iommu_debugfs_devid_write(struct file *filp,
+					    const char __user *ubuf,
+					    size_t count, loff_t *offp)
+{
+	unsigned int pci_id, pci_slot, pci_func;
+	unsigned int obuflen = 80;
+	ssize_t ret;
+	char *obuf;
+
+	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	ret = simple_write_to_buffer(obuf, OBUFLEN, offp, ubuf, count);
+
+	if (strnchr(obuf, OBUFLEN, ':')) {
+		int n;
+		n = sscanf(obuf, "%x:%x.%x", &pci_id, &pci_slot, &pci_func);
+		if (n == 3)
+			amd_iommu_devid = PCI_DEVID(pci_id, PCI_DEVFN(pci_slot, pci_func));
+	} else {
+		kstrtoint(obuf, 0, &amd_iommu_devid);
+	}
+
+	kfree(obuf);
+
+	return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_devid_ops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = amd_iommu_debugfs_devid_read,
+	.write = amd_iommu_debugfs_devid_write,
+};
+
 #define	MAX_PCI_ID	0xFFFF
 
-#define	PRINTDTE(i)	OSCNPRINTF("%02x:%02x:%x - %016llx %016llx %016llx %016llx\n", \
-				   PCI_BUS_NUM(i), PCI_SLOT(i), PCI_FUNC(i), \
-				   amd_iommu_dev_table[i].data[0], \
-				   amd_iommu_dev_table[i].data[1], \
-				   amd_iommu_dev_table[i].data[2], \
-				   amd_iommu_dev_table[i].data[3]);
+static inline int amd_iommu_debugfs_printdte(int i, char *obuf, unsigned int obuflen, unsigned int oboff)
+{
+	int rc;
+
+	rc = OSCNPRINTF("%02x:%02x:%x - %016llx %016llx %016llx %016llx\n",
+			PCI_BUS_NUM(i), PCI_SLOT(i), PCI_FUNC(i),
+			amd_iommu_dev_table[i].data[0],
+			amd_iommu_dev_table[i].data[1],
+			amd_iommu_dev_table[i].data[2],
+			amd_iommu_dev_table[i].data[3]);
+	return rc;
+}
 
 static ssize_t amd_iommu_debugfs_dte_read(struct file *filp,
 					  char __user *ubuf,
@@ -113,19 +184,28 @@ static ssize_t amd_iommu_debugfs_dte_read(struct file *filp,
 	char *obuf;
 
 	/* Count the number of valid entries in the device table */
-	istart = 0;
-	iend = MAX_PCI_ID;
-	n = amd_iommu_count_valid_dtes(istart, iend);
+	if (amd_iommu_devid > MAX_PCI_ID) {
+		istart = 0;
+		iend = MAX_PCI_ID;
+		n = amd_iommu_count_valid_dtes(istart, iend);
+	} else {
+		n = 1;
+	}
 	obuflen = n * 80;
 
 	obuf = kmalloc(OBUFLEN, GFP_KERNEL);
 	if (!obuf)
 		return -ENOMEM;
 
-	for (i = istart ; i <= iend ; i++)
-		if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
-		     || amd_iommu_dev_table[i].data[1])
-			oboff += PRINTDTE(i);
+	if (amd_iommu_devid > MAX_PCI_ID) {
+		for (i = istart ; i <= iend ; i++)
+			if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+			     || amd_iommu_dev_table[i].data[1])
+				oboff += amd_iommu_debugfs_printdte(i, obuf, obuflen, oboff);
+	} else {
+		i = amd_iommu_devid;
+		oboff += amd_iommu_debugfs_printdte(i, obuf, obuflen, oboff);
+	}
 
 	ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
 	kfree(obuf);
@@ -142,12 +222,17 @@ static const struct file_operations amd_iommu_debugfs_dte_ops = {
 
 static char readmetext[] =
 "devicetable             Print active entries in the device table\n"
+"devid                   Controls which device IDs are printed\n"
 "count                   Count of active devices\n"
 "verbose                 Provide additional descriptive text\n"
 "\n"
 "                        Dumping the Device Table\n"
-"The device table is scanned for entries that appear to be active. The\n"
-"default range is from 0 to 0xFFFF, and only active entries will be reported\n"
+"The device table is scanned for entries that appear to be active.\n"
+"The default (initial) range is from 0 to 0xFFFF, represented by a devid\n"
+"value greater than 0xFFFF, and only active entries will be reported.\n"
+"If devid is set to a specific value, only that device entry will be\n"
+"displayed (active or not). devid may be specified as ##:##:#, a decimal\n"
+"value, or a hex value.\n"
 "\n";
 
 static ssize_t amd_iommu_debugfs_readme_read(struct file *filp,
@@ -210,6 +295,12 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 	if (!d_dte)
 		goto err;
 
+	d_dte = debugfs_create_file("devid", 0400,
+				    iommu->debugfs_instance, iommu,
+				    &amd_iommu_debugfs_devid_ops);
+	if (!d_dte)
+		goto err;
+
 	d_dte = debugfs_create_file("README", 0400,
 				    iommu->debugfs_instance, iommu,
 				    &amd_iommu_debugfs_readme_ops);

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3 0/5] Add debugfs info for the AMD IOMMU
@ 2018-03-15 13:58   ` Joerg Roedel
  0 siblings, 0 replies; 17+ messages in thread
From: Joerg Roedel @ 2018-03-15 13:58 UTC (permalink / raw)
  To: Gary R Hook; +Cc: iommu, linux-kernel

On Wed, Mar 14, 2018 at 06:04:44PM -0500, Gary R Hook wrote:
> Gary R Hook (5):
>       iommu/amd - Add debugfs support
>       iommu/amd - Add a 'verbose' switch for IOMMU debugfs
>       iommu/amd - Add a README variable for the IOMMU debugfs
>       iommu/amd - Expose the active IOMMU device table entries
>       iommu/amd - Add a debugfs entry to specify a IOMMU device table entry

Same problem here as with the Intel patches, I don't think it's a good
idea to reveal internal iommu data structures to user-space in this way.

I've debugged iommu issues for around 10 years now and never had the
need for an interface that reveals those internals. How exactly are you
planning to use this information?



	Joerg

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v3 0/5] Add debugfs info for the AMD IOMMU
@ 2018-03-15 13:58   ` Joerg Roedel
  0 siblings, 0 replies; 17+ messages in thread
From: Joerg Roedel @ 2018-03-15 13:58 UTC (permalink / raw)
  To: Gary R Hook
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Wed, Mar 14, 2018 at 06:04:44PM -0500, Gary R Hook wrote:
> Gary R Hook (5):
>       iommu/amd - Add debugfs support
>       iommu/amd - Add a 'verbose' switch for IOMMU debugfs
>       iommu/amd - Add a README variable for the IOMMU debugfs
>       iommu/amd - Expose the active IOMMU device table entries
>       iommu/amd - Add a debugfs entry to specify a IOMMU device table entry

Same problem here as with the Intel patches, I don't think it's a good
idea to reveal internal iommu data structures to user-space in this way.

I've debugged iommu issues for around 10 years now and never had the
need for an interface that reveals those internals. How exactly are you
planning to use this information?



	Joerg

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v3 5/5] iommu/amd - Add a debugfs entry to specify a IOMMU device table entry
@ 2018-03-17 20:09     ` kbuild test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2018-03-17 20:09 UTC (permalink / raw)
  To: Gary R Hook; +Cc: kbuild-all, iommu, joro, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1983 bytes --]

Hi Gary,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gary-R-Hook/Add-debugfs-info-for-the-AMD-IOMMU/20180317-232302
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/iommu/amd_iommu_debugfs.c: In function 'amd_iommu_debugfs_devid_write':
>> drivers/iommu/amd_iommu_debugfs.c:145:3: warning: ignoring return value of 'kstrtoint', declared with attribute warn_unused_result [-Wunused-result]
      kstrtoint(obuf, 0, &amd_iommu_devid);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/kstrtoint +145 drivers/iommu/amd_iommu_debugfs.c

   123	
   124	static ssize_t amd_iommu_debugfs_devid_write(struct file *filp,
   125						    const char __user *ubuf,
   126						    size_t count, loff_t *offp)
   127	{
   128		unsigned int pci_id, pci_slot, pci_func;
   129		unsigned int obuflen = 80;
   130		ssize_t ret;
   131		char *obuf;
   132	
   133		obuf = kmalloc(OBUFLEN, GFP_KERNEL);
   134		if (!obuf)
   135			return -ENOMEM;
   136	
   137		ret = simple_write_to_buffer(obuf, OBUFLEN, offp, ubuf, count);
   138	
   139		if (strnchr(obuf, OBUFLEN, ':')) {
   140			int n;
   141			n = sscanf(obuf, "%x:%x.%x", &pci_id, &pci_slot, &pci_func);
   142			if (n == 3)
   143				amd_iommu_devid = PCI_DEVID(pci_id, PCI_DEVFN(pci_slot, pci_func));
   144		} else {
 > 145			kstrtoint(obuf, 0, &amd_iommu_devid);
   146		}
   147	
   148		kfree(obuf);
   149	
   150		return ret;
   151	}
   152	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63073 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v3 5/5] iommu/amd - Add a debugfs entry to specify a IOMMU device table entry
@ 2018-03-17 20:09     ` kbuild test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2018-03-17 20:09 UTC (permalink / raw)
  To: Gary R Hook
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	kbuild-all-JC7UmRfGjtg

[-- Attachment #1: Type: text/plain, Size: 1983 bytes --]

Hi Gary,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gary-R-Hook/Add-debugfs-info-for-the-AMD-IOMMU/20180317-232302
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/iommu/amd_iommu_debugfs.c: In function 'amd_iommu_debugfs_devid_write':
>> drivers/iommu/amd_iommu_debugfs.c:145:3: warning: ignoring return value of 'kstrtoint', declared with attribute warn_unused_result [-Wunused-result]
      kstrtoint(obuf, 0, &amd_iommu_devid);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/kstrtoint +145 drivers/iommu/amd_iommu_debugfs.c

   123	
   124	static ssize_t amd_iommu_debugfs_devid_write(struct file *filp,
   125						    const char __user *ubuf,
   126						    size_t count, loff_t *offp)
   127	{
   128		unsigned int pci_id, pci_slot, pci_func;
   129		unsigned int obuflen = 80;
   130		ssize_t ret;
   131		char *obuf;
   132	
   133		obuf = kmalloc(OBUFLEN, GFP_KERNEL);
   134		if (!obuf)
   135			return -ENOMEM;
   136	
   137		ret = simple_write_to_buffer(obuf, OBUFLEN, offp, ubuf, count);
   138	
   139		if (strnchr(obuf, OBUFLEN, ':')) {
   140			int n;
   141			n = sscanf(obuf, "%x:%x.%x", &pci_id, &pci_slot, &pci_func);
   142			if (n == 3)
   143				amd_iommu_devid = PCI_DEVID(pci_id, PCI_DEVFN(pci_slot, pci_func));
   144		} else {
 > 145			kstrtoint(obuf, 0, &amd_iommu_devid);
   146		}
   147	
   148		kfree(obuf);
   149	
   150		return ret;
   151	}
   152	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63073 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v3 0/5] Add debugfs info for the AMD IOMMU
  2018-03-15 13:58   ` Joerg Roedel
  (?)
@ 2018-03-26 23:33   ` Gary R Hook
  -1 siblings, 0 replies; 17+ messages in thread
From: Gary R Hook @ 2018-03-26 23:33 UTC (permalink / raw)
  To: Joerg Roedel, Gary R Hook; +Cc: iommu, linux-kernel

On 03/15/2018 08:58 AM, Joerg Roedel wrote:
> On Wed, Mar 14, 2018 at 06:04:44PM -0500, Gary R Hook wrote:
>> Gary R Hook (5):
>>        iommu/amd - Add debugfs support
>>        iommu/amd - Add a 'verbose' switch for IOMMU debugfs
>>        iommu/amd - Add a README variable for the IOMMU debugfs
>>        iommu/amd - Expose the active IOMMU device table entries
>>        iommu/amd - Add a debugfs entry to specify a IOMMU device table entry
> 
> Same problem here as with the Intel patches, I don't think it's a good
> idea to reveal internal iommu data structures to user-space in this way.

I'm afraid I'm not following the line of thought here. AFAIK, if we 
restrict access of any debugfs info to root, then only root can see any 
of that information. If root is compromised, the whole system is 
compromised. Which seems to me to make all of debugfs suspect from a 
security perspective. Therefore, I'm likely not understanding the 
security concerns as they relate specifically to the IOMMU.

As for stability, you mention on the Intel IOMMU thread issues 
surrounding kABI, which I appreciate. But I'm exposing well-documented 
device structures in my patches, not kernel structures. There's nothing 
there that is going to change underneath me without my knowledge, if 
ever (I vote for never). And I'm likely ignorant about policy nuances 
surrounding the kernel ABI.

> I've debugged iommu issues for around 10 years now and never had the
> need for an interface that reveals those internals. How exactly are you
> planning to use this information?

Well, I've already had to dig into the DTEs and page tables for a couple 
of reasons (both debug and development), and it made life easier (for 
me) to make the live data available this way. Then I thought that others 
might be interested as well. Admittedly, I could be wrong.

Finally, I'm no guru, and likely am unaware of other techniques in which 
I should develop skills. I'm always open to input.

^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-03-26 23:33 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 23:04 [PATCH v3 0/5] Add debugfs info for the AMD IOMMU Gary R Hook
2018-03-14 23:04 ` Gary R Hook
2018-03-14 23:04 ` [PATCH v3 1/5] iommu/amd - Add debugfs support Gary R Hook
2018-03-14 23:04   ` Gary R Hook
2018-03-14 23:05 ` [PATCH v3 2/5] iommu/amd - Add a 'verbose' switch for IOMMU debugfs Gary R Hook
2018-03-14 23:05   ` Gary R Hook
2018-03-14 23:05 ` [PATCH v3 3/5] iommu/amd - Add a README variable for the " Gary R Hook
2018-03-14 23:05   ` Gary R Hook
2018-03-14 23:05 ` [PATCH v3 4/5] iommu/amd - Expose the active IOMMU device table entries Gary R Hook
2018-03-14 23:05   ` Gary R Hook
2018-03-14 23:05 ` [PATCH v3 5/5] iommu/amd - Add a debugfs entry to specify a IOMMU device table entry Gary R Hook
2018-03-14 23:05   ` Gary R Hook
2018-03-17 20:09   ` kbuild test robot
2018-03-17 20:09     ` kbuild test robot
2018-03-15 13:58 ` [PATCH v3 0/5] Add debugfs info for the AMD IOMMU Joerg Roedel
2018-03-15 13:58   ` Joerg Roedel
2018-03-26 23:33   ` Gary R Hook

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.