All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] Base enablement of IOMMU debugfs support
@ 2018-05-07 20:01 Gary R Hook
  2018-05-07 20:02   ` Gary R Hook
  2018-05-07 20:02   ` Gary R Hook
  0 siblings, 2 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-07 20:01 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

These patches create a top-level function, called at IOMMU initialization,
to create a debugfs directory for the IOMMU. Under this directory drivers
may create and populate-specific directories for their device internals.

Patch 1: general IOMMU enablement
Patch 2: basic AMD enablement to demonstrate linkage with patch 1

Introduce a new Kconfig parameter IOMMU_DEBUGFS to globally allow/disallow
debugfs code to be built.

The Makefile structure is intended to allow the use of a single switch for
turning on DebugFS.

Changes since v4:
 - Guard vendor-specific debugfs files in the Makefile
 - Call top-level routine from iommu_init()
 - Add function for instantiating a driver-specific directory
 - Change AMD driver code to use this new format

Changes since v3:
 - Remove superfluous calls to debugfs_initialized()
 - Emit a warning exactly one time
 - Change the Kconfig name to IOMMU_DEBUGFS
 - Change the way debugfs modules are made

Changes since v2:
 - Move a declaration to outside an ifdef
 - Remove a spurious blank line

Changes since v1:
 - Remove debug cruft
 - Remove cruft produced by design change
 - Change the lock to a mutex
 - Coding style fixes
 - Add a comment to document the framework

---

Gary R Hook (2):
      iommu - Enable debugfs exposure of IOMMU driver internals
      iommu/amd: Add basic debugfs infrastructure for AMD IOMMU


 drivers/iommu/Kconfig             |   11 ++++++
 drivers/iommu/Makefile            |    6 +++
 drivers/iommu/amd_iommu_debugfs.c |   41 ++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c    |    6 ++-
 drivers/iommu/amd_iommu_proto.h   |    6 +++
 drivers/iommu/amd_iommu_types.h   |    3 ++
 drivers/iommu/iommu-debugfs.c     |   64 +++++++++++++++++++++++++++++++++++++
 drivers/iommu/iommu.c             |    2 +
 include/linux/iommu.h             |    8 +++++
 9 files changed, 145 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iommu/amd_iommu_debugfs.c
 create mode 100644 drivers/iommu/iommu-debugfs.c

--

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
@ 2018-05-07 20:02   ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-07 20:02 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

Provide base enablement for using debugfs to expose internal data of an
IOMMU driver. When called, create the /sys/kernel/debug/iommu directory.

Emit a strong warning at boot time to indicate that this feature is
enabled.

This function is called from iommu_init, and creates the initial DebugFS
directory. Drivers may then call iommu_debugfs_new_driver_dir() to
instantiate a device-specific directory to expose internal data.
It will return a pointer to the new dentry structure created in
/sys/kernel/debug/iommu, or NULL in the event of a failure.

Since the IOMMU driver can not be removed from the running system, there
is no need for an "off" function.

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/iommu/Kconfig         |   11 +++++++
 drivers/iommu/Makefile        |    1 +
 drivers/iommu/iommu-debugfs.c |   64 +++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/iommu.c         |    2 +
 include/linux/iommu.h         |    8 +++++
 5 files changed, 86 insertions(+)
 create mode 100644 drivers/iommu/iommu-debugfs.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index f3a21343e636..ff511fa8ca7d 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -60,6 +60,17 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
 
 endmenu
 
+config IOMMU_DEBUGFS
+	bool "Export IOMMU internals in DebugFS"
+	depends on DEBUG_FS
+	default n
+	help
+	  Allows exposure of IOMMU device internals. This option enables
+	  the use of debugfs by IOMMU drivers as required. Devices can,
+	  at initialization time, cause the IOMMU code to create a top-level
+	  debug/iommu directory, and then populate a subdirectory with
+	  entries as required.
+
 config IOMMU_IOVA
 	tristate
 
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb695854809..74cfbc392862 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
+obj-$(CONFIG_IOMMU_DEBUGFS) += iommu-debugfs.o
 obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
