[PATCH/RFC v2 0/4] iommu/ipmmu-vmsa: IPMMU slave device whitelist V2

[PATCH/RFC v2 0/4] iommu/ipmmu-vmsa: IPMMU slave device whitelist V2

[Magnus Damm]

iommu/ipmmu-vmsa: IPMMU slave device whitelist V2

[PATCH/RFC v2 1/4] iommu/of: Skip IOMMU devices disabled in DT
[PATCH/RFC v2 2/4] iommu/ipmmu-vmsa: Get rid of disabled device check
[PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
[PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Opt-in slave devices based on ES version

Here's an updated prototype that shows how DT integration of IPMMU details
may be integrated and merged upstream based on SoC data sheet ahead of
time followed by enablement in the IPMMU driver code once the appropriate
SoC ES version has been released and the hardware has been tested.

Changes since V1:
 - Broke out patch 1 from the IPMMU driver
 - Moved slave device check from ->add_device() to ->xlate() (Thanks Robin!)
 - Updated white list patch to hook into ->xlate()

Patch 1 may be suitable for upstream merge, however other patches should
in the future if agreed on be rolled into the IPMMU driver series.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
---

 Developed on top of renesas-drivers-2017-01-24-v4.10-rc5

 drivers/iommu/ipmmu-vmsa.c |   59 +++++++++++++++++++++++---------------------
 drivers/iommu/of_iommu.c   |    2 -
 2 files changed, 33 insertions(+), 28 deletions(-)

[PATCH/RFC v2 1/4] iommu/of: Skip IOMMU devices disabled in DT

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Extend the shared IOMMU code to skip over ->xlate() in case the
IOMMU device pointed to by a slave device has been disabled in DT.

Difficult to trigger in case a single IOMMU device is used, however
when multiple IOMMUs are used and some of them are disabled in DT
then this patch makes sure that ->xlate() only gets invoked for the
enabled ones.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
---

 I used to keep this as a local check in the xlate() callback
 for the not-yet-merged-upstream R-Car Gen3 IPMMU driver stack.

 Since honoring DT disabled devices probably makes sense for most users
 it seems like a good plan to try to push it into the common subsystem level.

 Thanks to Geert for suggesting this ages ago.

 Developed on top of renesas-drivers-2017-01-24-v4.10-rc5 which
 includes a recent version of iommu/next.

 drivers/iommu/of_iommu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 0001/drivers/iommu/of_iommu.c
+++ work/drivers/iommu/of_iommu.c	2017-01-27 13:19:22.540607110 +0900
@@ -159,7 +159,7 @@ const struct iommu_ops *of_iommu_configu
 		np = iommu_spec.np;
 		ops = of_iommu_get_ops(np);
 
-		if (!ops || !ops->of_xlate ||
+		if (!ops || !ops->of_xlate || !of_device_is_available(np) ||
 		    iommu_fwspec_init(dev, &np->fwnode, ops) ||
 		    ops->of_xlate(dev, &iommu_spec))
 			goto err_put_node;

[PATCH/RFC v2 2/4] iommu/ipmmu-vmsa: Get rid of disabled device check

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

Since of_iommu_configure() now skips over disabled devices
we can simply drop this check in the IPMMU driver.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 drivers/iommu/ipmmu-vmsa.c |    7 -------
 1 file changed, 7 deletions(-)

--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-01-27 13:09:36.840607110 +0900
@@ -1051,13 +1051,6 @@ static struct iommu_group *ipmmu_device_
 static int ipmmu_of_xlate_dma(struct device *dev,
 			      struct of_phandle_args *spec)
 {
-	/* If the IPMMU device is disabled in DT then return error
-	 * to make sure the of_iommu code does not install ops
-	 * even though the iommu device is disabled
-	 */
-	if (!of_device_is_available(spec->np))
-		return -ENODEV;
-
 	return 0;
 }
 

[PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

Rework the IPMMU code to validate devices in ->xlate() instead of
accepting all devices in xlate() and instead validating devices
in ->add_device(). This makes it possible for the IPMMU device
driver to reject slave devices based on software policy.

Once a slave device is rejected by the ->xlate() callback the shared
function of_iommu_configure() will fail as well which in turn disables
per-device IOMMU handing in the arch-specific mapping code by not
passing any IOMMU callbacks to arch_setup_dma_ops().

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 TODO: Make sure this does not break R-Car Gen2 support

 drivers/iommu/ipmmu-vmsa.c |   27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

--- 0007/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-01-27 13:11:35.970607110 +0900
@@ -1007,16 +1007,14 @@ static int ipmmu_add_device_dma(struct d
 	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 	struct iommu_group *group;
 
-	/* only accept devices with iommus property */
-	if (of_count_phandle_with_args(dev->of_node, "iommus",
-				       "#iommu-cells") < 0)
+	/* The device needs to be verified in xlate() */
+	if (!archdata)
 		return -ENODEV;
 
 	group = iommu_group_get_for_dev(dev);
 	if (IS_ERR(group))
 		return PTR_ERR(group);
 
-	archdata = dev->archdata.iommu;
 	spin_lock(&ipmmu_slave_devices_lock);
 	list_add(&archdata->list, &ipmmu_slave_devices);
 	spin_unlock(&ipmmu_slave_devices_lock);
@@ -1034,24 +1032,13 @@ static void ipmmu_remove_device_dma(stru
 	iommu_group_remove_device(dev);
 }
 
-static struct iommu_group *ipmmu_device_group_dma(struct device *dev)
-{
-	struct iommu_group *group;
-	int ret;
-
-	ret = ipmmu_init_platform_device(dev);
-	if (!ret)
-		group = ipmmu_find_group(dev);
-	else
-		group = ERR_PTR(ret);
-
-	return group;
-}
-
 static int ipmmu_of_xlate_dma(struct device *dev,
 			      struct of_phandle_args *spec)
 {
-	return 0;
+	/* For now only tested on R-Car Gen3 with ARM64 arch init order
+	 * TODO: Test R-Car Gen2 with ARM32 arch init order
+	 */
+	return ipmmu_init_platform_device(dev);
 }
 
 static const struct iommu_ops ipmmu_ops = {
@@ -1065,7 +1052,7 @@ static const struct iommu_ops ipmmu_ops
 	.iova_to_phys = ipmmu_iova_to_phys,
 	.add_device = ipmmu_add_device_dma,
 	.remove_device = ipmmu_remove_device_dma,
-	.device_group = ipmmu_device_group_dma,
+	.device_group = ipmmu_find_group,
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 	.of_xlate = ipmmu_of_xlate_dma,
 };

[PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Opt-in slave devices based on ES version

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

Match on r8a7795 ES2 and enable a certain DMA controller.
In other cases the IPMMU driver remains disabled.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Changes since V1:
 - Perform white list check in ->xlate() instead of ->add_device()

 drivers/iommu/ipmmu-vmsa.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

--- 0009/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-01-27 13:14:47.470607110 +0900
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/sys_soc.h>
 
 #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 #include <asm/dma-iommu.h>
@@ -1032,9 +1033,33 @@ static void ipmmu_remove_device_dma(stru
 	iommu_group_remove_device(dev);
 }
 
+static const struct soc_device_attribute r8a7795es2[] = {
+	{ .soc_id = "r8a7795", .revision = "ES2.*" },
+	{ /* sentinel */ }
+};
+
+static int ipmmu_slave_whitelist(struct device *dev)
+{
+	/* Opt-in slave devices based on SoC and ES version */
+	if (soc_device_match(r8a7795es2)) {
+		if (!strcmp(dev_name(dev), "e7310000.dma-controller"))
+			return 0;
+	}
+
+	/* By default, do not allow use of IPMMU */
+	return -ENODEV;
+}
+
 static int ipmmu_of_xlate_dma(struct device *dev,
 			      struct of_phandle_args *spec)
 {
+	int ret;
+
+	/* Opt-in devices based on SoC and ES version */
+	ret = ipmmu_slave_whitelist(dev);
+	if (ret)
+		return ret;
+
 	/* For now only tested on R-Car Gen3 with ARM64 arch init order
 	 * TODO: Test R-Car Gen2 with ARM32 arch init order
 	 */

Re: [PATCH/RFC v2 0/4] iommu/ipmmu-vmsa: IPMMU slave device whitelist V2

[Magnus Damm]

On Fri, Jan 27, 2017 at 3:14 PM, Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> iommu/ipmmu-vmsa: IPMMU slave device whitelist V2
>
> [PATCH/RFC v2 1/4] iommu/of: Skip IOMMU devices disabled in DT
> [PATCH/RFC v2 2/4] iommu/ipmmu-vmsa: Get rid of disabled device check
> [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
> [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Opt-in slave devices based on ES version
>
> Here's an updated prototype that shows how DT integration of IPMMU details
> may be integrated and merged upstream based on SoC data sheet ahead of
> time followed by enablement in the IPMMU driver code once the appropriate
> SoC ES version has been released and the hardware has been tested.
>
> Changes since V1:
>  - Broke out patch 1 from the IPMMU driver
>  - Moved slave device check from ->add_device() to ->xlate() (Thanks Robin!)
>  - Updated white list patch to hook into ->xlate()
>
> Patch 1 may be suitable for upstream merge, however other patches should
> in the future if agreed on be rolled into the IPMMU driver series.

Hi Geert, everyone,

Do you have any opinion about the code in this version of the series?

I recall that you agreed with the approach in "[PATCH/RFC 2/2]
iommu/ipmmu-vmsa: Opt-in slave devices based on ES version", however
it was suggested to me by Robin that my code in "[PATCH/RFC 1/2]
arm64: mm: Silently allow devices lacking IOMMU group" should be
reworked to use ->xlate().

Now this series makes use of ->xlate() to implement the white list, so
I hope that makes everyone happy.

Also, based on your suggestion I finally managed to break out the code
that skips over disabled devices in "[PATCH/RFC v2 1/4] iommu/of: Skip
IOMMU devices disabled in DT" but I'm not sure if this will cause
problem for other platforms.

Anyway, my current plan is to wait for feedback for "[PATCH/RFC v2
1/4] iommu/of: Skip IOMMU devices disabled in DT" and handle that
independently, and also roll in the other changes in this series into
my other IPMMU code.

Thanks,

/ magnus

Re: [PATCH/RFC v2 0/4] iommu/ipmmu-vmsa: IPMMU slave device whitelist V2

[Joerg Roedel]

On Fri, Jan 27, 2017 at 03:14:07PM +0900, Magnus Damm wrote:
> iommu/ipmmu-vmsa: IPMMU slave device whitelist V2
> 
> [PATCH/RFC v2 1/4] iommu/of: Skip IOMMU devices disabled in DT
> [PATCH/RFC v2 2/4] iommu/ipmmu-vmsa: Get rid of disabled device check
> [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
> [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Opt-in slave devices based on ES version
> 
> Here's an updated prototype that shows how DT integration of IPMMU details
> may be integrated and merged upstream based on SoC data sheet ahead of
> time followed by enablement in the IPMMU driver code once the appropriate
> SoC ES version has been released and the hardware has been tested.
> 
> Changes since V1:
>  - Broke out patch 1 from the IPMMU driver
>  - Moved slave device check from ->add_device() to ->xlate() (Thanks Robin!)
>  - Updated white list patch to hook into ->xlate()
> 
> Patch 1 may be suitable for upstream merge, however other patches should
> in the future if agreed on be rolled into the IPMMU driver series.
> 
> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> ---
> 
>  Developed on top of renesas-drivers-2017-01-24-v4.10-rc5
> 
>  drivers/iommu/ipmmu-vmsa.c |   59 +++++++++++++++++++++++---------------------
>  drivers/iommu/of_iommu.c   |    2 -
>  2 files changed, 33 insertions(+), 28 deletions(-)

For the series:

Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>