Re: [PATCH 1/5] iommu/amd - Add debugfs support

2018-01-26 Thread Borislav Petkov
On Fri, Jan 26, 2018 at 05:52:15PM -0600, Gary R Hook wrote:
> Create the basic debugfs functions. Expose a count of IOMMU device
> table entries that appear to be in use.
> 
> Signed-off-by: Gary R Hook 
> ---
>  drivers/iommu/Kconfig |9 +++
>  drivers/iommu/Makefile|2 -
>  drivers/iommu/amd_iommu_debugfs.c |  112 
> +
>  drivers/iommu/amd_iommu_init.c|7 ++
>  drivers/iommu/amd_iommu_proto.h   |6 ++
>  drivers/iommu/amd_iommu_types.h   |3 +
>  6 files changed, 136 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/iommu/amd_iommu_debugfs.c
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index f3a21343e636..7753989b6be7 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -135,6 +135,15 @@ config AMD_IOMMU_V2
> hardware. Select this option if you want to use devices that support
> the PCI PRI and PASID interface.
>  
> +config AMD_IOMMU_DEBUG
> + bool "Expose AMD IOMMU internals in DebugFS"
> + depends on AMD_IOMMU && DEBUG_FS
> + default n
> + help
> +   With this option you can enable access to AMD IOMMU registers and
> +   data structures through debugfs. Select this to see information
> +   about the internal state of the device.
> +
>  # Intel IOMMU support
>  config DMAR_TABLE
>   bool
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 1fb695854809..d9e9ed5f6cfc 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
>  obj-$(CONFIG_IOMMU_IOVA) += iova.o
>  obj-$(CONFIG_OF_IOMMU)   += of_iommu.o
>  obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
> -obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
> +obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_debugfs.o

That looks like it needs to be:

obj-$(CONFIG_AMD_IOMMU_DEBUG) += amd_iommu_debugfs.o

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 5/5] iommu/amd - Add a debugfs entry to specify a IOMMU device table entry

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

Signed-off-by: Gary R Hook 
---
 drivers/iommu/amd_iommu_debugfs.c |  127 -
 1 file changed, 109 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/amd_iommu_debugfs.c 
b/drivers/iommu/amd_iommu_debugfs.c
index 87840ae9889d..efb666873daa 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -38,6 +38,7 @@ static DEFINE_RWLOCK(iommu_debugfs_lock);
 #defineMAX_NAME_LEN20
 
 static unsigned int amd_iommu_verbose;
