[PATCH v3 0/6] Intel IOMMU debugfs support

[PATCH v3 0/6] Intel IOMMU debugfs support

[Sohil Mehta]

Hi all,

This series aims to add debugfs support for Intel IOMMU. It exposes IOMMU
registers, internal context and dumps individual table entries to help debug
Intel IOMMUs.

The first patch does the ground work for the following patches by creating a
new Kconfig option - INTEL_IOMMU_DEBUG. It also reorganizes some Intel IOMMU
data structures. The next five patches add debugfs support for IOMMU context
internals, extended context, register contents, PASID internals, and Interrupt
remapping in that order. The information can be accessed in sysfs at
'/sys/kernel/debug/intel_iommu/'.

Regards,
Sohil

Changes since v2:
 - Added a macro for seq file operations based on recommendation by Andy 
   Shevchenko. The marco can be moved to seq_file.h at a future point
 - Changed the debugfs file names to more relevant ones
 - Added information for MTRR registers in the regset file

Changes since v1:
 - Fixed seq_printf formatting
 - Handled the case when Interrupt remapping is not enabled

Gayatri Kammela (5):
  iommu/vt-d: Add debugfs support for Intel IOMMU internals
  iommu/vt-d: Add Intel IOMMU debugfs to show context internals
  iommu/vt-d: Add Intel IOMMU debugfs to show extended context internals
  iommu/vt-d: Add debugfs extension to show register contents
  iommu/vt-d: Add debugfs extension to show Pasid table contents

Sohil Mehta (1):
  iommu/vt-d: Add debugfs support for Intel IOMMU Interrupt remapping

 drivers/iommu/Kconfig             |  10 +
 drivers/iommu/Makefile            |   1 +
 drivers/iommu/intel-iommu-debug.c | 417 ++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-iommu.c       |  35 +---
 drivers/iommu/intel-svm.c         |   8 -
 include/linux/intel-iommu.h       |  34 ++++
 include/linux/intel-svm.h         |  10 +-
 7 files changed, 478 insertions(+), 37 deletions(-)
 create mode 100644 drivers/iommu/intel-iommu-debug.c

-- 
2.7.4

[PATCH v3 1/6] iommu/vt-d: Add debugfs support for Intel IOMMU internals

[Sohil Mehta]

From: Gayatri Kammela <gayatri.kammela@intel.com>

Enable Intel IOMMU debug to export Intel IOMMU internals in debugfs

Cc: Sohil Mehta <sohil.mehta@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
---

v3: No change

v2: No change

 drivers/iommu/Kconfig       | 10 ++++++++++
 drivers/iommu/intel-iommu.c | 31 +++----------------------------
 include/linux/intel-iommu.h | 32 ++++++++++++++++++++++++++++++++
 include/linux/intel-svm.h   |  2 +-
 4 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index f3a2134..d7588ee 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -152,6 +152,16 @@ config INTEL_IOMMU
 	  and include PCI device scope covered by these DMA
 	  remapping devices.
 
+config INTEL_IOMMU_DEBUG
+	bool "Export Intel IOMMU internals in DebugFS"
+	depends on INTEL_IOMMU && DEBUG_FS
+	default n
+	help
+	  IOMMU internal states such as context, PASID tables can be seen via
+	  debugfs. Select this option if you want to export these internals.
+
+	  Say Y if you need this.
+
 config INTEL_IOMMU_SVM
 	bool "Support for Shared Virtual Memory with Intel IOMMU"
 	depends on INTEL_IOMMU && X86
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 6784a05..419a559 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -185,16 +185,6 @@ static int rwbf_quirk;
 static int force_on = 0;
 int intel_iommu_tboot_noforce;
 
-/*
- * 0: Present
- * 1-11: Reserved
- * 12-63: Context Ptr (12 - (haw-1))
- * 64-127: Reserved
- */
-struct root_entry {
-	u64	lo;
-	u64	hi;
-};
 #define ROOT_ENTRY_NR (VTD_PAGE_SIZE/sizeof(struct root_entry))
 
 /*
@@ -220,21 +210,6 @@ static phys_addr_t root_entry_uctp(struct root_entry *re)
 
 	return re->hi & VTD_PAGE_MASK;
 }
-/*
- * low 64 bits:
- * 0: present
- * 1: fault processing disable
- * 2-3: translation type
- * 12-63: address space root
- * high 64 bits:
- * 0-2: address width
- * 3-6: aval
- * 8-23: domain id
- */
-struct context_entry {
-	u64 lo;
-	u64 hi;
-};
 
 static inline void context_clear_pasid_enable(struct context_entry *context)
 {
@@ -261,7 +236,7 @@ static inline bool __context_present(struct context_entry *context)
 	return (context->lo & 1);
 }
 
-static inline bool context_present(struct context_entry *context)
+bool context_present(struct context_entry *context)
 {
 	return context_pasid_enabled(context) ?
 	     __context_present(context) :
@@ -821,7 +796,7 @@ static void domain_update_iommu_cap(struct dmar_domain *domain)
 	domain->iommu_superpage = domain_update_iommu_superpage(NULL);
 }
 