diff --git a/drivers/iommu/iommu-debugfs.c b/drivers/iommu/iommu-debugfs.c
new file mode 100644
index 000000000000..d82a10b2478b
--- /dev/null
+++ b/drivers/iommu/iommu-debugfs.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * IOMMU driver
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ *
+ * Author: Gary R Hook <gary.hook@amd.com>
+ */
+
+#include <linux/pci.h>
+#include <linux/iommu.h>
+#include <linux/debugfs.h>
+
+static struct dentry *iommu_debugfs_dir;
+
+/*
+ * Provide base enablement for using debugfs to expose internal data of an
+ * IOMMU driver. When called, create the /sys/kernel/debug/iommu directory.
+ *
+ * Emit a strong warning at boot time to indicate that this feature is
+ * enabled.
+ *
+ * This function is called from iommu_init, and creates the initial DebugFS
+ * directory. Drivers may then call iommu_debugfs_new_driver_dir() to
+ * instantiate a device-specific directory to expose internal data.
+ * It will return a pointer to the new dentry structure created in
+ * /sys/kernel/debug/iommu, or NULL in the event of a failure.
+ *
+ * Since the IOMMU driver can not be removed from the running system, there
+ * is no need for an "off" function.
+ */
+void iommu_debugfs_setup(void)
+{
+	if (!iommu_debugfs_dir) {
+		iommu_debugfs_dir = debugfs_create_dir("iommu", NULL);
+		if (iommu_debugfs_dir) {
+			pr_warn("\n");
+			pr_warn("*************************************************************\n");
+			pr_warn("**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE    **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("**  IOMMU DebugFS SUPPORT HAS BEEN ENABLED IN THIS KERNEL  **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("** This means that this kernel is built to expose internal **\n");
+			pr_warn("** IOMMU data structures, which may compromise security on **\n");
+			pr_warn("** your system.                                            **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("** If you see this message and you are not debugging the   **\n");
+			pr_warn("** kernel, report this immediately to your vendor!         **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE    **\n");
+			pr_warn("*************************************************************\n");
+		}
+	}
+}
+
+struct dentry *iommu_debugfs_new_driver_dir(char *name)
+{
+	struct dentry *d_new;
+
+	d_new = debugfs_create_dir(name, iommu_debugfs_dir);
+
+	return d_new;
+}
+EXPORT_SYMBOL_GPL(iommu_debugfs_new_driver_dir);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d2aa23202bb9..350819f1c5e1 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1747,6 +1747,8 @@ static int __init iommu_init(void)
 					       NULL, kernel_kobj);
 	BUG_ON(!iommu_group_kset);
 
+	iommu_debugfs_setup();
+
 	return 0;
 }
 core_initcall(iommu_init);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 19938ee6eb31..6bdfe942068b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -698,4 +698,12 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 
 #endif /* CONFIG_IOMMU_API */
 
+#ifdef CONFIG_IOMMU_DEBUGFS
+void iommu_debugfs_setup(void);
+struct dentry *iommu_debugfs_new_driver_dir(char *);
+#else
+static inline void iommu_debugfs_setup(void) {}
+struct dentry *iommu_debugfs_new_driver_dir(char *) {};
+#endif
+
 #endif /* __LINUX_IOMMU_H */

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
@ 2018-05-07 20:02   ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-07 20:02 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Provide base enablement for using debugfs to expose internal data of an
IOMMU driver. When called, create the /sys/kernel/debug/iommu directory.

Emit a strong warning at boot time to indicate that this feature is
enabled.

This function is called from iommu_init, and creates the initial DebugFS
directory. Drivers may then call iommu_debugfs_new_driver_dir() to
instantiate a device-specific directory to expose internal data.
It will return a pointer to the new dentry structure created in
/sys/kernel/debug/iommu, or NULL in the event of a failure.

Since the IOMMU driver can not be removed from the running system, there
is no need for an "off" function.

Signed-off-by: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/Kconfig         |   11 +++++++
 drivers/iommu/Makefile        |    1 +
 drivers/iommu/iommu-debugfs.c |   64 +++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/iommu.c         |    2 +
 include/linux/iommu.h         |    8 +++++
 5 files changed, 86 insertions(+)
 create mode 100644 drivers/iommu/iommu-debugfs.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index f3a21343e636..ff511fa8ca7d 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -60,6 +60,17 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
 
 endmenu
 
+config IOMMU_DEBUGFS
+	bool "Export IOMMU internals in DebugFS"
+	depends on DEBUG_FS
+	default n
+	help
+	  Allows exposure of IOMMU device internals. This option enables
+	  the use of debugfs by IOMMU drivers as required. Devices can,
+	  at initialization time, cause the IOMMU code to create a top-level
+	  debug/iommu directory, and then populate a subdirectory with
+	  entries as required.
+
 config IOMMU_IOVA
 	tristate
 
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb695854809..74cfbc392862 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
+obj-$(CONFIG_IOMMU_DEBUGFS) += iommu-debugfs.o
 obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
