iommu/pamu: Support generic iommu-device handles

Hi,

Here is a patch-set to support the iommu_device_register() interface in the fsl-pamu iommu driver. To make it work a few fixes (Patch 1 and 2), an additional check (Patch 3) were necessary.

Please review.

Regards,

Joerg

Joerg Roedel (4):
iommu/pamu: Let PAMU depend on PCI
iommu/pamu: Make driver depend on CONFIG_PHYS_64BIT iommu/pamu: WARN when fsl_pamu_probe() is called more than once iommu/pamu: Add support for generic iommu-device

 drivers/iommu/Kconfig           |  2 ++
 drivers/iommu/fsl_pamu.c        | 24 ++++++++++++++++++++++++
 drivers/iommu/fsl_pamu.h        |  3 +++
 drivers/iommu/fsl_pamu_domain.c |  5 ++++-
 drivers/iommu/fsl_pamu_domain.h |  2 ++
 5 files changed, 35 insertions(+), 1 deletion(-)

2.7.4

From: Joerg Roedel <[email protected]>

The driver does not compile when PCI is not selected, so make it depend on it.

---
 drivers/iommu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

index f73ff28..e73b7c5 100644
@@ -76,6 +76,7 @@ config IOMMU_DMA
 
 config FSL_PAMU
 	bool "Freescale IOMMU support"
+	depends on PCI
 	depends on PPC_E500MC || (COMPILE_TEST && PPC)
 	select IOMMU_API
 	select GENERIC_ALLOCATOR

2.7.4

From: Joerg Roedel <[email protected]>

Certain address calculations in the driver make the assumption that phys_addr_t and dma_addr_t are 64 bit wide. Force this by depending on CONFIG_PHYS_64BIT to be set.

---
 drivers/iommu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

index e73b7c5..e163b02 100644
@@ -77,6 +77,7 @@ config IOMMU_DMA
 config FSL_PAMU
 	bool "Freescale IOMMU support"
 	depends on PCI
+	depends on PHYS_64BIT
 	depends on PPC_E500MC || (COMPILE_TEST && PPC)
 	select IOMMU_API
 	select GENERIC_ALLOCATOR

2.7.4

From: Joerg Roedel <[email protected]>

The function probes the PAMU hardware from device-tree specifications. It initializes global variables and can thus be only safely called once.

Add a check that that prints a warning when its called more than once.

---
 drivers/iommu/fsl_pamu.c | 7 +++++++
 1 file changed, 7 insertions(+)