-static inline struct context_entry *iommu_context_addr(struct intel_iommu *iommu,
+struct context_entry *iommu_context_addr(struct intel_iommu *iommu,
 						       u8 bus, u8 devfn, int alloc)
 {
 	struct root_entry *root = &iommu->root_entry[bus];
@@ -5200,7 +5175,7 @@ static void intel_iommu_put_resv_regions(struct device *dev,
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 #define MAX_NR_PASID_BITS (20)
-static inline unsigned long intel_iommu_get_pts(struct intel_iommu *iommu)
+unsigned long intel_iommu_get_pts(struct intel_iommu *iommu)
 {
 	/*
 	 * Convert ecap_pss to extend context entry pts encoding, also
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 485a5b4..defbc32 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -382,6 +382,33 @@ struct pasid_entry;
 struct pasid_state_entry;
 struct page_req_dsc;
 
+/*
+ * 0: Present
+ * 1-11: Reserved
+ * 12-63: Context Ptr (12 - (haw-1))
+ * 64-127: Reserved
+ */
+struct root_entry {
+	u64     lo;
+	u64     hi;
+};
+
+/*
+ * low 64 bits:
+ * 0: present
+ * 1: fault processing disable
+ * 2-3: translation type
+ * 12-63: address space root
+ * high 64 bits:
+ * 0-2: address width
+ * 3-6: aval
+ * 8-23: domain id
+ */
+struct context_entry {
+	u64 lo;
+	u64 hi;
+};
+
 struct intel_iommu {
 	void __iomem	*reg; /* Pointer to hardware regs, virtual addr */
 	u64 		reg_phys; /* physical address of hw register set */
@@ -487,8 +514,13 @@ struct intel_svm {
 
 extern int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev);
 extern struct intel_iommu *intel_svm_device_to_iommu(struct device *dev);
+extern unsigned long intel_iommu_get_pts(struct intel_iommu *iommu);
 #endif
 
 extern const struct attribute_group *intel_iommu_groups[];
+extern void intel_iommu_debugfs_init(void);
+extern bool context_present(struct context_entry *context);
+extern struct context_entry *iommu_context_addr(struct intel_iommu *iommu,
+						u8 bus, u8 devfn, int alloc);
 
 #endif
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 99bc5b3..733eaf9 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -130,7 +130,7 @@ static inline int intel_svm_unbind_mm(struct device *dev, int pasid)
 	BUG();
 }
 
-static int intel_svm_is_pasid_valid(struct device *dev, int pasid)
+static inline int intel_svm_is_pasid_valid(struct device *dev, int pasid)
 {
 	return -EINVAL;
 }
-- 
2.7.4

[PATCH v3 2/6] iommu/vt-d: Add Intel IOMMU debugfs to show context internals

[Sohil Mehta]

From: Gayatri Kammela <gayatri.kammela@intel.com>

IOMMU internals states such as root and context can be exported to the
userspace.

Example of such dump in Kabylake:

root@OTC-KBLH-01:~# cat
/sys/kernel/debug/intel_iommu/dmar_translation_struct

IOMMU dmar2:    Root Table Addr:4558a3000
 Root tbl entries:
Bus 0 L: 4558aa001 H: 0
 Context table entries for Bus: 0
[entry] DID :B :D .F    Low             High
[160]   0000:00:14.00   4558a9001       102
[184]   0000:00:17.00   400eac001       302
[248]   0000:00:1f.00   4558af001       202
[251]   0000:00:1f.03   40070e001       502
[254]   0000:00:1f.06   4467c9001       602
 Root tbl entries:
Bus 1 L: 3fc8c2001 H: 0
 Context table entries for Bus: 1
[entry] DID :B :D .F    Low             High
[0]     0000:01:00.00   3fc8c3001       402

Cc: Sohil Mehta <sohil.mehta@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
---

v3: Add a macro for seq file operations 
    Change the intel_iommu_ctx file name to dmar_translation_struct

v2: No change

 drivers/iommu/Makefile            |   1 +
 drivers/iommu/intel-iommu-debug.c | 152 ++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-iommu.c       |   4 +
 3 files changed, 157 insertions(+)
 create mode 100644 drivers/iommu/intel-iommu-debug.c

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb6958..fdbaf46 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
 obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o
+obj-$(CONFIG_INTEL_IOMMU_DEBUG) += intel-iommu-debug.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
 obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o
diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
new file mode 100644
index 0000000..8ae0c4d
--- /dev/null
+++ b/drivers/iommu/intel-iommu-debug.c
@@ -0,0 +1,152 @@
+/*
+ * Copyright © 2017 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * Authors: Gayatri Kammela <gayatri.kammela@intel.com>
+ *          Jacob Pan <jacob.jun.pan@linux.intel.com>
+ *
+ */
+
+#define pr_fmt(fmt)     "INTEL_IOMMU: " fmt
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/pm_runtime.h>
+#include <linux/debugfs.h>
+#include <linux/pci.h>
+#include <linux/iommu.h>
+#include <linux/intel-iommu.h>
+#include <linux/intel-svm.h>
+#include <linux/dmar.h>
+#include <linux/spinlock.h>
+
+#include "irq_remapping.h"
+
+#define TOTAL_BUS_NR (256) /* full bus range 256 */
+#define DEFINE_SHOW_ATTRIBUTE(__name)					\
+static int __name ## _open(struct inode *inode, struct file *file)	\
+{									\
+	return single_open(file, __name ## _show, inode->i_private);	\
+}									\
+static const struct file_operations __name ## _fops =			\
+{									\
+	.open		= __name ## _open,				\
+	.read		= seq_read,					\
+	.llseek		= seq_lseek,					\
+	.release	= single_release,				\
+	.owner		= THIS_MODULE,					\
+}
+
+static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
+			       struct intel_iommu *iommu, int bus, bool ext,
+			       bool new_ext)
+{
+	struct context_entry *context;
+	int ctx;
+	unsigned long flags;
+
+	seq_printf(m, "%s Context table entries for Bus: %d\n",
+		   ext ? "Lower" : "", bus);
+	seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n");
+
+	spin_lock_irqsave(&iommu->lock, flags);
+
+	/* Publish either context entries or extended contenxt entries */
+	for (ctx = 0; ctx < (ext ? 128 : 256); ctx++) {
+		context = iommu_context_addr(iommu, bus, ctx, 0);
+		if (!context)
+			goto out;
+		if (context_present(context)) {
+			seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n",
+				   ctx, iommu->segment, bus, PCI_SLOT(ctx),
+				   PCI_FUNC(ctx), context[0].lo, context[0].hi);
+		}
+	}
+out:
+	spin_unlock_irqrestore(&iommu->lock, flags);
+}
+
+static void root_tbl_entry_show(struct seq_file *m, void *unused,
+				struct intel_iommu *iommu, u64 rtaddr_reg,
+				bool ext, bool new_ext)
+{
+	int bus;
+
+	seq_printf(m, "\nIOMMU %s: %2s Root Table Addr:%llx\n", iommu->name,
+		   ext ? "Extended" : "", rtaddr_reg);
+	/* Publish extended root table entries or root table entries here */
+	for (bus = 0; bus < TOTAL_BUS_NR; bus++) {
+		if (!iommu->root_entry[bus].lo)
+			continue;
+
+		seq_printf(m, "%s Root tbl entries:\n", ext ? "Extended" : "");
+		seq_printf(m, "Bus %d L: %llx H: %llx\n", bus,
+			   iommu->root_entry[bus].lo, iommu->root_entry[bus].hi
+			  );
+
+		ctx_tbl_entry_show(m, unused, iommu, bus, ext, new_ext);
+	}
+}
+
+static int dmar_translation_struct_show(struct seq_file *m, void *unused)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	u64 rtaddr_reg;
+	bool new_ext, ext;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (iommu) {
+			/* Check if root table type is set */
+			rtaddr_reg = dmar_readq(iommu->reg + DMAR_RTADDR_REG);
+			ext        = !!(rtaddr_reg & DMA_RTADDR_RTT);
+			new_ext    = !!ecap_ecs(iommu->ecap);
+			if (new_ext != ext) {
+				seq_printf(m, "IOMMU %s: invalid ecs\n",
+					   iommu->name);
+				rcu_read_unlock();
+				return -EINVAL;
+			}
+			root_tbl_entry_show(m, unused, iommu, rtaddr_reg, ext,
+					    new_ext);
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(dmar_translation_struct);
+
+void __init intel_iommu_debugfs_init(void)
+{
+	struct dentry *iommu_debug_root;
+
+	iommu_debug_root = debugfs_create_dir("intel_iommu", NULL);
+
+	if (!iommu_debug_root) {
+		pr_err("can't create debugfs dir\n");
+		goto err;
+	}
+
+	if (!debugfs_create_file("dmar_translation_struct", S_IRUGO,
+				 iommu_debug_root, NULL,
+				 &dmar_translation_struct_fops)) {
+		pr_err("Can't create dmar_translation_struct file\n");
+		goto err;
+	}
+
+err:
+	debugfs_remove_recursive(iommu_debug_root);
+
+}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 419a559..7794e0c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4797,6 +4797,10 @@ int __init intel_iommu_init(void)
 			  intel_iommu_cpu_dead);
 	intel_iommu_enabled = 1;
 
+#ifdef CONFIG_INTEL_IOMMU_DEBUG
+	intel_iommu_debugfs_init();
+#endif
+
 	return 0;
 
 out_free_reserved_range:
-- 
2.7.4

[PATCH v3 3/6] iommu/vt-d: Add Intel IOMMU debugfs to show extended context internals

[Sohil Mehta]

From: Gayatri Kammela <gayatri.kammela@intel.com>

Debugfs extension to dump internals such as extended context table
entries for each IOMMU to the userspace.

root@OTC-KBLH-01:~# cat
/sys/kernel/debug/intel_iommu/dmar_translation_struct

IOMMU dmar1: Extended Root Table Addr:4558a1800
Extended Root tbl entries:
Bus 0 L: 4558a6001 H: 0
Lower Context table entries for Bus: 0
[entry] DID :B :D .F    Low             High
[16]    0000:00:02.00   4558a5005       102
Higher Context tbl entries for Bus: 0
[16]    0000:00:02.00   401b0000c       401400000

IOMMU dmar0: Extended Root Table Addr:4558a2800
Extended Root tbl entries:
Bus 0 L: 4016f4001 H: 0
Lower Context table entries for Bus: 0
[entry] DID :B :D .F    Low             High
[80]    0000:00:0a.00   4016f3a05       102
Higher Context tbl entries for Bus: 0
[80]    0000:00:0a.00   40150000c       671b80000000

Cc: Sohil Mehta <sohil.mehta@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
---

v3: No change

v2: No change

 drivers/iommu/intel-iommu-debug.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
index 8ae0c4d..8e7f5d2 100644
--- a/drivers/iommu/intel-iommu-debug.c
+++ b/drivers/iommu/intel-iommu-debug.c
@@ -46,6 +46,38 @@ static const struct file_operations __name ## _fops =			\
 	.owner		= THIS_MODULE,					\
 }
 
+#ifdef CONFIG_INTEL_IOMMU_SVM
+static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
+				   struct intel_iommu *iommu, int bus, int ctx,
+				   struct context_entry *context, bool new_ext)
+{
+	u64 ctx_lo;
+
+	if (new_ext) {
+		seq_printf(m, "Higher Context tbl entries for Bus: %d\n", bus);
+		ctx_lo = context[0].lo;
+
+		if (!(ctx_lo & CONTEXT_PASIDE)) {
+			context[1].hi = (u64)virt_to_phys(
+					iommu->pasid_state_table);
+			context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
+					intel_iommu_get_pts(iommu);
+		}
+
+		seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n", ctx,
+			   iommu->segment, bus, PCI_SLOT(ctx), PCI_FUNC(ctx),
+			   context[1].lo, context[1].hi);
+	}
+}
+#else /* CONFIG_INTEL_IOMMU_SVM */
+static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
+				   struct intel_iommu *iommu, int bus, int ctx,
+				   struct context_entry *context, bool new_ext)
+{
+	return;
+}
+#endif /* CONFIG_INTEL_IOMMU_SVM */
+
 static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
 			       struct intel_iommu *iommu, int bus, bool ext,
 			       bool new_ext)