+static unsigned int amd_iommu_devid = ~0;
 
 static unsigned int amd_iommu_count_valid_dtes(int start, int end)
 {
@@ -91,6 +92,72 @@ static const struct file_operations 
amd_iommu_debugfs_dtecount_ops = {
.write = NULL,
 };
 
+static ssize_t amd_iommu_debugfs_devid_read(struct file *filp,
+   char __user *ubuf,
+   size_t count, loff_t *offp)
+{
+   struct amd_iommu *iommu = filp->private_data;
+   unsigned int obuflen = 512;
+   unsigned int oboff = 0;
+   ssize_t ret;
+   char *obuf;
+
+   if (!iommu)
+   return 0;
+
+   obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+   if (!obuf)
+   return -ENOMEM;
+
+   if (amd_iommu_verbose)
+   oboff += OSCNPRINTF("%02x:%02x.%x 0x%04x %u\n",
+   PCI_BUS_NUM(amd_iommu_devid),
+   PCI_SLOT(amd_iommu_devid),
+   PCI_FUNC(amd_iommu_devid),
+   amd_iommu_devid, amd_iommu_devid);
+   else
+   oboff += OSCNPRINTF("%u\n", amd_iommu_devid);
+
+   ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+   kfree(obuf);
+
+   return ret;
+}
+
+static ssize_t amd_iommu_debugfs_devid_write(struct file *filp,
+   const char __user *ubuf,
+   size_t count, loff_t *offp)
+{
+   unsigned int pci_id, pci_slot, pci_func;
+   unsigned int obuflen = 80;
+   ssize_t ret;
+   char *obuf;
+   int n;
+
+   obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+   if (!obuf)
+   return -ENOMEM;
+
+   ret = simple_write_to_buffer(obuf, OBUFLEN, offp, ubuf, count);
+
+   if (strnchr(obuf, OBUFLEN, ':')) {
+   n = sscanf(obuf, "%x:%x.%x", &pci_id, &pci_slot, &pci_func);
+   if (n == 3)
+   amd_iommu_devid = PCI_DEVID(pci_id, PCI_DEVFN(pci_slot, 
pci_func));
+   } else
+   n = kstrtouint(obuf, 0, &amd_iommu_devid);
+   kfree(obuf);
+
+   return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_devid_ops = {
+   .owner = THIS_MODULE,
+   .open = simple_open,
+   .read = amd_iommu_debugfs_devid_read,
+   .write = amd_iommu_debugfs_devid_write,
+};
+
 struct bits {
uint bit;
uint len;
@@ -211,9 +278,13 @@ static ssize_t amd_iommu_debugfs_dte_read(struct file 
*filp,
return 0;
 
/* Count the number of valid entries in the device table */
-   istart = 0;
-   iend = MAX_PCI_ID;
-   n = amd_iommu_count_valid_dtes(istart, iend);
+   if (amd_iommu_devid > MAX_PCI_ID) {
+   istart = 0;
+   iend = MAX_PCI_ID;
+   n = amd_iommu_count_valid_dtes(istart, iend);
+   } else {
+   n = 1;
+   }
if (amd_iommu_verbose)
obuflen = n * 2048;
else
@@ -223,20 +294,29 @@ static ssize_t amd_iommu_debugfs_dte_read(struct file 
*filp,
if (!obuf)
return -ENOMEM;
 
-   for (i = istart ; i <= iend ; i++)
-   if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
-|| amd_iommu_dev_table[i].data[1]) {
-   if (amd_iommu_verbose) {
-   oboff += OSCNPRINTF("Device %02x:%02x.%x\n",
-   PCI_BUS_NUM(i),
-   PCI_SLOT(i),
-   PCI_FUNC(i));
-   amd_iommu_print_dte_verbose(i, &obuf,
-   &obuflen, &oboff);
-   } else {
-   oboff += PRINTDTE(i);
+   if (amd_iommu_devid > MAX_PCI_ID) {
+   for (i = istart ; i <= iend ; i++)
+   if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+|| amd_iommu_dev_table[i].data[1]) {
+   if (amd_iommu_verbo

[PATCH 4/5] iommu/amd - Expose the active IOMMU device table entries

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

Use arrays to store DTE bit field definitions for debugfs printing
in verbose mode

Signed-off-by: Gary R Hook 
---
 drivers/iommu/amd_iommu_debugfs.c |  182 +
 1 file changed, 182 insertions(+)

diff --git a/drivers/iommu/amd_iommu_debugfs.c 
b/drivers/iommu/amd_iommu_debugfs.c
index 5066d3976912..87840ae9889d 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -22,6 +22,16 @@
 #defineOSCNPRINTF(fmt, ...) \
scnprintf(OBUFP, OBUFSPC, fmt, ## __VA_ARGS__)
 
+#defineMAX_PCI_ID  0x
+
+#definePRINTDTE(i) OSCNPRINTF("%02x:%02x:%x - " \
+  "%016llx %016llx %016llx %016llx\n", \
+  PCI_BUS_NUM(i), PCI_SLOT(i), PCI_FUNC(i), \
+  amd_iommu_dev_table[i].data[0], \
+  amd_iommu_dev_table[i].data[1], \
+  amd_iommu_dev_table[i].data[2], \
+  amd_iommu_dev_table[i].data[3])
+
 static struct dentry *iommu_debugfs_dir;
 static DEFINE_RWLOCK(iommu_debugfs_lock);
 
@@ -81,9 +91,174 @@ static const struct file_operations 
amd_iommu_debugfs_dtecount_ops = {
.write = NULL,
 };
 
+struct bits {
+   uint bit;
+   uint len;
+   char *lbl;
+   char *note;
+};
+
+static struct bits dte_lft[] = {
+   {  0, 1, "V:", "[0]" },
+   {  1, 1, "TV:", "[1]" },
+   {  7, 2, "Host Access Dirty:", "[8:7]" },
+   {  9, 3, "Paging Mode:", "[11:9]" },
+   { 52, 1, "PPR:", "[52]" },
+   { 53, 1, "GPRP:", "[53]" },
+   { 54, 1, "GIoV:", "[54]" },
+   { 55, 1, "GV:", "[55]" },
+   { 56, 2, "GLX:", "[57:56]" },
+   { 61, 1, "IR:", "[61]" },
+   { 62, 1, "IW:", "[62]" },
+   { 96, 1, "I:", "[96]" },
+   { 97, 1, "SE:", "[97]" },
+   { 98, 1, "SA:", "[98]" },
+   { 99, 2, "IOCtl:", "[100:99]" },
+};
+static uint lftlen = sizeof(dte_lft) / sizeof(struct bits);
+
+static struct bits dte_rght[] = {
+   { 101, 1, "Cache:", "[101]" },
+   { 102, 1, "SD:", "[102]" },
+   { 103, 1, "EX  :", "[103]" },
+   { 104, 2, "SysMgt:", "[105:104]" },
+   { 128, 1, "IV:", "[128]" },
+   { 129, 4, "IntTabLen:", "[132:129]" },
+   { 133, 1, "IG:", "[133]" },
+   { 184, 1, "InitPass:", "[184]" },
+   { 185, 1, "EIntPass:", "[185]" },
+   { 186, 1, "NMIPass:", "[186]" },
+   { 188, 2, "IntClt:", "[189:188]" },
+   { 190, 1, "Lint0Pass:", "[190]" },
+   { 191, 1, "Lint1Pass:", "[191]" },
+   { 246, 1, "AttrV:", "[246]" },
+   { 247, 1, "Mode0FC:", "[247]" },
+};
+static uint rghtlen = sizeof(dte_rght) / sizeof(struct bits);
+
+#defineDTE_BIT(ds, p, n)   ((ds[p/64] >> (p%64)) & ((1ULL< rghtlen ? lftlen : rghtlen;
+
+   for (j = 0; j < max ; j++) {
+   lft = &dte_lft[j];
+   if (j < lftlen)
+   oboff += OSCNPRINTF("%-19s%3llx %*s", lft->lbl,
+   DTE_BIT(dte, lft->bit, lft->len),
+   (j < rghtlen) ? -16 : 1,
+   lft->note);
+   else
+   oboff += OSCNPRINTF("%38s", "");
+
+   rght = &dte_rght[j];
+   if (j < rghtlen)
+   oboff += OSCNPRINTF("%-19s%3llx %s\n", rght->lbl,
+   DTE_BIT(dte, rght->bit, rght->len),
+   rght->note);
+   else
+   oboff += OSCNPRINTF("\n");
+   }
+
+   oboff += OSCNPRINTF("%-28s: %4llx %s\n", "Domain ID",
+   DTE_BIT(dte, 64, 16), "[79:64]");
+   oboff += OSCNPRINTF("%-28s: %4llx %s\n", "Snoop Attribute",
+   DTE_BIT(dte, 248, 8), "[255:248]");
+   oboff += OSCNPRINTF("%-28s: %12llx %s\n", "Page Table Root Pointer",
+   DTE_BIT(dte, 12, 40) << 12, "[51:12]");
+   oboff += OSCNPRINTF("%-28s: %12llx %s\n",
+   "Interrupt Table Root Pointer",
+   DTE_BIT(dte, 134, 46) << 6, "[179:134]");
+
+   part1 = DTE_BIT(dte, 107, 21) << 19;
+   part2 = DTE_BIT(dte, 80, 16) << 3;
+   part3 = DTE_BIT(dte, 58, 3);
+   oboff += OSCNPRINTF("%-28s: %12llx %s\n\n",
+   "Guest CR3 Table Root Pointer",
+   (part1 | part2 | part3) << 12,
+   "[127:107][95:80][60:58]");
+
+   *buflen = obuflen;
+   *offset = oboff;
+   *buf = obuf;
+}
+
+static ssize_t am

[PATCH 1/5] iommu/amd - Add debugfs support

2018-01-26 Thread Gary R Hook
Create the basic debugfs functions. Expose a count of IOMMU device
table entries that appear to be in use.

Signed-off-by: Gary R Hook 
---
 drivers/iommu/Kconfig |9 +++
 drivers/iommu/Makefile|2 -
 drivers/iommu/amd_iommu_debugfs.c |  112 +
 drivers/iommu/amd_iommu_init.c|7 ++
 drivers/iommu/amd_iommu_proto.h   |6 ++
 drivers/iommu/amd_iommu_types.h   |3 +
 6 files changed, 136 insertions(+), 3 deletions(-)
 create mode 100644 drivers/iommu/amd_iommu_debugfs.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index f3a21343e636..7753989b6be7 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -135,6 +135,15 @@ config AMD_IOMMU_V2
  hardware. Select this option if you want to use devices that support
  the PCI PRI and PASID interface.
 
+config AMD_IOMMU_DEBUG
+   bool "Expose AMD IOMMU internals in DebugFS"
+   depends on AMD_IOMMU && DEBUG_FS
+   default n
+   help
+ With this option you can enable access to AMD IOMMU registers and
+ data structures through debugfs. Select this to see information
+ about the internal state of the device.
+
 # Intel IOMMU support
 config DMAR_TABLE
bool
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb695854809..d9e9ed5f6cfc 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU) += of_iommu.o
 obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
-obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
+obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_debugfs.o
 obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
 obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
diff --git a/drivers/iommu/amd_iommu_debugfs.c 
b/drivers/iommu/amd_iommu_debugfs.c
new file mode 100644
index ..18f70934961d
--- /dev/null
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD IOMMU driver
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ *
+ * Author: Gary R Hook 
+ */
+
+#ifdef CONFIG_DEBUG_FS
+
+#include 
+#include 
+#include 
+#include "amd_iommu_proto.h"
+#include "amd_iommu_types.h"
+
+/* DebugFS helpers */
+#defineOBUFP   (obuf + oboff)
+#defineOBUFLEN obuflen
+#defineOBUFSPC (OBUFLEN - oboff)
+#defineOSCNPRINTF(fmt, ...) \
+   scnprintf(OBUFP, OBUFSPC, fmt, ## __VA_ARGS__)
+
+static struct dentry *iommu_debugfs_dir;
+static DEFINE_RWLOCK(iommu_debugfs_lock);
+
+#defineMAX_NAME_LEN20
+
+static unsigned int amd_iommu_count_valid_dtes(int start, int end)
+{
+   unsigned int n = 0;
+   int i;
+
+   /* Scan the DTE table from entry 'start' through entry 'end' for
+* active entries
+*/
+   for (i = start ; i <= end ; i++)
+   if ((amd_iommu_dev_table[i].data[0] ^ 0x3)
+|| amd_iommu_dev_table[i].data[1])
+   n++;
+   return n;
+}
+
+static ssize_t amd_iommu_debugfs_dtecount_read(struct file *filp,
+  char __user *ubuf,
+  size_t count, loff_t *offp)
+{
+   struct amd_iommu *iommu = filp->private_data;
+   unsigned int obuflen = 512;
+   unsigned int oboff = 0;
+   unsigned int n;
+   ssize_t ret;
+   char *obuf;
+
+   if (!iommu)
+   return 0;
+
+   obuf = kmalloc(OBUFLEN, GFP_KERNEL);
+   if (!obuf)
+   return -ENOMEM;
+
+   n = amd_iommu_count_valid_dtes(0, 0x);
+   oboff += OSCNPRINTF("%d\n", n);
+
+   ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
+   kfree(obuf);
+
+   return ret;
+}
+
+static const struct file_operations amd_iommu_debugfs_dtecount_ops = {
+   .owner = THIS_MODULE,
+   .open = simple_open,
+   .read = amd_iommu_debugfs_dtecount_read,
+   .write = NULL,
+};
+
+void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
+{
+   char name[MAX_NAME_LEN + 1];
+   struct dentry *d_dte;
+   unsigned long flags;
+
+   if (!debugfs_initialized())
+   return;
+
+   write_lock_irqsave(&iommu_debugfs_lock, flags);
+   if (!iommu_debugfs_dir)
+   iommu_debugfs_dir = debugfs_create_dir("amd-iommu", NULL);
+   write_unlock_irqrestore(&iommu_debugfs_lock, flags);
+   if (!iommu_debugfs_dir)
+   goto err;
+
+   snprintf(name, MAX_NAME_LEN, "iommu%02d", iommu->index);
+   iommu->debugfs_instance = debugfs_create_dir(name, iommu_debugfs_dir);
+   if (!iommu->debugfs_instance)
+   goto err;
+
+   d_dte = debugfs_create_file("count", 0400,
+   iommu->debugfs_instance, iommu,
+

[PATCH 3/5] iommu/amd - Add a README variable for the IOMMU debugfs

2018-01-26 Thread Gary R Hook
Provide help text via a filesystem entry

Signed-off-by: Gary R Hook 
---
 drivers/iommu/amd_iommu_debugfs.c |   31 +++
 1 file changed, 31 insertions(+)

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

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 0/5] Add debugfs info for the AMD IOMMU

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

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

---

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


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

--
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 2/5] iommu/amd - Add a 'verbose' switch for IOMMU debugfs

2018-01-26 Thread Gary R Hook
Enable more descriptive debugfs output via a 'verbose' variable.

Signed-off-by: Gary R Hook 
---
 drivers/iommu/amd_iommu_debugfs.c |   14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu_debugfs.c 
b/drivers/iommu/amd_iommu_debugfs.c
index 18f70934961d..c449f3a7452c 100644
--- a/drivers/iommu/amd_iommu_debugfs.c
+++ b/drivers/iommu/amd_iommu_debugfs.c
@@ -27,6 +27,8 @@ static DEFINE_RWLOCK(iommu_debugfs_lock);
 
 #defineMAX_NAME_LEN20
 
+static unsigned int amd_iommu_verbose;
+
 static unsigned int amd_iommu_count_valid_dtes(int start, int end)
 {
unsigned int n = 0;
@@ -61,7 +63,10 @@ static ssize_t amd_iommu_debugfs_dtecount_read(struct file 
*filp,
return -ENOMEM;
 
n = amd_iommu_count_valid_dtes(0, 0x);
-   oboff += OSCNPRINTF("%d\n", n);
+   if (amd_iommu_verbose)
+   oboff += OSCNPRINTF("# DTEs:  %d\n", n);
+   else
+   oboff += OSCNPRINTF("%d\n", n);
 
ret = simple_read_from_buffer(ubuf, count, offp, obuf, oboff);
kfree(obuf);
@@ -79,6 +84,7 @@ static const struct file_operations 
amd_iommu_debugfs_dtecount_ops = {
 void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
 {
char name[MAX_NAME_LEN + 1];
+   struct dentry *d_verbose;
struct dentry *d_dte;
unsigned long flags;
 
@@ -97,6 +103,12 @@ void amd_iommu_debugfs_setup(struct amd_iommu *iommu)
if (!iommu->debugfs_instance)
goto err;
 
+   d_verbose = debugfs_create_u32("verbose", 0600,
+  iommu->debugfs_instance,
+  &amd_iommu_verbose);
+   if (!d_verbose)
+   goto err;
+
d_dte = debugfs_create_file("count", 0400,
iommu->debugfs_instance, iommu,
&amd_iommu_debugfs_dtecount_ops);

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/exynos: Use generic group callback

2018-01-26 Thread Marek Szyprowski

Hi Robin,

On 2018-01-24 15:22, Robin Murphy wrote:

Since iommu_group_get_for_dev() already tries iommu_group_get() and will
not call ops->device_group if the group is already non-NULL, the check
in get_device_iommu_group() is always redundant and it reduces to a
duplicate of the generic version; let's just use that one instead.

Signed-off-by: Robin Murphy 


Thanks for this cleanup!

Tested-by: Marek Szyprowski 


---
  drivers/iommu/exynos-iommu.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 79c45650f8de..5d5a9b0a0561 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1238,17 +1238,6 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct 
iommu_domain *iommu_domain,
return phys;
  }
  
-static struct iommu_group *get_device_iommu_group(struct device *dev)

-{
-   struct iommu_group *group;
-
-   group = iommu_group_get(dev);
-   if (!group)
-   group = iommu_group_alloc();
-
-   return group;
-}
-
  static int exynos_iommu_add_device(struct device *dev)
  {
struct exynos_iommu_owner *owner = dev->archdata.iommu;
@@ -1344,7 +1333,7 @@ static const struct iommu_ops exynos_iommu_ops = {
.unmap = exynos_iommu_unmap,
.map_sg = default_iommu_map_sg,
.iova_to_phys = exynos_iommu_iova_to_phys,
-   .device_group = get_device_iommu_group,
+   .device_group = generic_device_group,
.add_device = exynos_iommu_add_device,
.remove_device = exynos_iommu_remove_device,
.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,


Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

2018-01-26 Thread JeffyChen

Hi Robin,

Thanks for your reply.

On 01/24/2018 09:49 PM, Robin Murphy wrote:


+Optional properties:
+- clocks : A list of master clocks requires for the IOMMU to be
accessible


s/requires/required/

ok



+   by the host CPU. The number of clocks depends on the master
+   block and might as well be zero. See [1] for generic clock


Oops, some subtleties of English here :)

To say "the number of clocks ... might as well be zero" effectively
implies "there's no point ever specifying any clocks". I guess what you
really mean here is "...might well be...", i.e. it is both valid and
reasonably likely to require zero clocks.

ok



+   bindings description.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
  Optional properties:
  - rockchip,disable-mmu-reset : Don't use the mmu reset operation.
@@ -27,5 +34,6 @@ Example:
  reg = <0xff940300 0x100>;
  interrupts = ;
  interrupt-names = "vopl_mmu";
+clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>;
  #iommu-cells = <0>;
  };
diff --git a/drivers/iommu/rockchip-iommu.c
b/drivers/iommu/rockchip-iommu.c
index c4131ca792e0..8a5e2a659b67 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -4,6 +4,7 @@
   * published by the Free Software Foundation.
   */
+#include 
  #include 
  #include 
  #include 
@@ -91,6 +92,8 @@ struct rk_iommu {
  struct device *dev;
  void __iomem **bases;
  int num_mmu;
+struct clk_bulk_data *clocks;
+int num_clocks;
  bool reset_disabled;
  struct iommu_device iommu;
  struct list_head node; /* entry in rk_iommu_domain.iommus */
@@ -450,6 +453,38 @@ static int rk_iommu_force_reset(struct rk_iommu
*iommu)
  return 0;
  }
+static int rk_iommu_of_get_clocks(struct rk_iommu *iommu)
+{
+struct device_node *np = iommu->dev->of_node;
+int ret;
+int i;
+
+ret = of_count_phandle_with_args(np, "clocks", "#clock-cells");
+if (ret == -ENOENT)
+return 0;
+else if (ret < 0)
+return ret;
+
+iommu->num_clocks = ret;
+iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
+ sizeof(*iommu->clocks), GFP_KERNEL);
+if (!iommu->clocks)
+return -ENOMEM;
+
+for (i = 0; i < iommu->num_clocks; ++i) {
+iommu->clocks[i].clk = of_clk_get(np, i);
+if (IS_ERR(iommu->clocks[i].clk)) {
+ret = PTR_ERR(iommu->clocks[i].clk);
+goto err_clk_put;
+}
+}


Just to confirm my understanding from a quick scan through the code, the
reason we can't use clk_bulk_get() here is that currently, clocks[i].id
being NULL means we'd end up just getting the first clock multiple
times, right?

right, without a valid name, it would return the first clock.

/* Walk up the tree of devices looking for a clock that matches */
while (np) {
int index = 0;

/*
 * For named clocks, first look up the name in the
 * "clock-names" property.  If it cannot be found, then
 * index will be an error code, and of_clk_get() will fail.
 */
if (name)
index = of_property_match_string(np, "clock-names", name);
clk = __of_clk_get(np, index, dev_id, name);




I guess there could be other users who also want "just get whatever
clocks I have" functionality, so it might be worth proposing that for
the core API as a separate/follow-up patch, but it definitely doesn't
need to be part of this series.

right, i can try to do it later :)


I really don't know enough about correct clk API usage, but modulo the
binding comments it certainly looks nice and tidy now;

Acked-by: Robin Murphy 

thanks.


Thanks,
Robin.



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3] iommu/mediatek: Move attach_device after iommu-group is ready for M4Uv1

2018-01-26 Thread Yong Wu
On Thu, 2018-01-25 at 12:02 +, Robin Murphy wrote:
> On 25/01/18 11:14, Yong Wu wrote:
> > In the commit 05f80300dc8b, the iommu framework has supposed all the
> > iommu drivers have their owner iommu-group, it get rid of the FIXME
> > workarounds while the group is NULL. But the flow of Mediatek M4U gen1
> > looks a bit trick that it will hang at this case:
> > 
> > ==
> > Unable to handle kernel NULL pointer dereference at virtual address 0030
> > pgd = c0004000
> > [0030] *pgd=
> > PC is at mutex_lock+0x28/0x54
> > LR is at iommu_attach_device+0xa4/0xd4
> > pc : []lr : []psr: 6013
> > sp : df0edbb8  ip : df0edbc8  fp : df0edbc4
> > r10: c114da14  r9 : df2a3e40  r8 : 0003
> > r7 : df27a210  r6 : df2a90c4  r5 : 0030  r4 : 
> > r3 : df0f8000  r2 : f000  r1 : df29c610  r0 : 0030
> > Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> > xxx
> > (mutex_lock) from [] (iommu_attach_device+0xa4/0xd4)
> > (iommu_attach_device) from [] 
> > (__arm_iommu_attach_device+0x28/0x90)
> > (__arm_iommu_attach_device) from [] 
> > (arm_iommu_attach_device+0x1c/0x30)
> > (arm_iommu_attach_device) from [] 
> > (mtk_iommu_add_device+0xfc/0x214)
> > (mtk_iommu_add_device) from [] (add_iommu_group+0x3c/0x68)
> > (add_iommu_group) from [] (bus_for_each_dev+0x78/0xac)
> > (bus_for_each_dev) from [] (bus_set_iommu+0xb0/0xec)
> > (bus_set_iommu) from [] (mtk_iommu_probe+0x328/0x368)
> > (mtk_iommu_probe) from [] (platform_drv_probe+0x5c/0xc0)
> > (platform_drv_probe) from [] (driver_probe_device+0x2f4/0x4d8)
> > (driver_probe_device) from [] (__driver_attach+0x10c/0x128)
> > (__driver_attach) from [] (bus_for_each_dev+0x78/0xac)
> > (bus_for_each_dev) from [] (driver_attach+0x2c/0x30)
> > (driver_attach) from [] (bus_add_driver+0x1e0/0x278)
> > (bus_add_driver) from [] (driver_register+0x88/0x108)
> > (driver_register) from [] (__platform_driver_register+0x50/0x58)
> > (__platform_driver_register) from [] (m4u_init+0x24/0x28)
> > (m4u_init) from [] (do_one_initcall+0xf0/0x17c)
> > =
> > 
> > The root cause is that the device's iommu-group is NULL while
> > arm_iommu_attach_device is called. This patch prepare a new iommu-group
> > for the iommu consumer devices to fix this issue.
> > 
> > CC: Robin Murphy 
> > CC: Honghui Zhang 
> > Fixes: 05f80300dc8b ('iommu: Finish making iommu_group support mandatory')
> > Reported-by: Ryder Lee 
> > Signed-off-by: Yong Wu 
> > ---
> > changes notes:
> > v3: don't use the global variable and allocate a new iommu group before
> >  arm_iommu_attach_device following Robin's suggestion.
> > 
> > v2: 
> > http://lists.infradead.org/pipermail/linux-mediatek/2018-January/011810.html
> > Add mtk_domain_v1=NULL in domain_free for symmetry.
> > 
> > v1: https://patchwork.kernel.org/patch/10176255/
> > ---
> >   drivers/iommu/mtk_iommu_v1.c | 49 
> > ++--
> >   1 file changed, 25 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> > index 542930c..aca76d2 100644
> > --- a/drivers/iommu/mtk_iommu_v1.c
> > +++ b/drivers/iommu/mtk_iommu_v1.c
> > @@ -418,20 +418,12 @@ static int mtk_iommu_create_mapping(struct device 
> > *dev,
> > m4udev->archdata.iommu = mtk_mapping;
> > }
> >   
> > -   ret = arm_iommu_attach_device(dev, mtk_mapping);
> > -   if (ret)
> > -   goto err_release_mapping;
> > -
> > return 0;
> > -
> > -err_release_mapping:
> > -   arm_iommu_release_mapping(mtk_mapping);
> > -   m4udev->archdata.iommu = NULL;
> > -   return ret;
> >   }
> >   
> >   static int mtk_iommu_add_device(struct device *dev)
> >   {
> > +   struct dma_iommu_mapping *mtk_mapping;
> > struct of_phandle_args iommu_spec;
> > struct of_phandle_iterator it;
> > struct mtk_iommu_data *data;
> > @@ -452,9 +444,30 @@ static int mtk_iommu_add_device(struct device *dev)
> > if (!dev->iommu_fwspec || dev->iommu_fwspec->ops != &mtk_iommu_ops)
> > return -ENODEV; /* Not a iommu client device */
> >   
> > +   /*
> > +* This is a short-term bodge because the ARM DMA code doesn't
> > +* understand multi-device groups, but we have to call into it
> > +* successfully (and not just rely on a normal IOMMU API attach
> > +* here) in order to set the correct DMA API ops on @dev.
> > +*/
> > +   group = iommu_group_alloc();
> > +   if (IS_ERR(group))
> > +   return PTR_ERR(group);
> > +
> > +   err = iommu_group_add_device(group, dev);
> > +   iommu_group_put(group);
> > +   if (err)
> > +   return err;
> > +
> > data = dev->iommu_fwspec->iommu_priv;
> > -   iommu_device_link(&data->iommu, dev);
> > +   mtk_mapping = data->dev->archdata.iommu;
> > +   err = arm_iommu_attach_device(dev, mtk_mapping);
> > +   if (err) {
> > +   iommu_group_remove_device(dev);
> > +   return err;
> > +   }
>