index a34355f..9ee8e9e 100644
@@ -42,6 +42,8 @@ struct pamu_isr_data {
 static struct paace *ppaact;
 static struct paace *spaact;
 
+static bool probed;			/* Has PAMU been probed? */
+
 /*
  * Table for matching compatible strings, for device tree
  * guts node, for QorIQ SOCs.

@@ -1033,6 +1035,9 @@ static int fsl_pamu_probe(struct platform_device *pdev)
 	 * NOTE : All PAMUs share the same LIODN tables.
 	 */
 
+	if (WARN_ON(probed))
+		return -EBUSY;
+
 	pamu_regs = of_iomap(dev->of_node, 0);
 	if (!pamu_regs) {
 		dev_err(dev, "ioremap of PAMU node failed\n");

@@ -1172,6 +1177,8 @@ static int fsl_pamu_probe(struct platform_device *pdev)
 
 	setup_liodns();
 
+	probed = true;
+
 	return 0;
 
 error_genpool:

2.7.4

From: Joerg Roedel <[email protected]>

This patch adds a global iommu-handle to the pamu driver and initializes it at probe time. Also link devices added to the iommu to this handle.

---
 drivers/iommu/fsl_pamu.c        | 17 +++++++++++++++++
 drivers/iommu/fsl_pamu.h        |  3 +++
 drivers/iommu/fsl_pamu_domain.c |  5 ++++-
 drivers/iommu/fsl_pamu_domain.h |  2 ++
 4 files changed, 26 insertions(+), 1 deletion(-)

index 9ee8e9e..9238a85 100644
@@ -44,6 +44,8 @@ static struct paace *spaact;
 
 static bool probed;			/* Has PAMU been probed? */
 
+struct iommu_device pamu_iommu;	/* IOMMU core code handle */
+
 /*
  * Table for matching compatible strings, for device tree
  * guts node, for QorIQ SOCs.

@@ -1154,6 +1156,18 @@ static int fsl_pamu_probe(struct platform_device *pdev)
 	if (ret)
 		goto error_genpool;
 
+	ret = iommu_device_sysfs_add(&pamu_iommu, dev, NULL, "iommu0");
+	if (ret)
+		goto error_genpool;
+
+	iommu_device_set_ops(&pamu_iommu, &fsl_pamu_ops);
+
+	ret = iommu_device_register(&pamu_iommu);
+	if (ret) {
+		dev_err(dev, "Can't register iommu device\n");
+		goto error_sysfs;
+	}
+
 	pamubypenr = in_be32(&guts_regs->pamubypenr);
 
 	for (pamu_reg_off = 0, pamu_counter = 0x80000000; pamu_reg_off < size;

@@ -1181,6 +1195,9 @@ static int fsl_pamu_probe(struct platform_device *pdev)
 
 	return 0;
 
+error_sysfs:
+	iommu_device_sysfs_remove(&pamu_iommu);
+
 error_genpool:
 	gen_pool_destroy(spaace_pool);
 

index c3434f2..fa48222 100644
@@ -391,6 +391,9 @@ struct ome {
 #define EOE_WWSAOL      0x1e    /* Write with stash allocate only and lock */
 #define EOE_VALID       0x80
 
+extern const struct iommu_ops fsl_pamu_ops;
+extern struct iommu_device pamu_iommu;	/* IOMMU core code handle */
+
 /* Function prototypes */
 int pamu_domain_init(void);
 int pamu_enable_liodn(int liodn);

index da0e1e3..914953b 100644
@@ -983,11 +983,14 @@ static int fsl_pamu_add_device(struct device *dev)
 
 	iommu_group_put(group);
 
+	iommu_device_link(&pamu_iommu, dev);
+
 	return 0;
 }
 
 static void fsl_pamu_remove_device(struct device *dev)
 {
+	iommu_device_unlink(&pamu_iommu, dev);
 	iommu_group_remove_device(dev);
 }
 

@@ -1047,7 +1050,7 @@ static u32 fsl_pamu_get_windows(struct iommu_domain *domain)
 	return dma_domain->win_cnt;
 }
 