@@ -69,6 +101,9 @@ static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
 			seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n",
 				   ctx, iommu->segment, bus, PCI_SLOT(ctx),
 				   PCI_FUNC(ctx), context[0].lo, context[0].hi);
+
+			ext_ctx_tbl_entry_show(m, unused, iommu, bus, ctx,
+					       context, new_ext);
 		}
 	}
 out:
-- 
2.7.4

[PATCH v3 4/6] iommu/vt-d: Add debugfs extension to show register contents

[Sohil Mehta]

From: Gayatri Kammela <gayatri.kammela@intel.com>

Debugfs extension to dump all the register contents for each IOMMU
device to the user space via debugfs.

example:
root@OTC-KBLH-01:~# cat /sys/kernel/debug/intel_iommu/iommu_regset

DMAR: dmar1: reg_base_addr fed90000
Name         Offset     Contents
VER          0x00       0x0000000000000010
CAP          0x08       0x01c0000c40660462
ECAP         0x10       0x0000019e2ff0505e
GCMD         0x18       0x0000000000000000
GSTS         0x1c       0x00000000c7000000
RTADDR       0x20       0x00000004558d6800
CCMD         0x28       0x0800000000000000
FSTS         0x34       0x0000000000000000
FECTL        0x38       0x0000000000000000
FEDATA       0x3c       0xfee0100c00004141

Cc: Sohil Mehta <sohil.mehta@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
---

v3: Use a macro for seq file operations 
    Change the intel_iommu_regset file name to iommu_regset
    Add information for MTRR registers

v2: Fix seq_printf formatting

 drivers/iommu/intel-iommu-debug.c | 86 +++++++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h       |  2 +
 2 files changed, 88 insertions(+)

diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
index 8e7f5d2..0951d58 100644
--- a/drivers/iommu/intel-iommu-debug.c
+++ b/drivers/iommu/intel-iommu-debug.c
@@ -163,6 +163,84 @@ static int dmar_translation_struct_show(struct seq_file *m, void *unused)
 
 DEFINE_SHOW_ATTRIBUTE(dmar_translation_struct);
 