diff --git a/drivers/iommu/iommu-debugfs.c b/drivers/iommu/iommu-debugfs.c
new file mode 100644
index 000000000000..d82a10b2478b
--- /dev/null
+++ b/drivers/iommu/iommu-debugfs.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * IOMMU driver
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ *
+ * Author: Gary R Hook <gary.hook-5C7GfCeVMHo@public.gmane.org>
+ */
+
+#include <linux/pci.h>
+#include <linux/iommu.h>
+#include <linux/debugfs.h>
+
+static struct dentry *iommu_debugfs_dir;
+
+/*
+ * Provide base enablement for using debugfs to expose internal data of an
+ * IOMMU driver. When called, create the /sys/kernel/debug/iommu directory.
+ *
+ * Emit a strong warning at boot time to indicate that this feature is
+ * enabled.
+ *
+ * This function is called from iommu_init, and creates the initial DebugFS
+ * directory. Drivers may then call iommu_debugfs_new_driver_dir() to
+ * instantiate a device-specific directory to expose internal data.
+ * It will return a pointer to the new dentry structure created in
+ * /sys/kernel/debug/iommu, or NULL in the event of a failure.
+ *
+ * Since the IOMMU driver can not be removed from the running system, there
+ * is no need for an "off" function.
+ */
+void iommu_debugfs_setup(void)
+{
+	if (!iommu_debugfs_dir) {
+		iommu_debugfs_dir = debugfs_create_dir("iommu", NULL);
+		if (iommu_debugfs_dir) {
+			pr_warn("\n");
+			pr_warn("*************************************************************\n");
+			pr_warn("**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE    **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("**  IOMMU DebugFS SUPPORT HAS BEEN ENABLED IN THIS KERNEL  **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("** This means that this kernel is built to expose internal **\n");
+			pr_warn("** IOMMU data structures, which may compromise security on **\n");
+			pr_warn("** your system.                                            **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("** If you see this message and you are not debugging the   **\n");
+			pr_warn("** kernel, report this immediately to your vendor!         **\n");
+			pr_warn("**                                                         **\n");
+			pr_warn("**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE    **\n");
+			pr_warn("*************************************************************\n");
+		}
+	}
+}
+
+struct dentry *iommu_debugfs_new_driver_dir(char *name)
+{
+	struct dentry *d_new;
+
+	d_new = debugfs_create_dir(name, iommu_debugfs_dir);
+
+	return d_new;
+}
+EXPORT_SYMBOL_GPL(iommu_debugfs_new_driver_dir);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d2aa23202bb9..350819f1c5e1 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1747,6 +1747,8 @@ static int __init iommu_init(void)
 					       NULL, kernel_kobj);
 	BUG_ON(!iommu_group_kset);
 
+	iommu_debugfs_setup();
+
 	return 0;
 }
 core_initcall(iommu_init);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 19938ee6eb31..6bdfe942068b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -698,4 +698,12 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 
 #endif /* CONFIG_IOMMU_API */
 
+#ifdef CONFIG_IOMMU_DEBUGFS
+void iommu_debugfs_setup(void);
+struct dentry *iommu_debugfs_new_driver_dir(char *);
+#else
+static inline void iommu_debugfs_setup(void) {}
+struct dentry *iommu_debugfs_new_driver_dir(char *) {};
+#endif
+
 #endif /* __LINUX_IOMMU_H */

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 2/2] iommu/amd: Add basic debugfs infrastructure for AMD IOMMU
@ 2018-05-07 20:02   ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-07 20:02 UTC (permalink / raw)
  To: iommu; +Cc: joro, linux-kernel

Implement a skeleton framework for debugfs support in the
AMD IOMMU.


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

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 74cfbc392862..dd980f7dd8b6 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -30,3 +30,8 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
 obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
 obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
+
+# This ensures that only the required files are compiled
+ifeq ($(CONFIG_IOMMU_DEBUGFS), y)
+obj-$(CONFIG_AMD_IOMMU) += amd_iommu_debugfs.o
+endif
diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
new file mode 100644
index 000000000000..2b351b9f9130
--- /dev/null
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD IOMMU driver
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ *
+ * Author: Gary R Hook <gary.hook@amd.com>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/iommu.h>
+#include <linux/pci.h>
+#include "amd_iommu_proto.h"
+#include "amd_iommu_types.h"
+
+static struct dentry *amd_iommu_debugfs;
+static DEFINE_MUTEX(amd_iommu_debugfs_lock);
+
+#define	MAX_NAME_LEN	20
+
+void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
+{
+	char name[MAX_NAME_LEN + 1];
+
+pr_warn("GRH: %s:%d\n", __func__, __LINE__);
+
+	mutex_lock(&amd_iommu_debugfs_lock);
+	if (!amd_iommu_debugfs)
+		amd_iommu_debugfs = iommu_debugfs_new_driver_dir("amd");
+	mutex_unlock(&amd_iommu_debugfs_lock);
+
+	if (amd_iommu_debugfs) {
+		snprintf(name, MAX_NAME_LEN, "iommu%02d", iommu->index);
+		iommu->debugfs = debugfs_create_dir(name,
+						    amd_iommu_debugfs);
+		if (!iommu->debugfs) {
+			debugfs_remove_recursive(amd_iommu_debugfs);
+			amd_iommu_debugfs = NULL;
+		}
+	}
+}
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 904c575d1677..031e6dbb8345 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..39053f11dda3 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -33,6 +33,12 @@ extern void amd_iommu_uninit_devices(void);
 extern void amd_iommu_init_notifier(void);
 extern int amd_iommu_init_api(void);
 