-static const struct iommu_ops fsl_pamu_ops = {
+const struct iommu_ops fsl_pamu_ops = {
 	.capable	= fsl_pamu_capable,
 	.domain_alloc	= fsl_pamu_domain_alloc,
 	.domain_free    = fsl_pamu_domain_free,

index f2b0f74..6d8661e 100644
@@ -21,6 +21,8 @@
 
 #include "fsl_pamu.h"
 
+const struct iommu_ops fsl_pamu_ops;
+
 struct dma_window {
 	phys_addr_t paddr;
 	u64 size;

2.7.4

This seems to be causing my p5020ds to blow up with:

[    0.096728] Machine: fsl,P5020DS
[    0.096729] SoC family: QorIQ
[    0.096730] SoC ID: svr:0x82280020, Revision: 2.0
[    0.098143] Found FSL PCI host bridge at 0x0000000ffe200000. Firmware bus number: 0->0
[    0.098145] PCI host bridge /pcie@ffe200000  ranges:
[    0.098149]  MEM 0x0000000c00000000..0x0000000c1fffffff -> 0x00000000e0000000 
[    0.098151]   IO 0x0000000ff8000000..0x0000000ff800ffff -> 0x0000000000000000
[    0.098161] /pcie@ffe200000: PCICSRBAR @ 0xdf000000
[    0.098162] setup_pci_atmu: end of DRAM 100000000
[    0.098167] /pcie@ffe200000: Setup 64-bit PCI DMA window
[    0.098168] /pcie@ffe200000: DMA window size is 0xdf000000
[    0.098326] Found FSL PCI host bridge at 0x0000000ffe202000. Firmware bus number: 0->1
[    0.098327] PCI host bridge /pcie@ffe202000  ranges:
[    0.098330]  MEM 0x0000000c40000000..0x0000000c5fffffff -> 0x00000000e0000000 
[    0.098332]   IO 0x0000000ff8020000..0x0000000ff802ffff -> 0x0000000000000000
[    0.098340] /pcie@ffe202000: PCICSRBAR @ 0xdf000000
[    0.098340] setup_pci_atmu: end of DRAM 100000000
[    0.098345] /pcie@ffe202000: Setup 64-bit PCI DMA window
[    0.098346] /pcie@ffe202000: DMA window size is 0xdf000000
[    0.204289] iommu: Adding device ffe100300.dma to group 0
[    0.209606] Unable to handle kernel paging request for data at address 0x00000068
[    0.217059] Faulting instruction address: 0xc0000000002abe18
[    0.222703] Oops: Kernel access of bad area, sig: 11 [#1]
[    0.228078] SMP NR_CPUS=24 
[    0.228080] CoreNet Generic
[    0.233634] Modules linked in:
[    0.236674] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc5-gcc-6.3.1-00004-g68a17f0be6fe #41
[    0.245613] task: c0000000f7168000 task.stack: c0000000f7170000
[    0.251515] NIP: c0000000002abe18 LR: c0000000002abf7c CTR: c000000000025dc8
[    0.258545] REGS: c0000000f7173400 TRAP: 0300   Not tainted  (4.13.0-rc5-gcc-6.3.1-00004-g68a17f0be6fe)
[    0.267919] MSR: 0000000080029000 <CE,EE,ME>
[    0.267924]   CR: 28adb242  XER: 20000000
[    0.276165] DEAR: 0000000000000068 ESR: 0000000000000000 SOFTE: 1 
[    0.276165] GPR00: c0000000002abf7c c0000000f7173680 c000000000fbf600 0000000000000000 
[    0.276165] GPR04: c000000000c6dc88 0000000000000000 c0000000f72e0190 0000000000000000 
[    0.276165] GPR08: c0000000f7168000 0000000000000000 0000000000000000 0000000000000038 
[    0.276165] GPR12: 0000000022adb444 c00000003fff5000 c000000000001b70 0000000000000000 
[    0.276165] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[    0.276165] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[    0.276165] GPR24: 0000000000000000 c000000000ef4f58 c000000000d3b408 c000000000c6dc88 
[    0.276165] GPR28: c000000000c6dc88 0000000000000000 0000000000000000 c000000000e3b658 
[    0.346218] NIP [c0000000002abe18] .kernfs_find_ns+0x30/0x148
[    0.351943] LR [c0000000002abf7c] .kernfs_find_and_get_ns+0x4c/0x7c
[    0.358192] Call Trace:
[    0.360624] [c0000000f7173680] [c0000000f7173710] 0xc0000000f7173710 (unreliable)
[    0.368088] [c0000000f7173710] [c0000000002abf7c] .kernfs_find_and_get_ns+0x4c/0x7c
[    0.375729] [c0000000f71737a0] [c0000000002b13c8] .sysfs_add_link_to_group+0x44/0x9c
[    0.383456] [c0000000f7173840] [c000000000591660] .iommu_device_link+0x70/0x144
[    0.390744] [c0000000f71738e0] [c0000000005942dc] .fsl_pamu_add_device+0x4c/0x80
[    0.398121] [c0000000f7173960] [c00000000058ce8c] .add_iommu_group+0x5c/0x9c
[    0.405153] [c0000000f71739e0] [c00000000059d6b8] .bus_for_each_dev+0x98/0xfc
[    0.412269] [c0000000f7173a80] [c00000000058f1a0] .bus_set_iommu+0xd8/0x11c
[    0.419218] [c0000000f7173b20] [c000000000d86998] .pamu_domain_init+0xb0/0xe0
[    0.426331] [c0000000f7173ba0] [c000000000d86864] .fsl_pamu_init+0xec/0x170
[    0.433276] [c0000000f7173c30] [c000000000001934] .do_one_initcall+0x68/0x1b8
[    0.440395] [c0000000f7173d00] [c000000000d440e4] .kernel_init_freeable+0x24c/0x324
[    0.448031] [c0000000f7173db0] [c000000000001b90] .kernel_init+0x20/0x140
[    0.454801] [c0000000f7173e30] [c0000000000009bc] .ret_from_kernel_thread+0x58/0x9c
[    0.462438] Instruction dump:
[    0.465390] 7c0802a6 fb81ffe0 f8010010 fba1ffe8 fbc1fff0 fbe1fff8 f821ff71 7c7e1b78 
[    0.473114] 7cbd2b78 7c9c2378 60000000 60000000 <a13e0068> 311dffff ebfe0048 552906b4 
[    0.481017] ---[ end trace 5d11d3e6c29380bd ]---

# first bad commit: [68a17f0be6feb8de1f5e26b93f49791031374c4c] iommu/pamu: Add support for generic iommu-device

commit 68a17f0be6feb8de1f5e26b93f49791031374c4c

Author:     Joerg Roedel <[email protected]>
AuthorDate: Wed Aug 9 16:36:00 2017 +0200
Commit:     Joerg Roedel <[email protected]>
CommitDate: Tue Aug 15 13:59:34 2017 +0200

iommu/pamu: Add support for generic iommu-device

This patch adds a global iommu-handle to the pamu driver and initializes it at probe time. Also link devices added to the iommu to this handle.

cheers

Hello Michael,

Thanks for the report, this looks like an initialization ordering problem. Can you please try the attached patch?

index 9238a85de53e..9ee8e9e161f5 100644
@@ -44,8 +44,6 @@ static struct paace *spaact;
 
 static bool probed;			/* Has PAMU been probed? */
 
-struct iommu_device pamu_iommu;	/* IOMMU core code handle */
-
 /*
  * Table for matching compatible strings, for device tree
  * guts node, for QorIQ SOCs.

@@ -1156,18 +1154,6 @@ static int fsl_pamu_probe(struct platform_device *pdev)
 	if (ret)
 		goto error_genpool;
 
-	ret = iommu_device_sysfs_add(&pamu_iommu, dev, NULL, "iommu0");
-	if (ret)
-		goto error_genpool;
-
-	iommu_device_set_ops(&pamu_iommu, &fsl_pamu_ops);
-
-	ret = iommu_device_register(&pamu_iommu);
-	if (ret) {
-		dev_err(dev, "Can't register iommu device\n");
-		goto error_sysfs;
-	}
-
 	pamubypenr = in_be32(&guts_regs->pamubypenr);
 
 	for (pamu_reg_off = 0, pamu_counter = 0x80000000; pamu_reg_off < size;

@@ -1195,9 +1181,6 @@ static int fsl_pamu_probe(struct platform_device *pdev)
 
 	return 0;
 
-error_sysfs:
-	iommu_device_sysfs_remove(&pamu_iommu);
-
 error_genpool:
 	gen_pool_destroy(spaace_pool);
 

index fa48222f3421..c3434f29c967 100644
@@ -391,9 +391,6 @@ struct ome {
 #define EOE_WWSAOL      0x1e    /* Write with stash allocate only and lock */
 #define EOE_VALID       0x80
 
-extern const struct iommu_ops fsl_pamu_ops;
-extern struct iommu_device pamu_iommu;	/* IOMMU core code handle */
-
 /* Function prototypes */
 int pamu_domain_init(void);
 int pamu_enable_liodn(int liodn);

index 914953b87bf1..e0fcd079cca9 100644
@@ -33,6 +33,8 @@ static struct kmem_cache *fsl_pamu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 static DEFINE_SPINLOCK(device_domain_lock);
 
+struct iommu_device pamu_iommu;	/* IOMMU core code handle */
+
 static struct fsl_dma_domain *to_fsl_dma_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct fsl_dma_domain, iommu_domain);

@@ -1050,7 +1052,7 @@ static u32 fsl_pamu_get_windows(struct iommu_domain *domain)
 	return dma_domain->win_cnt;
 }
 
-const struct iommu_ops fsl_pamu_ops = {
+static const struct iommu_ops fsl_pamu_ops = {
 	.capable	= fsl_pamu_capable,
 	.domain_alloc	= fsl_pamu_domain_alloc,
 	.domain_free    = fsl_pamu_domain_free,

@@ -1076,6 +1078,19 @@ int __init pamu_domain_init(void)
 	if (ret)
 		return ret;
 
+	ret = iommu_device_sysfs_add(&pamu_iommu, NULL, NULL, "iommu0");
+	if (ret)
+		return ret;
+
+	iommu_device_set_ops(&pamu_iommu, &fsl_pamu_ops);
+
+	ret = iommu_device_register(&pamu_iommu);
+	if (ret) {
+		iommu_device_sysfs_remove(&pamu_iommu);
+		pr_err("Can't register iommu device\n");
+		return ret;
+	}
+
 	bus_set_iommu(&platform_bus_type, &fsl_pamu_ops);
 	bus_set_iommu(&pci_bus_type, &fsl_pamu_ops);
 

index 6d8661e488fb..f2b0f741d3de 100644
@@ -21,8 +21,6 @@
 
 #include "fsl_pamu.h"
 
-const struct iommu_ops fsl_pamu_ops;
-
 struct dma_window {
 	phys_addr_t paddr;
 	u64 size;

Yes I suspect so.

Thanks, that works.

It boots happily, much later in boot I see these messages:

[    2.085616] iommu: Adding device ffe301000.jr to group 9
[    2.091101] iommu: Adding device ffe302000.jr to group 10
[    2.096633] iommu: Adding device ffe303000.jr to group 11
[    2.102161] iommu: Adding device ffe304000.jr to group 12

In /sys I see:

root@p5020ds:~# ls -l /sys/class/iommu/iommu0
lrwxrwxrwx 1 root root 0 Aug 24 12:01 /sys/class/iommu/iommu0 -> ../../devices/virtual/iommu/iommu0

And:

root@p5020ds:~# ls -l /sys/devices/virtual/iommu/iommu0/devices/
total 0
lrwxrwxrwx 1 root root 0 Aug 24 12:02 0000:00:00.0 -> ../../../../platform/ffe200000.pcie/pci0000:00/0000:00:00.0
lrwxrwxrwx 1 root root 0 Aug 24 12:02 2000:00:00.0 -> ../../../../platform/ffe202000.pcie/pci2000:00/2000:00:00.0
lrwxrwxrwx 1 root root 0 Aug 24 12:02 2000:01:00.0 -> ../../../../platform/ffe202000.pcie/pci2000:00/2000:00:00.0/2000:01:00.0
lrwxrwxrwx 1 root root 0 Aug 24 12:02 2000:01:00.1 -> ../../../../platform/ffe202000.pcie/pci2000:00/2000:00:00.0/2000:01:00.1
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe100300.dma -> ../../../../platform/ffe000000.soc/ffe100300.dma
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe101300.dma -> ../../../../platform/ffe000000.soc/ffe101300.dma
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe114000.sdhc -> ../../../../platform/ffe000000.soc/ffe114000.sdhc
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe200000.pcie -> ../../../../platform/ffe200000.pcie
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe202000.pcie -> ../../../../platform/ffe202000.pcie
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe210000.usb -> ../../../../platform/ffe000000.soc/ffe210000.usb
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe211000.usb -> ../../../../platform/ffe000000.soc/ffe211000.usb
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe220000.sata -> ../../../../platform/ffe000000.soc/ffe220000.sata
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe221000.sata -> ../../../../platform/ffe000000.soc/ffe221000.sata
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe301000.jr -> ../../../../platform/ffe000000.soc/ffe300000.crypto/ffe301000.jr
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe302000.jr -> ../../../../platform/ffe000000.soc/ffe300000.crypto/ffe302000.jr
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe303000.jr -> ../../../../platform/ffe000000.soc/ffe300000.crypto/ffe303000.jr
lrwxrwxrwx 1 root root 0 Aug 24 12:02 ffe304000.jr -> ../../../../platform/ffe000000.soc/ffe300000.crypto/ffe304000.jr

Which seems like it's working?

cheers

Hi Michael,

Great, thanks for testing it. I'll queue it on-top of the original patch-set.

Looks good.

Exactly, that is what I'd expect to see with the patches.

Thanks again and cheers,

Joerg

Thanks.

cheers