+static int iommu_regset_show(struct seq_file *m, void *unused)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	unsigned long long base;
+	int i;
+	struct regset {
+		int offset;
+		char *regs;
+	};
+
+	static const struct regset regstr[] = {{DMAR_VER_REG, "VER"},
+					       {DMAR_CAP_REG, "CAP"},
+					       {DMAR_ECAP_REG, "ECAP"},
+					       {DMAR_GCMD_REG, "GCMD"},
+					       {DMAR_GSTS_REG, "GSTS"},
+					       {DMAR_RTADDR_REG, "RTADDR"},
+					       {DMAR_CCMD_REG, "CCMD"},
+					       {DMAR_FSTS_REG, "FSTS"},
+					       {DMAR_FECTL_REG, "FECTL"},
+					       {DMAR_FEDATA_REG, "FEDATA"},
+					       {DMAR_FEADDR_REG, "FEADDR"},
+					       {DMAR_FEUADDR_REG, "FEUADDR"},
+					       {DMAR_AFLOG_REG, "AFLOG"},
+					       {DMAR_PMEN_REG, "PMEN"},
+					       {DMAR_PLMBASE_REG, "PLMBASE"},
+					       {DMAR_PLMLIMIT_REG, "PLMLIMIT"},
+					       {DMAR_PHMBASE_REG, "PHMBASE"},
+					       {DMAR_PHMLIMIT_REG,  "PHMLIMIT"},
+					       {DMAR_IQH_REG, "IQH"},
+					       {DMAR_IQT_REG, "IQT"},
+					       {DMAR_IQ_SHIFT, "IQ"},
+					       {DMAR_IQA_REG, "IQA"},
+					       {DMAR_ICS_REG, "ICS"},
+					       {DMAR_IRTA_REG, "IRTA"},
+					       {DMAR_PQH_REG, "PQH"},
+					       {DMAR_PQT_REG, "PQT"},
+					       {DMAR_PQA_REG, "PQA"},
+					       {DMAR_PRS_REG, "PRS"},
+					       {DMAR_PECTL_REG, "PECTL"},
+					       {DMAR_PEDATA_REG, "PEDATA"},
+					       {DMAR_PEADDR_REG, "PEADDR"},
+					       {DMAR_PEUADDR_REG, "PEUADDR"},
+					       {DMAR_MTRRCAP_REG, "MTRRCAP"},
+					       {DMAR_MTRRDEF_REG, "MTRRDEF"} };
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (iommu) {
+			if (!drhd->reg_base_addr) {
+				seq_printf(m, "IOMMU: Invalid base address\n");
+				rcu_read_unlock();
+				return -EINVAL;
+			}
+
+			base = drhd->reg_base_addr;
+			seq_printf(m, "\nDMAR: %s: reg_base_addr %llx\n",
+				   iommu->name, base);
+			seq_printf(m, "Name\t\t\tOffset\t\tContents\n");
+			/*
+			 * Publish the contents of the 64-bit hardware registers
+			 * by adding the offset to the pointer(virtual addr)
+			 */
+			for (i = 0 ; i < ARRAY_SIZE(regstr); i++) {
+				seq_printf(m, "%-8s\t\t0x%02x\t\t0x%016lx\n",
+					   regstr[i].regs, regstr[i].offset,
+					   readq(iommu->reg + regstr[i].offset)
+					  );
+			}
+		}
+	}
+
+	rcu_read_unlock();
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(iommu_regset);
+
 void __init intel_iommu_debugfs_init(void)
 {
 	struct dentry *iommu_debug_root;
@@ -181,6 +259,14 @@ void __init intel_iommu_debugfs_init(void)
 		goto err;
 	}
 
+	if (!debugfs_create_file("iommu_regset", S_IRUGO,
+				 iommu_debug_root, NULL, &iommu_regset_fops)) {
+		pr_err("Can't create iommu_regset file\n");
+		goto err;
+	}
+
+	return;
+
 err:
 	debugfs_remove_recursive(iommu_debug_root);
 
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index defbc32..c2c147b 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -71,6 +71,8 @@
 #define	DMAR_PEDATA_REG	0xe4	/* Page request event interrupt data register */
 #define	DMAR_PEADDR_REG	0xe8	/* Page request event interrupt addr register */
 #define	DMAR_PEUADDR_REG 0xec	/* Page request event Upper address register */
+#define	DMAR_MTRRCAP_REG 0x100	/* Memory type range register capability register */
+#define	DMAR_MTRRDEF_REG 0x108	/* Memory type range register default type register */
 
 #define OFFSET_STRIDE		(9)
 
-- 
2.7.4

[PATCH v3 5/6] iommu/vt-d: Add debugfs extension to show Pasid table contents

[Sohil Mehta]

From: Gayatri Kammela <gayatri.kammela@intel.com>

Debugfs extension to dump the internals such as pasid table entries for
each IOMMU to the userspace.

Example of such dump in Kabylake:

root@OTC-KBLH-01:~# cat
/sys/kernel/debug/intel_iommu/dmar_translation_struct

IOMMU dmar0: Extended Root Table Addr:4310c4800
Extended Root tbl entries:
Bus 0 L: 3ff5a8001 H: 0
Lower Context table entries for Bus: 0
[entry] DID :B :D .F    Low             High
[80]    0000:00:0a.00   3ff5a9a05       102
Higher Context tbl entries for Bus: 0
[80]    0000:00:0a.00   40160000c       725140000000
Pasid Table Addr : ffff8db2c1600000
Pasid table entries for domain: 0000
[Entry]         Contents
[0]             26a609801

Cc: Sohil Mehta <sohil.mehta@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
---

v3: No change

v2: Fix seq_printf formatting

 drivers/iommu/intel-iommu-debug.c | 32 ++++++++++++++++++++++++++++++++
 drivers/iommu/intel-svm.c         |  8 --------
 include/linux/intel-svm.h         |  8 ++++++++
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
index 0951d58..66a99f5 100644
--- a/drivers/iommu/intel-iommu-debug.c
+++ b/drivers/iommu/intel-iommu-debug.c
@@ -47,6 +47,31 @@ static const struct file_operations __name ## _fops =			\
 }
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
+static void pasid_tbl_entry_show(struct seq_file *m, void *unused,
+				 struct intel_iommu *iommu)
+{
+	int pasid_size = 0, i;
+
+	if (ecap_pasid(iommu->ecap)) {
+		pasid_size = intel_iommu_get_pts(iommu);
+		seq_printf(m, "Pasid Table Addr : %p\n", iommu->pasid_table);
+
+		if (iommu->pasid_table) {
+			seq_printf(m, "Pasid table entries for domain %d:\n",
+				   iommu->segment);
+			seq_printf(m, "[Entry]\t\tContents\n");
+
+			/* Publish the pasid table entries here */
+			for (i = 0; i < pasid_size; i++) {
+				if (!iommu->pasid_table[i].val)
+					continue;
+				seq_printf(m, "[%d]\t\t%04llx\n", i,
+					   iommu->pasid_table[i].val);
+			}
+		}
+	}
+}
+
 static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
 				   struct intel_iommu *iommu, int bus, int ctx,
 				   struct context_entry *context, bool new_ext)
@@ -70,6 +95,12 @@ static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
 	}
 }
 #else /* CONFIG_INTEL_IOMMU_SVM */
+static void pasid_tbl_entry_show(struct seq_file *m, void *unused,
+				 struct intel_iommu *iommu)
+{
+	return;
+}
+
 static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
 				   struct intel_iommu *iommu, int bus, int ctx,
 				   struct context_entry *context, bool new_ext)
@@ -106,6 +137,7 @@ static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
 					       context, new_ext);
 		}
 	}
+	pasid_tbl_entry_show(m, unused, iommu);
 out:
 	spin_unlock_irqrestore(&iommu->lock, flags);
 }
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index f6697e5..2003f23 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -28,14 +28,6 @@
 
 static irqreturn_t prq_event_thread(int irq, void *d);
 
