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
--
Implement a skeleton framework for debugfs support in the
AMD IOMMU.
Signed-off-by: Gary R Hook <[email protected]>
---
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 <[email protected]>
+ */
+
+#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)
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 <[email protected]>
---
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 <[email protected]>
+ */
+
+#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 */
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
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?
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;
}
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.
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
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.