+#ifdef CONFIG_IOMMU_DEBUGFS
+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 1c9b080276c9..2ca0959ae9e6 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;
 };
 
 static inline struct amd_iommu *dev_to_amd_iommu(struct device *dev)

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 2/2] iommu/amd: Add basic debugfs infrastructure for AMD IOMMU
@ 2018-05-07 20:02   ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-07 20:02 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Implement a skeleton framework for debugfs support in the
AMD IOMMU.


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

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 74cfbc392862..dd980f7dd8b6 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -30,3 +30,8 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
 obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
 obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
+
+# This ensures that only the required files are compiled
+ifeq ($(CONFIG_IOMMU_DEBUGFS), y)
+obj-$(CONFIG_AMD_IOMMU) += amd_iommu_debugfs.o
+endif
diff --git a/drivers/iommu/amd_iommu_debugfs.c b/drivers/iommu/amd_iommu_debugfs.c
new file mode 100644
index 000000000000..2b351b9f9130
--- /dev/null
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -0,0 +1,41 @@
+// 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>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/iommu.h>
+#include <linux/pci.h>
+#include "amd_iommu_proto.h"
+#include "amd_iommu_types.h"
+
+static struct dentry *amd_iommu_debugfs;
+static DEFINE_MUTEX(amd_iommu_debugfs_lock);
+
+#define	MAX_NAME_LEN	20
+
+void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
+{
+	char name[MAX_NAME_LEN + 1];
+
+pr_warn("GRH: %s:%d\n", __func__, __LINE__);
+
+	mutex_lock(&amd_iommu_debugfs_lock);
+	if (!amd_iommu_debugfs)
+		amd_iommu_debugfs = iommu_debugfs_new_driver_dir("amd");
+	mutex_unlock(&amd_iommu_debugfs_lock);
+
+	if (amd_iommu_debugfs) {
+		snprintf(name, MAX_NAME_LEN, "iommu%02d", iommu->index);
+		iommu->debugfs = debugfs_create_dir(name,
+						    amd_iommu_debugfs);
+		if (!iommu->debugfs) {
+			debugfs_remove_recursive(amd_iommu_debugfs);
+			amd_iommu_debugfs = NULL;
+		}
+	}
+}
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 904c575d1677..031e6dbb8345 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..39053f11dda3 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -33,6 +33,12 @@ extern void amd_iommu_uninit_devices(void);
 extern void amd_iommu_init_notifier(void);
 extern int amd_iommu_init_api(void);
 
+#ifdef CONFIG_IOMMU_DEBUGFS
+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 1c9b080276c9..2ca0959ae9e6 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;
 };
 
 static inline struct amd_iommu *dev_to_amd_iommu(struct device *dev)

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
  2018-05-07 20:02   ` Gary R Hook
  (?)
@ 2018-05-07 23:47   ` kbuild test robot
  2018-05-08 17:08     ` Hook, Gary
  -1 siblings, 1 reply; 13+ messages in thread
From: kbuild test robot @ 2018-05-07 23:47 UTC (permalink / raw)
  To: Gary R Hook; +Cc: kbuild-all, iommu, joro, linux-kernel

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

Hi Gary,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iommu/next]
[also build test ERROR on v4.17-rc4 next-20180507]
[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/iommu-Enable-debugfs-exposure-of-IOMMU-driver-internals/20180508-062918
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-randconfig-x016-201818 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/intel-iommu.h:32:0,
                    from drivers/gpu/drm/i915/i915_drv.h:41,
                    from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
   include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
>> include/linux/iommu.h:706:8: error: parameter name omitted
    struct dentry *iommu_debugfs_new_driver_dir(char *) {};
           ^~~~~~
   In file included from include/linux/intel-iommu.h:32:0,
                    from drivers/gpu/drm/i915/i915_drv.h:41,
                    from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>> include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
    struct dentry *iommu_debugfs_new_driver_dir(char *) {};
           ^~~~~~

vim +706 include/linux/iommu.h

   700	
   701	#ifdef CONFIG_IOMMU_DEBUGFS
   702	void iommu_debugfs_setup(void);
   703	struct dentry *iommu_debugfs_new_driver_dir(char *);
   704	#else
   705	static inline void iommu_debugfs_setup(void) {}
 > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
   707	#endif
   708	

---
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: 29250 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
  2018-05-07 23:47   ` kbuild test robot
@ 2018-05-08 17:08     ` Hook, Gary
  2018-05-08 18:48       ` Joe Perches
  0 siblings, 1 reply; 13+ messages in thread