-struct pasid_entry {
-	u64 val;
-};
-
-struct pasid_state_entry {
-	u64 val;
-};
-
 int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
 {
 	struct page *pages;
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 733eaf9..a8abad6 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -18,6 +18,14 @@
 
 struct device;
 
+struct pasid_entry {
+	u64 val;
+};
+
+struct pasid_state_entry {
+	u64 val;
+};
+
 struct svm_dev_ops {
 	void (*fault_cb)(struct device *dev, int pasid, u64 address,
 			 u32 private, int rwxp, int response);
-- 
2.7.4

[PATCH v3 6/6] iommu/vt-d: Add debugfs support for Intel IOMMU Interrupt remapping

[Sohil Mehta]

Debugfs extension for Intel IOMMU to dump Interrupt remapping table
entries for Interrupt remapping and Interrupt posting.

The file /sys/kernel/debug/intel_iommu/ir_translation_struct provides
detailed information, such as Index, Source Id, Destination Id, Vector
and the raw values for entries with the present bit set, in the format
shown.

Remapped Interrupt supported on IOMMU: dmar5
 IR table address:ffff93e09d54c310
-----------------------------------------------------------
 Index  SID  Dest_ID  Vct Raw_value_high   Raw_value_low
 1      3a00 00000600 2c  0000000000043a00 00000600002c0009
 111    4301 00000900 a2  0000000000044301 0000090000a20009

Posted Interrupt supported on IOMMU: dmar5
 IR table address:ffff93e09d54c310
--------------------------------------------------------------------
 Index  SID  PDA_high PDA_low  Vct Raw_value_high   Raw_value_low
 4      4300 00000010 40c7c880 41  0000001000044300 40c7c88000418001
 5      4300 00000010 40c7c880 51  0000001000044300 40c7c88000518001

Cc: Gayatri Kammela <gayatri.kammela@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---

v3: Use a macro for seq file operations 
    Change the intel_iommu_interrupt_remap file name to ir_translation_struct

v2: Handle the case when IR is not enabled. Fix seq_printf formatting

 drivers/iommu/intel-iommu-debug.c | 112 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
index 66a99f5..8803277 100644
--- a/drivers/iommu/intel-iommu-debug.c
+++ b/drivers/iommu/intel-iommu-debug.c
@@ -12,6 +12,7 @@
  *
  * Authors: Gayatri Kammela <gayatri.kammela@intel.com>
  *          Jacob Pan <jacob.jun.pan@linux.intel.com>
+ *          Sohil Mehta <sohil.mehta@intel.com>
  *
  */
 
@@ -273,6 +274,108 @@ static int iommu_regset_show(struct seq_file *m, void *unused)
 
 DEFINE_SHOW_ATTRIBUTE(iommu_regset);
 
+#ifdef CONFIG_IRQ_REMAP
+static void ir_tbl_remap_entry_show(struct seq_file *m, void *unused,
+				    struct intel_iommu *iommu)
+{
+	int idx;
+	struct irte *ri_entry;
+
+	/* Print the header only once */
+	seq_printf(m, " Index  SID  Dest_ID  Vct"
+		   " Raw_value_high   Raw_value_low\n");
+
+	for (idx = 0; idx < INTR_REMAP_TABLE_ENTRIES; idx++) {
+		ri_entry = &iommu->ir_table->base[idx];
+		if (!ri_entry->present || ri_entry->p_pst)
+			continue;
+		seq_printf(m,
+			   " %d\t%04x %08x %02x  %016llx %016llx\n",
+			   idx, ri_entry->sid,
+			   ri_entry->dest_id, ri_entry->vector,
+			   ri_entry->high, ri_entry->low);
+	}
+}
+
+static void ir_tbl_posted_entry_show(struct seq_file *m, void *unused,
+				     struct intel_iommu *iommu)
+{
+	int idx;
+	struct irte *pi_entry;
+
+	/* Print the header only once */
+	seq_printf(m, " Index  SID  PDA_high PDA_low  Vct"
+		   " Raw_value_high   Raw_value_low\n");
+
+	for (idx = 0; idx < INTR_REMAP_TABLE_ENTRIES; idx++) {
+		pi_entry = &iommu->ir_table->base[idx];
+		if (!pi_entry->present || !pi_entry->p_pst)
+			continue;
+		seq_printf(m,
+			   " %d\t%04x %08x %08x %02x  %016llx %016llx\n",
+			   idx, pi_entry->sid,
+			   pi_entry->pda_h, (pi_entry->pda_l)<<6,
+			   pi_entry->vector, pi_entry->high,
+			   pi_entry->low);
+	}
+}
+
+/*
+ * For active IOMMUs go through the Interrupt remapping
+ * table and print valid entries in a table format for
+ * Remapped and Posted Interrupts.
+ */
+static int ir_translation_struct_show(struct seq_file *m, void *unused)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!iommu || !ecap_ir_support(iommu->ecap))
+			continue;
+
+		seq_printf(m,
+			   "\nRemapped Interrupt supported on IOMMU: %s\n"
+			   " IR table address:%p\n",
+			   iommu->name, iommu->ir_table);
+
+		seq_printf(m, "---------------------------------------"
+			   "--------------------\n");
+
+		if (iommu->ir_table)
+			ir_tbl_remap_entry_show(m, unused, iommu);
+		else
+			seq_printf(m, "Interrupt Remapping is not enabled\n");
+	}
+
+	seq_printf(m, "\n****\t****\t****\t****\t****\t****\t****\t****\n");
+
+	for_each_active_iommu(iommu, drhd) {
+		if (!iommu || !cap_pi_support(iommu->cap))
+			continue;
+
+		seq_printf(m,
+			   "\nPosted Interrupt supported on IOMMU: %s\n"
+			   " IR table address:%p\n",
+			   iommu->name, iommu->ir_table);
+
+		seq_printf(m, "---------------------------------------"
+			   "-----------------------------\n");
+
+		if (iommu->ir_table)
+			ir_tbl_posted_entry_show(m, unused, iommu);
+		else
+			seq_printf(m, "Interrupt Remapping is not enabled\n");
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(ir_translation_struct);
+#endif
+
 void __init intel_iommu_debugfs_init(void)
 {
 	struct dentry *iommu_debug_root;
@@ -297,6 +400,15 @@ void __init intel_iommu_debugfs_init(void)
 		goto err;
 	}
 
+#ifdef CONFIG_IRQ_REMAP
+	if (!debugfs_create_file("ir_translation_struct", S_IRUGO,
+				 iommu_debug_root, NULL,
+				 &ir_translation_struct_fops)) {
+		pr_err("Can't create ir_translation_struct file\n");
+		goto err;
+	}
+#endif
+
 	return;
 
 err:
-- 
2.7.4

Re: [PATCH v3 2/6] iommu/vt-d: Add Intel IOMMU debugfs to show context internals

[Lu Baolu]

Hi,

On 12/06/2017 11:43 AM, Sohil Mehta wrote:
> From: Gayatri Kammela <gayatri.kammela@intel.com>
>
> IOMMU internals states such as root and context can be exported to the
> userspace.
>
> Example of such dump in Kabylake:
>
> root@OTC-KBLH-01:~# cat
> /sys/kernel/debug/intel_iommu/dmar_translation_struct
>
> IOMMU dmar2:    Root Table Addr:4558a3000
>  Root tbl entries:
> Bus 0 L: 4558aa001 H: 0
>  Context table entries for Bus: 0
> [entry] DID :B :D .F    Low             High
> [160]   0000:00:14.00   4558a9001       102
> [184]   0000:00:17.00   400eac001       302
> [248]   0000:00:1f.00   4558af001       202
> [251]   0000:00:1f.03   40070e001       502
> [254]   0000:00:1f.06   4467c9001       602
>  Root tbl entries:
> Bus 1 L: 3fc8c2001 H: 0
>  Context table entries for Bus: 1
> [entry] DID :B :D .F    Low             High
> [0]     0000:01:00.00   3fc8c3001       402
>
> Cc: Sohil Mehta <sohil.mehta@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
> ---
>
> v3: Add a macro for seq file operations 
>     Change the intel_iommu_ctx file name to dmar_translation_struct
>
> v2: No change
>
>  drivers/iommu/Makefile            |   1 +
>  drivers/iommu/intel-iommu-debug.c | 152 ++++++++++++++++++++++++++++++++++++++
>  drivers/iommu/intel-iommu.c       |   4 +
>  3 files changed, 157 insertions(+)
>  create mode 100644 drivers/iommu/intel-iommu-debug.c
>
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 1fb6958..fdbaf46 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
>  obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
>  obj-$(CONFIG_DMAR_TABLE) += dmar.o
>  obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o
> +obj-$(CONFIG_INTEL_IOMMU_DEBUG) += intel-iommu-debug.o
>  obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
>  obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
>  obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o
> diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
> new file mode 100644
> index 0000000..8ae0c4d
> --- /dev/null
> +++ b/drivers/iommu/intel-iommu-debug.c
> @@ -0,0 +1,152 @@
> +/*
> + * Copyright © 2017 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * Authors: Gayatri Kammela <gayatri.kammela@intel.com>
> + *          Jacob Pan <jacob.jun.pan@linux.intel.com>
> + *
> + */
> +
> +#define pr_fmt(fmt)     "INTEL_IOMMU: " fmt
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/debugfs.h>
> +#include <linux/pci.h>
> +#include <linux/iommu.h>
> +#include <linux/intel-iommu.h>
> +#include <linux/intel-svm.h>
> +#include <linux/dmar.h>
> +#include <linux/spinlock.h>
> +
> +#include "irq_remapping.h"
> +
> +#define TOTAL_BUS_NR (256) /* full bus range 256 */
> +#define DEFINE_SHOW_ATTRIBUTE(__name)					\
> +static int __name ## _open(struct inode *inode, struct file *file)	\
> +{									\
> +	return single_open(file, __name ## _show, inode->i_private);	\
> +}									\
> +static const struct file_operations __name ## _fops =			\
> +{									\
> +	.open		= __name ## _open,				\
> +	.read		= seq_read,					\
> +	.llseek		= seq_lseek,					\
> +	.release	= single_release,				\
> +	.owner		= THIS_MODULE,					\
> +}
> +
> +static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
> +			       struct intel_iommu *iommu, int bus, bool ext,
> +			       bool new_ext)
> +{
> +	struct context_entry *context;
> +	int ctx;
> +	unsigned long flags;
> +
> +	seq_printf(m, "%s Context table entries for Bus: %d\n",
> +		   ext ? "Lower" : "", bus);
> +	seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n");

WARNING: Prefer seq_puts to seq_printf
#119: FILE: drivers/iommu/intel-iommu-debug.c:59:
+    seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n");

(caught by checkpatch.pl)

> +
> +	spin_lock_irqsave(&iommu->lock, flags);
> +
> +	/* Publish either context entries or extended contenxt entries */
> +	for (ctx = 0; ctx < (ext ? 128 : 256); ctx++) {
> +		context = iommu_context_addr(iommu, bus, ctx, 0);
> +		if (!context)
> +			goto out;
> +		if (context_present(context)) {
> +			seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n",
> +				   ctx, iommu->segment, bus, PCI_SLOT(ctx),
> +				   PCI_FUNC(ctx), context[0].lo, context[0].hi);
> +		}
> +	}
> +out:
> +	spin_unlock_irqrestore(&iommu->lock, flags);
> +}
> +
> +static void root_tbl_entry_show(struct seq_file *m, void *unused,

Why do you define the "unused" parameter which will never been used?
The same questions to other show functions.

> +				struct intel_iommu *iommu, u64 rtaddr_reg,
> +				bool ext, bool new_ext)
> +{
> +	int bus;
> +
> +	seq_printf(m, "\nIOMMU %s: %2s Root Table Addr:%llx\n", iommu->name,
> +		   ext ? "Extended" : "", rtaddr_reg);
> +	/* Publish extended root table entries or root table entries here */
> +	for (bus = 0; bus < TOTAL_BUS_NR; bus++) {
> +		if (!iommu->root_entry[bus].lo)
> +			continue;
> +
> +		seq_printf(m, "%s Root tbl entries:\n", ext ? "Extended" : "");
> +		seq_printf(m, "Bus %d L: %llx H: %llx\n", bus,
> +			   iommu->root_entry[bus].lo, iommu->root_entry[bus].hi
> +			  );
> +
> +		ctx_tbl_entry_show(m, unused, iommu, bus, ext, new_ext);
> +	}
> +}
> +
> +static int dmar_translation_struct_show(struct seq_file *m, void *unused)
> +{
> +	struct dmar_drhd_unit *drhd;
> +	struct intel_iommu *iommu;
> +	u64 rtaddr_reg;
> +	bool new_ext, ext;
> +
> +	rcu_read_lock();
> +	for_each_active_iommu(iommu, drhd) {
> +		if (iommu) {
> +			/* Check if root table type is set */
> +			rtaddr_reg = dmar_readq(iommu->reg + DMAR_RTADDR_REG);
> +			ext        = !!(rtaddr_reg & DMA_RTADDR_RTT);
> +			new_ext    = !!ecap_ecs(iommu->ecap);
> +			if (new_ext != ext) {
> +				seq_printf(m, "IOMMU %s: invalid ecs\n",
> +					   iommu->name);
> +				rcu_read_unlock();
> +				return -EINVAL;
> +			}
> +			root_tbl_entry_show(m, unused, iommu, rtaddr_reg, ext,
> +					    new_ext);
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(dmar_translation_struct);
> +
> +void __init intel_iommu_debugfs_init(void)
> +{
> +	struct dentry *iommu_debug_root;
> +
> +	iommu_debug_root = debugfs_create_dir("intel_iommu", NULL);
> +
> +	if (!iommu_debug_root) {
> +		pr_err("can't create debugfs dir\n");

I don't think we need a pr_err() here. System works well even
debugfs_create_dir() returns NULL.

This is same to all pr_err() in this file.

> +		goto err;
> +	}
> +
> +	if (!debugfs_create_file("dmar_translation_struct", S_IRUGO,
> +				 iommu_debug_root, NULL,
> +				 &dmar_translation_struct_fops)) {

WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#202: FILE: drivers/iommu/intel-iommu-debug.c:142:
+    if (!debugfs_create_file("dmar_translation_struct", S_IRUGO,

(caught by checkpatch.pl)

> +		pr_err("Can't create dmar_translation_struct file\n");
> +		goto err;
> +	}
> +
> +err:
> +	debugfs_remove_recursive(iommu_debug_root);
> +

Blank line isn't necessary here.

> +}
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 419a559..7794e0c 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4797,6 +4797,10 @@ int __init intel_iommu_init(void)
>  			  intel_iommu_cpu_dead);
>  	intel_iommu_enabled = 1;
>  
> +#ifdef CONFIG_INTEL_IOMMU_DEBUG
> +	intel_iommu_debugfs_init();
> +#endif
> +
>  	return 0;
>  
>  out_free_reserved_range:

Re: [PATCH v3 3/6] iommu/vt-d: Add Intel IOMMU debugfs to show extended context internals

[Lu Baolu]

Hi,

On 12/06/2017 11:43 AM, Sohil Mehta wrote:
> From: Gayatri Kammela <gayatri.kammela@intel.com>
>
> Debugfs extension to dump internals such as extended context table
> entries for each IOMMU to the userspace.
>
> root@OTC-KBLH-01:~# cat
> /sys/kernel/debug/intel_iommu/dmar_translation_struct
>
> IOMMU dmar1: Extended Root Table Addr:4558a1800
> Extended Root tbl entries:
> Bus 0 L: 4558a6001 H: 0
> Lower Context table entries for Bus: 0
> [entry] DID :B :D .F    Low             High
> [16]    0000:00:02.00   4558a5005       102
> Higher Context tbl entries for Bus: 0
> [16]    0000:00:02.00   401b0000c       401400000
>
> IOMMU dmar0: Extended Root Table Addr:4558a2800
> Extended Root tbl entries:
> Bus 0 L: 4016f4001 H: 0
> Lower Context table entries for Bus: 0
> [entry] DID :B :D .F    Low             High
> [80]    0000:00:0a.00   4016f3a05       102
> Higher Context tbl entries for Bus: 0
> [80]    0000:00:0a.00   40150000c       671b80000000
>
> Cc: Sohil Mehta <sohil.mehta@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
> ---
>
> v3: No change
>
> v2: No change
>
>  drivers/iommu/intel-iommu-debug.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
> index 8ae0c4d..8e7f5d2 100644
> --- a/drivers/iommu/intel-iommu-debug.c
> +++ b/drivers/iommu/intel-iommu-debug.c
> @@ -46,6 +46,38 @@ static const struct file_operations __name ## _fops =			\
>  	.owner		= THIS_MODULE,					\
>  }
>  
> +#ifdef CONFIG_INTEL_IOMMU_SVM
> +static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
> +				   struct intel_iommu *iommu, int bus, int ctx,
> +				   struct context_entry *context, bool new_ext)
> +{
> +	u64 ctx_lo;
> +
> +	if (new_ext) {
> +		seq_printf(m, "Higher Context tbl entries for Bus: %d\n", bus);
> +		ctx_lo = context[0].lo;
> +
> +		if (!(ctx_lo & CONTEXT_PASIDE)) {
> +			context[1].hi = (u64)virt_to_phys(
> +					iommu->pasid_state_table);
> +			context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
> +					intel_iommu_get_pts(iommu);

Why do you change the context entries here?

> +		}
> +
> +		seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n", ctx,
> +			   iommu->segment, bus, PCI_SLOT(ctx), PCI_FUNC(ctx),
> +			   context[1].lo, context[1].hi);
> +	}
> +}
> +#else /* CONFIG_INTEL_IOMMU_SVM */
> +static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
> +				   struct intel_iommu *iommu, int bus, int ctx,
> +				   struct context_entry *context, bool new_ext)
> +{
> +	return;
> +}
> +#endif /* CONFIG_INTEL_IOMMU_SVM */
> +
>  static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
>  			       struct intel_iommu *iommu, int bus, bool ext,
>  			       bool new_ext)
> @@ -69,6 +101,9 @@ static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
>  			seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n",
>  				   ctx, iommu->segment, bus, PCI_SLOT(ctx),
>  				   PCI_FUNC(ctx), context[0].lo, context[0].hi);
> +
> +			ext_ctx_tbl_entry_show(m, unused, iommu, bus, ctx,
> +					       context, new_ext);

How about

+			if (new_ext)
+				ext_ctx_tbl_entry_show(m, unused, iommu, bus, ctx, context);


and remove checking new_ext in ext_ctx_tbl_entry_show()?
 
Best regards,
Lu Baolu

Re: [PATCH v3 3/6] iommu/vt-d: Add Intel IOMMU debugfs to show extended context internals

[Lu Baolu]

Hi,

On 12/06/2017 11:43 AM, Sohil Mehta wrote:
> From: Gayatri Kammela <gayatri.kammela@intel.com>
>
> Debugfs extension to dump internals such as extended context table
> entries for each IOMMU to the userspace.
>
> root@OTC-KBLH-01:~# cat
> /sys/kernel/debug/intel_iommu/dmar_translation_struct
>
> IOMMU dmar1: Extended Root Table Addr:4558a1800
> Extended Root tbl entries:
> Bus 0 L: 4558a6001 H: 0
> Lower Context table entries for Bus: 0
> [entry] DID :B :D .F    Low             High
> [16]    0000:00:02.00   4558a5005       102
> Higher Context tbl entries for Bus: 0
> [16]    0000:00:02.00   401b0000c       401400000
>
> IOMMU dmar0: Extended Root Table Addr:4558a2800
> Extended Root tbl entries:
> Bus 0 L: 4016f4001 H: 0
> Lower Context table entries for Bus: 0
> [entry] DID :B :D .F    Low             High
> [80]    0000:00:0a.00   4016f3a05       102
> Higher Context tbl entries for Bus: 0