From: Hook, Gary @ 2018-05-08 17:08 UTC (permalink / raw)
  To: kbuild test robot, Gary R Hook; +Cc: kbuild-all, iommu, joro, linux-kernel

On 5/7/2018 6:47 PM, kbuild test robot wrote:
> Hi Gary,
> 

I imagine these questions have been asked before, so I'll ask for
forgiveness up front.


> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on iommu/next]
> [also build test ERROR on v4.17-rc4 next-20180507]
> [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/iommu-Enable-debugfs-exposure-of-IOMMU-driver-internals/20180508-062918
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
> config: x86_64-randconfig-x016-201818 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=x86_64

Is the gcc 7 compiler now a requirement to build the kernel? Or only to 
run krobot tests?

Is this the earliest version of the compiler that can be used? I'm still 
using 4.8 and 5.4, which seems to suffice for the kernel.

Googling about this has been fruitless.

> 
> All error/warnings (new ones prefixed by >>):
> 
>     In file included from include/linux/intel-iommu.h:32:0,
>                      from drivers/gpu/drm/i915/i915_drv.h:41,
>                      from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>     include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
>>> include/linux/iommu.h:706:8: error: parameter name omitted
>      struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>             ^~~~~~
>     In file included from include/linux/intel-iommu.h:32:0,
>                      from drivers/gpu/drm/i915/i915_drv.h:41,
>                      from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>> include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
>      struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>             ^~~~~~
> 
> vim +706 include/linux/iommu.h
> 
>     700	
>     701	#ifdef CONFIG_IOMMU_DEBUGFS
>     702	void iommu_debugfs_setup(void);
>     703	struct dentry *iommu_debugfs_new_driver_dir(char *);
>     704	#else
>     705	static inline void iommu_debugfs_setup(void) {}
>   > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>     707	#endif
>     708	

I have no problems with adding parameter names. But 
scripts/checkpatch.pl doesn't seem to check for this, nor require it. 
Should checkpatch be updated?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
  2018-05-08 17:08     ` Hook, Gary
@ 2018-05-08 18:48       ` Joe Perches
  2018-05-08 20:07           ` Gary R Hook
  0 siblings, 1 reply; 13+ messages in thread
From: Joe Perches @ 2018-05-08 18:48 UTC (permalink / raw)
  To: Hook, Gary, kbuild test robot, Gary R Hook
  Cc: kbuild-all, iommu, joro, linux-kernel

On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
> On 5/7/2018 6:47 PM, kbuild test robot wrote:
> > Hi Gary,
> > 
> 
> I imagine these questions have been asked before, so I'll ask for
> forgiveness up front.
> 
> 
> > Thank you for the patch! Yet something to improve:
> > 
> > [auto build test ERROR on iommu/next]
> > [also build test ERROR on v4.17-rc4 next-20180507]
> > [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/iommu-Enable-debugfs-exposure-of-IOMMU-driver-internals/20180508-062918
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
> > config: x86_64-randconfig-x016-201818 (attached as .config)
> > compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> > reproduce:
> >          # save the attached .config to linux build tree
> >          make ARCH=x86_64
> 
> Is the gcc 7 compiler now a requirement to build the kernel? Or only to 
> run krobot tests?
> 
> Is this the earliest version of the compiler that can be used? I'm still 
> using 4.8 and 5.4, which seems to suffice for the kernel.
> 
> Googling about this has been fruitless.
> 
> > 
> > All error/warnings (new ones prefixed by >>):
> > 
> >     In file included from include/linux/intel-iommu.h:32:0,
> >                      from drivers/gpu/drm/i915/i915_drv.h:41,
> >                      from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> >     include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
> > > > include/linux/iommu.h:706:8: error: parameter name omitted
> > 
> >      struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> >             ^~~~~~
> >     In file included from include/linux/intel-iommu.h:32:0,
> >                      from drivers/gpu/drm/i915/i915_drv.h:41,
> >                      from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > > include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
> > 
> >      struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> >             ^~~~~~
> > 
> > vim +706 include/linux/iommu.h
> > 
> >     700	
> >     701	#ifdef CONFIG_IOMMU_DEBUGFS
> >     702	void iommu_debugfs_setup(void);
> >     703	struct dentry *iommu_debugfs_new_driver_dir(char *);
> >     704	#else
> >     705	static inline void iommu_debugfs_setup(void) {}
> >   > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> >     707	#endif
> >     708	
> 
> I have no problems with adding parameter names. But 
> scripts/checkpatch.pl doesn't seem to check for this, nor require it. 
> Should checkpatch be updated?

I'm pretty sure that's not feasible.

And when the compiler tells you you've stuffed up some
syntactical bit, why should checkpatch duplicate the
output error message too?

btw:  That's an unnecessary ; at the end of that non-void
function and it should probably be something like:

static inline struct dentry *iommu_debugfs_new_driver_dir(char *dir)
{
	return NULL;
}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
@ 2018-05-08 20:07           ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-08 20:07 UTC (permalink / raw)
  To: Joe Perches, Hook, Gary, kbuild test robot
  Cc: kbuild-all, iommu, joro, linux-kernel

On 05/08/2018 01:48 PM, Joe Perches wrote:
> On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
>> On 5/7/2018 6:47 PM, kbuild test robot wrote:
>>>
>>> All error/warnings (new ones prefixed by >>):
>>>
>>>      In file included from include/linux/intel-iommu.h:32:0,
>>>                       from drivers/gpu/drm/i915/i915_drv.h:41,
>>>                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>      include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
>>>>> include/linux/iommu.h:706:8: error: parameter name omitted
>>>
>>>       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>              ^~~~~~
>>>      In file included from include/linux/intel-iommu.h:32:0,
>>>                       from drivers/gpu/drm/i915/i915_drv.h:41,
>>>                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>>> include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
>>>
>>>       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>              ^~~~~~
>>>
>>> vim +706 include/linux/iommu.h
>>>
>>>      700	
>>>      701	#ifdef CONFIG_IOMMU_DEBUGFS
>>>      702	void iommu_debugfs_setup(void);
>>>      703	struct dentry *iommu_debugfs_new_driver_dir(char *);
>>>      704	#else
>>>      705	static inline void iommu_debugfs_setup(void) {}
>>>    > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>      707	#endif
>>>      708	
>>
>> I have no problems with adding parameter names. But
>> scripts/checkpatch.pl doesn't seem to check for this, nor require it.
>> Should checkpatch be updated?
> 
> I'm pretty sure that's not feasible.

Ugh. This is a definition, not a declaration. My bad. Which is likely 
why I decided to apologize up front.

> And when the compiler tells you you've stuffed up some
> syntactical bit, why should checkpatch duplicate the
> output error message too?

Well, that's the point: neither the 4.8 nor 5.4 compiler complained 
about this. Not as an error, despite the fact that (now that I read what 
is actually here, as opposed to what I think is there) this is wrong. 
Had an error message been emitted, and the make stopped, I would have 
figure this out before embarrassing myself in front of the entire interwebs.

> btw:  That's an unnecessary ; at the end of that non-void
> function and it should probably be something like:

You are correct, sir. I've made a change on this.

> 
> static inline struct dentry *iommu_debugfs_new_driver_dir(char *dir)
> {
> 	return NULL;
> }

Thanks for taking a few moments to comment. Much appreciated.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
@ 2018-05-08 20:07           ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-08 20:07 UTC (permalink / raw)
  To: Joe Perches, Hook, Gary, kbuild test robot
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	kbuild-all-JC7UmRfGjtg

On 05/08/2018 01:48 PM, Joe Perches wrote:
> On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
>> On 5/7/2018 6:47 PM, kbuild test robot wrote:
>>>
>>> All error/warnings (new ones prefixed by >>):
>>>
>>>      In file included from include/linux/intel-iommu.h:32:0,
>>>                       from drivers/gpu/drm/i915/i915_drv.h:41,
>>>                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>      include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
>>>>> include/linux/iommu.h:706:8: error: parameter name omitted
>>>
>>>       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>              ^~~~~~
>>>      In file included from include/linux/intel-iommu.h:32:0,
>>>                       from drivers/gpu/drm/i915/i915_drv.h:41,
>>>                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>>> include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
>>>
>>>       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>              ^~~~~~
>>>
>>> vim +706 include/linux/iommu.h
>>>
>>>      700	
>>>      701	#ifdef CONFIG_IOMMU_DEBUGFS
>>>      702	void iommu_debugfs_setup(void);
>>>      703	struct dentry *iommu_debugfs_new_driver_dir(char *);
>>>      704	#else
>>>      705	static inline void iommu_debugfs_setup(void) {}
>>>    > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>      707	#endif
>>>      708	
>>
>> I have no problems with adding parameter names. But
>> scripts/checkpatch.pl doesn't seem to check for this, nor require it.
>> Should checkpatch be updated?
> 
> I'm pretty sure that's not feasible.

Ugh. This is a definition, not a declaration. My bad. Which is likely 
why I decided to apologize up front.

> And when the compiler tells you you've stuffed up some
> syntactical bit, why should checkpatch duplicate the
> output error message too?

Well, that's the point: neither the 4.8 nor 5.4 compiler complained 
about this. Not as an error, despite the fact that (now that I read what 
is actually here, as opposed to what I think is there) this is wrong. 
Had an error message been emitted, and the make stopped, I would have 
figure this out before embarrassing myself in front of the entire interwebs.

> btw:  That's an unnecessary ; at the end of that non-void
> function and it should probably be something like:

You are correct, sir. I've made a change on this.

> 
> static inline struct dentry *iommu_debugfs_new_driver_dir(char *dir)
> {
> 	return NULL;
> }

Thanks for taking a few moments to comment. Much appreciated.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
  2018-05-08 20:07           ` Gary R Hook
  (?)
@ 2018-05-08 20:42           ` Joe Perches
  2018-05-08 20:48               ` Gary R Hook
  -1 siblings, 1 reply; 13+ messages in thread
From: Joe Perches @ 2018-05-08 20:42 UTC (permalink / raw)
  To: Gary R Hook, Hook, Gary, kbuild test robot
  Cc: kbuild-all, iommu, joro, linux-kernel

On Tue, 2018-05-08 at 15:07 -0500, Gary R Hook wrote:
> On 05/08/2018 01:48 PM, Joe Perches wrote:
> > On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
> > > On 5/7/2018 6:47 PM, kbuild test robot wrote:
> > > > 
> > > > All error/warnings (new ones prefixed by >>):
> > > > 
> > > >      In file included from include/linux/intel-iommu.h:32:0,
> > > >                       from drivers/gpu/drm/i915/i915_drv.h:41,
> > > >                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > >      include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
> > > > > > include/linux/iommu.h:706:8: error: parameter name omitted
> > > > 
> > > >       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > >              ^~~~~~
> > > >      In file included from include/linux/intel-iommu.h:32:0,
> > > >                       from drivers/gpu/drm/i915/i915_drv.h:41,
> > > >                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > > > > include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
> > > > 
> > > >       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > >              ^~~~~~
> > > > 
> > > > vim +706 include/linux/iommu.h
> > > > 
> > > >      700	
> > > >      701	#ifdef CONFIG_IOMMU_DEBUGFS
> > > >      702	void iommu_debugfs_setup(void);
> > > >      703	struct dentry *iommu_debugfs_new_driver_dir(char *);
> > > >      704	#else
> > > >      705	static inline void iommu_debugfs_setup(void) {}
> > > >    > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > >      707	#endif
> > > >      708	
> > > 
> > > I have no problems with adding parameter names. But
> > > scripts/checkpatch.pl doesn't seem to check for this, nor require it.
> > > Should checkpatch be updated?
> > 
> > I'm pretty sure that's not feasible.
> 
> Ugh. This is a definition, not a declaration. My bad. Which is likely 
> why I decided to apologize up front.
> 
> > And when the compiler tells you you've stuffed up some
> > syntactical bit, why should checkpatch duplicate the
> > output error message too?
> 
> Well, that's the point: neither the 4.8 nor 5.4 compiler complained 
> about this.

Perhaps because CONFIG_IOMMU_DEBUGFS was set in the .config
for all the compilation previously performed?

>  Not as an error, despite the fact that (now that I read what 
> is actually here, as opposed to what I think is there) this is wrong. 
> Had an error message been emitted, and the make stopped, I would have 
> figure this out before embarrassing myself in front of the entire interwebs.

There's no reason for that figuring out to be necessary.
Linux compilation complexity is pretty high and almost
no one understands it completely.

cheers, Joe

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
@ 2018-05-08 20:48               ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-08 20:48 UTC (permalink / raw)
  To: Joe Perches, Hook, Gary, kbuild test robot
  Cc: kbuild-all, iommu, joro, linux-kernel

On 05/08/2018 03:42 PM, Joe Perches wrote:
> On Tue, 2018-05-08 at 15:07 -0500, Gary R Hook wrote:
>> On 05/08/2018 01:48 PM, Joe Perches wrote:
>>> On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
>>>> On 5/7/2018 6:47 PM, kbuild test robot wrote:
>>>>>
>>>>> All error/warnings (new ones prefixed by >>):
>>>>>
>>>>>       In file included from include/linux/intel-iommu.h:32:0,
>>>>>                        from drivers/gpu/drm/i915/i915_drv.h:41,
>>>>>                        from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>>>       include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
>>>>>>> include/linux/iommu.h:706:8: error: parameter name omitted
>>>>>
>>>>>        struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>>>               ^~~~~~
>>>>>       In file included from include/linux/intel-iommu.h:32:0,
>>>>>                        from drivers/gpu/drm/i915/i915_drv.h:41,
>>>>>                        from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>>>>> include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
>>>>>
>>>>>        struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>>>               ^~~~~~
>>>>>
>>>>> vim +706 include/linux/iommu.h
>>>>>
>>>>>       700	
>>>>>       701	#ifdef CONFIG_IOMMU_DEBUGFS
>>>>>       702	void iommu_debugfs_setup(void);
>>>>>       703	struct dentry *iommu_debugfs_new_driver_dir(char *);
>>>>>       704	#else
>>>>>       705	static inline void iommu_debugfs_setup(void) {}
>>>>>     > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>>>       707	#endif
>>>>>       708	
>>>>
>>>> I have no problems with adding parameter names. But
>>>> scripts/checkpatch.pl doesn't seem to check for this, nor require it.
>>>> Should checkpatch be updated?
>>>
>>> I'm pretty sure that's not feasible.
>>
>> Ugh. This is a definition, not a declaration. My bad. Which is likely
>> why I decided to apologize up front.
>>
>>> And when the compiler tells you you've stuffed up some
>>> syntactical bit, why should checkpatch duplicate the
>>> output error message too?
>>
>> Well, that's the point: neither the 4.8 nor 5.4 compiler complained
>> about this.
> 
> Perhaps because CONFIG_IOMMU_DEBUGFS was set in the .config
> for all the compilation previously performed?

Well, you'd think maybe so, but I forced a recompilation of that one 
file (i915_oa_bxt.c) and no complaint with 5.4. Weird.

Ah, well. Onward to patch version 6.

Thanks again.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
@ 2018-05-08 20:48               ` Gary R Hook
  0 siblings, 0 replies; 13+ messages in thread
From: Gary R Hook @ 2018-05-08 20:48 UTC (permalink / raw)
  To: Joe Perches, Hook, Gary, kbuild test robot
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	kbuild-all-JC7UmRfGjtg

On 05/08/2018 03:42 PM, Joe Perches wrote:
> On Tue, 2018-05-08 at 15:07 -0500, Gary R Hook wrote:
>> On 05/08/2018 01:48 PM, Joe Perches wrote:
>>> On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
>>>> On 5/7/2018 6:47 PM, kbuild test robot wrote:
>>>>>
>>>>> All error/warnings (new ones prefixed by >>):
>>>>>
>>>>>       In file included from include/linux/intel-iommu.h:32:0,
>>>>>                        from drivers/gpu/drm/i915/i915_drv.h:41,
>>>>>                        from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>>>       include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
>>>>>>> include/linux/iommu.h:706:8: error: parameter name omitted
>>>>>
>>>>>        struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>>>               ^~~~~~
>>>>>       In file included from include/linux/intel-iommu.h:32:0,
>>>>>                        from drivers/gpu/drm/i915/i915_drv.h:41,
>>>>>                        from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
>>>>>>> include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
>>>>>
>>>>>        struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>>>               ^~~~~~
>>>>>
>>>>> vim +706 include/linux/iommu.h
>>>>>
>>>>>       700	
>>>>>       701	#ifdef CONFIG_IOMMU_DEBUGFS
>>>>>       702	void iommu_debugfs_setup(void);
>>>>>       703	struct dentry *iommu_debugfs_new_driver_dir(char *);
>>>>>       704	#else
>>>>>       705	static inline void iommu_debugfs_setup(void) {}
>>>>>     > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
>>>>>       707	#endif
>>>>>       708	
>>>>
>>>> I have no problems with adding parameter names. But
>>>> scripts/checkpatch.pl doesn't seem to check for this, nor require it.
>>>> Should checkpatch be updated?
>>>
>>> I'm pretty sure that's not feasible.
>>
>> Ugh. This is a definition, not a declaration. My bad. Which is likely
>> why I decided to apologize up front.
>>
>>> And when the compiler tells you you've stuffed up some
>>> syntactical bit, why should checkpatch duplicate the
>>> output error message too?
>>
>> Well, that's the point: neither the 4.8 nor 5.4 compiler complained
>> about this.
> 
> Perhaps because CONFIG_IOMMU_DEBUGFS was set in the .config
> for all the compilation previously performed?

Well, you'd think maybe so, but I forced a recompilation of that one 
file (i915_oa_bxt.c) and no complaint with 5.4. Weird.

Ah, well. Onward to patch version 6.

Thanks again.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-05-08 20:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 20:01 [PATCH v5 0/2] Base enablement of IOMMU debugfs support Gary R Hook
2018-05-07 20:02 ` [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals Gary R Hook
2018-05-07 20:02   ` Gary R Hook
2018-05-07 23:47   ` kbuild test robot
2018-05-08 17:08     ` Hook, Gary
2018-05-08 18:48       ` Joe Perches
2018-05-08 20:07         ` Gary R Hook
2018-05-08 20:07           ` Gary R Hook
2018-05-08 20:42           ` Joe Perches
2018-05-08 20:48             ` Gary R Hook
2018-05-08 20:48               ` Gary R Hook
2018-05-07 20:02 ` [PATCH v5 2/2] iommu/amd: Add basic debugfs infrastructure for AMD IOMMU Gary R Hook
2018-05-07 20:02   ` 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.