Do you mind changing "tbl entries" to "table entries" so that
it could be consistent with above lower one?

Best regards,
Lu Baolu

> [80]    0000:00:0a.00   40150000c       671b80000000
>
> Cc: Sohil Mehta <sohil.mehta@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
> ---
>
> v3: No change
>
> v2: No change
>
>  drivers/iommu/intel-iommu-debug.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/iommu/intel-iommu-debug.c b/drivers/iommu/intel-iommu-debug.c
> index 8ae0c4d..8e7f5d2 100644
> --- a/drivers/iommu/intel-iommu-debug.c
> +++ b/drivers/iommu/intel-iommu-debug.c
> @@ -46,6 +46,38 @@ static const struct file_operations __name ## _fops =			\
>  	.owner		= THIS_MODULE,					\
>  }
>  
> +#ifdef CONFIG_INTEL_IOMMU_SVM
> +static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
> +				   struct intel_iommu *iommu, int bus, int ctx,
> +				   struct context_entry *context, bool new_ext)
> +{
> +	u64 ctx_lo;
> +
> +	if (new_ext) {
> +		seq_printf(m, "Higher Context tbl entries for Bus: %d\n", bus);
> +		ctx_lo = context[0].lo;
> +
> +		if (!(ctx_lo & CONTEXT_PASIDE)) {
> +			context[1].hi = (u64)virt_to_phys(
> +					iommu->pasid_state_table);
> +			context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
> +					intel_iommu_get_pts(iommu);
> +		}
> +
> +		seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n", ctx,
> +			   iommu->segment, bus, PCI_SLOT(ctx), PCI_FUNC(ctx),
> +			   context[1].lo, context[1].hi);
> +	}
> +}
> +#else /* CONFIG_INTEL_IOMMU_SVM */
> +static void ext_ctx_tbl_entry_show(struct seq_file *m, void *unused,
> +				   struct intel_iommu *iommu, int bus, int ctx,
> +				   struct context_entry *context, bool new_ext)
> +{
> +	return;
> +}
> +#endif /* CONFIG_INTEL_IOMMU_SVM */
> +
>  static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
>  			       struct intel_iommu *iommu, int bus, bool ext,
>  			       bool new_ext)
> @@ -69,6 +101,9 @@ static void ctx_tbl_entry_show(struct seq_file *m, void *unused,
>  			seq_printf(m, "[%d]\t%04x:%02x:%02x.%02x\t%llx\t%llx\n",
>  				   ctx, iommu->segment, bus, PCI_SLOT(ctx),
>  				   PCI_FUNC(ctx), context[0].lo, context[0].hi);
> +
> +			ext_ctx_tbl_entry_show(m, unused, iommu, bus, ctx,
> +					       context, new_ext);
>  		}
>  	}
>  out:

Re: [PATCH v3 2/6] iommu/vt-d: Add Intel IOMMU debugfs to show context internals

[Mehta, Sohil]

On Wed, 2017-12-06 at 16:16 +0800, Lu Baolu wrote:
> Hi,
> 
> On 12/06/2017 11:43 AM, Sohil Mehta wrote:
> > 
> > From: Gayatri Kammela <gayatri.kammela@intel.com>
> > 
> > 
> > +	seq_printf(m, "%s Context table entries for Bus: %d\n",
> > +		   ext ? "Lower" : "", bus);
> > +	seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n");
> WARNING: Prefer seq_puts to seq_printf
> #119: FILE: drivers/iommu/intel-iommu-debug.c:59:
> +    seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n");
> 
> (caught by checkpatch.pl)
> 

Hi Lu,

We'll fix this and the other checkpatch.pl warnings.


> > +
> > +static void root_tbl_entry_show(struct seq_file *m, void *unused,
> Why do you define the "unused" parameter which will never been used?
> The same questions to other show functions.
> 

Some functions in our code that are registered with seq_file needed to
have an unused parameter since seq_file.h defines the show function as:
	int (*show) (struct seq_file *m, void *v);

But a lot of other functions including the one you pointed don't need
to have the unused parameter. We'll remove it from those.


> > +void __init intel_iommu_debugfs_init(void)
> > +{
> > +	struct dentry *iommu_debug_root;
> > +
> > +	iommu_debug_root = debugfs_create_dir("intel_iommu",
> > NULL);
> > +
> > +	if (!iommu_debug_root) {
> > +		pr_err("can't create debugfs dir\n");
> I don't think we need a pr_err() here. System works well even
> debugfs_create_dir() returns NULL.
> 
> This is same to all pr_err() in this file.
> 

Would the recommendation be to use pr_warn instead of pr_err or should
we entirely skip the message altogether?

Thanks,
Sohil

Re: [PATCH v3 3/6] iommu/vt-d: Add Intel IOMMU debugfs to show extended context internals

[Mehta, Sohil]

On Wed, 2017-12-06 at 16:17 +0800, Lu Baolu wrote:
> Hi,
> 
> On 12/06/2017 11:43 AM, Sohil Mehta wrote:
> > 
> > From: Gayatri Kammela <gayatri.kammela@intel.com>
> > 
> > +
> > +	if (new_ext) {
> > +		seq_printf(m, "Higher Context tbl entries for Bus:
> > %d\n", bus);
> > +		ctx_lo = context[0].lo;
> > +
> > +		if (!(ctx_lo & CONTEXT_PASIDE)) {
> > +			context[1].hi = (u64)virt_to_phys(
> > +					iommu->pasid_state_table);
> > +			context[1].lo = (u64)virt_to_phys(iommu-
> > >pasid_table) |
> > +					intel_iommu_get_pts(iommu)
> > ;
> Why do you change the context entries here?
> 

Thanks for catching this. The context entires were mistakenly getting
changed. We'll remove it.

Re: [PATCH v3 2/6] iommu/vt-d: Add Intel IOMMU debugfs to show context internals

[Lu Baolu]

Hi,

Sorry for late reply.

On 12/08/2017 04:19 AM, Mehta, Sohil wrote:
> On Wed, 2017-12-06 at 16:16 +0800, Lu Baolu wrote:
>> Hi,
>>
>> On 12/06/2017 11:43 AM, Sohil Mehta wrote:
>>> From: Gayatri Kammela <gayatri.kammela@intel.com>
>>>
>>>  
>>> +	seq_printf(m, "%s Context table entries for Bus: %d\n",
>>> +		   ext ? "Lower" : "", bus);
>>> +	seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n");
>> WARNING: Prefer seq_puts to seq_printf
>> #119: FILE: drivers/iommu/intel-iommu-debug.c:59:
>> +    seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n");
>>
>> (caught by checkpatch.pl)
>>
> Hi Lu,
>
> We'll fix this and the other checkpatch.pl warnings.
>
>
>>> +
>>> +static void root_tbl_entry_show(struct seq_file *m, void *unused,
>> Why do you define the "unused" parameter which will never been used?
>> The same questions to other show functions.
>>
> Some functions in our code that are registered with seq_file needed to
> have an unused parameter since seq_file.h defines the show function as:
> 	int (*show) (struct seq_file *m, void *v);
>
> But a lot of other functions including the one you pointed don't need
> to have the unused parameter. We'll remove it from those.
>
>
>>> +void __init intel_iommu_debugfs_init(void)
>>> +{
>>> +	struct dentry *iommu_debug_root;
>>> +
>>> +	iommu_debug_root = debugfs_create_dir("intel_iommu",
>>> NULL);
>>> +
>>> +	if (!iommu_debug_root) {
>>> +		pr_err("can't create debugfs dir\n");
>> I don't think we need a pr_err() here. System works well even
>> debugfs_create_dir() returns NULL.
>>
>> This is same to all pr_err() in this file.
>>
> Would the recommendation be to use pr_warn instead of pr_err or should
> we entirely skip the message altogether?

Greg ever educated me about the use of debugfs_ functions in
this thread.

https://spinics.net/lists/linux-usb/msg159384.html

At least we should avoid the warning/error messages here.

Best regards,
Lu Baolu

Re: [PATCH v3 2/6] iommu/vt-d: Add Intel IOMMU debugfs to show context internals

[Mehta, Sohil]

On Wed, 2017-12-13 at 10:28 +0800, Lu Baolu wrote:
> 
> > Would the recommendation be to use pr_warn instead of pr_err or
> > should
> > we entirely skip the message altogether?
> Greg ever educated me about the use of debugfs_ functions in
> this thread.
> 
> https://spinics.net/lists/linux-usb/msg159384.html
> 
> At least we should avoid the warning/error messages here.
> 

Thanks. We'll remove the error messages.