IOMMU Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] iommu: Support reserved-memory regions
@ 2019-08-29 11:14 Thierry Reding
  2019-08-29 11:14 ` [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions() Thierry Reding
  2019-08-29 11:14 ` [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions() Thierry Reding
  0 siblings, 2 replies; 10+ messages in thread
From: Thierry Reding @ 2019-08-29 11:14 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: devicetree, Frank Rowand, Robin Murphy, linux-kernel, iommu,
	Rob Herring, Will Deacon

From: Thierry Reding <treding@nvidia.com>

These two patches implement support for retrieving a list of reserved
regions for a device from its device tree node. These regions are
described by the reserved-memory bindings:

	Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

These reserved memory regions will be used to establish 1:1 mappings.
One case where this is useful is when the Linux kernel wants to take
over the display controller configuration from a bootloader. In order
to ensure that the display controller can keep scanning out from the
framebuffer allocated by the bootloader without faulting after the
IOMMU has been enabled, a 1:1 mapping needs to be established.

Thierry

Thierry Reding (2):
  iommu: Implement of_iommu_get_resv_regions()
  iommu: dma: Use of_iommu_get_resv_regions()

 drivers/iommu/dma-iommu.c |  3 +++
 drivers/iommu/of_iommu.c  | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/of_iommu.h  |  8 ++++++++
 3 files changed, 50 insertions(+)

-- 
2.22.0

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions()
  2019-08-29 11:14 [PATCH 0/2] iommu: Support reserved-memory regions Thierry Reding
@ 2019-08-29 11:14 ` Thierry Reding
  2019-09-02 13:38   ` Rob Herring
  2019-09-02 13:54   ` Robin Murphy
  2019-08-29 11:14 ` [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions() Thierry Reding
  1 sibling, 2 replies; 10+ messages in thread
From: Thierry Reding @ 2019-08-29 11:14 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: devicetree, Frank Rowand, Robin Murphy, linux-kernel, iommu,
	Rob Herring, Will Deacon

From: Thierry Reding <treding@nvidia.com>

This is an implementation that IOMMU drivers can use to obtain reserved
memory regions from a device tree node. It uses the reserved-memory DT
bindings to find the regions associated with a given device. These
regions will be used to create 1:1 mappings in the IOMMU domain that
the devices will be attached to.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/of_iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/of_iommu.h |  8 ++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 614a93aa5305..0d47f626b854 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -9,6 +9,7 @@
 #include <linux/iommu.h>
 #include <linux/limits.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_iommu.h>
 #include <linux/of_pci.h>
 #include <linux/slab.h>
@@ -225,3 +226,41 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
 
 	return ops;
 }
+
+/**
+ * of_iommu_get_resv_regions - reserved region driver helper for device tree
+ * @dev: device for which to get reserved regions
+ * @list: reserved region list
+ *
+ * IOMMU drivers can use this to implement their .get_resv_regions() callback
+ * for memory regions attached to a device tree node. See the reserved-memory
+ * device tree bindings on how to use these:
+ *
+ *   Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+ */
+void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
+{
+	struct of_phandle_iterator it;
+	int err;
+
+	of_for_each_phandle(&it, err, dev->of_node, "memory-region", NULL, 0) {
+		struct iommu_resv_region *region;
+		struct resource res;
+
+		err = of_address_to_resource(it.node, 0, &res);
+		if (err < 0) {
+			dev_err(dev, "failed to parse memory region %pOF: %d\n",
+				it.node, err);
+			continue;
+		}
+
+		region = iommu_alloc_resv_region(res.start, resource_size(&res),
+						 IOMMU_READ | IOMMU_WRITE,
+						 IOMMU_RESV_DIRECT_RELAXABLE);
+		if (!region)
+			continue;
+
+		list_add_tail(&region->list, list);
+	}
+}
+EXPORT_SYMBOL(of_iommu_get_resv_regions);
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index f3d40dd7bb66..fa16b26f55bc 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -15,6 +15,9 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
 extern const struct iommu_ops *of_iommu_configure(struct device *dev,
 					struct device_node *master_np);
 
+extern void of_iommu_get_resv_regions(struct device *dev,
+				      struct list_head *list);
+
 #else
 
 static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
@@ -30,6 +33,11 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 	return NULL;
 }
 
+static inline void of_iommu_get_resv_regions(struct device *dev,
+					     struct list_head *list)
+{
+}
+
 #endif	/* CONFIG_OF_IOMMU */
 
 #endif /* __OF_IOMMU_H */
-- 
2.22.0

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions()
  2019-08-29 11:14 [PATCH 0/2] iommu: Support reserved-memory regions Thierry Reding
  2019-08-29 11:14 ` [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions() Thierry Reding
@ 2019-08-29 11:14 ` Thierry Reding
  2019-09-02 14:22   ` Robin Murphy
  1 sibling, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2019-08-29 11:14 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: devicetree, Frank Rowand, Robin Murphy, linux-kernel, iommu,
	Rob Herring, Will Deacon

From: Thierry Reding <treding@nvidia.com>

For device tree nodes, use the standard of_iommu_get_resv_regions()
implementation to obtain the reserved memory regions associated with a
device.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/dma-iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index de68b4a02aea..31d48e55ab55 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -19,6 +19,7 @@
 #include <linux/iova.h>
 #include <linux/irq.h>
 #include <linux/mm.h>
+#include <linux/of_iommu.h>
 #include <linux/pci.h>
 #include <linux/scatterlist.h>
 #include <linux/vmalloc.h>
@@ -164,6 +165,8 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 	if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
 		iort_iommu_msi_get_resv_regions(dev, list);
 
+	if (dev->of_node)
+		of_iommu_get_resv_regions(dev, list);
 }
 EXPORT_SYMBOL(iommu_dma_get_resv_regions);
 
-- 
2.22.0

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions()
  2019-08-29 11:14 ` [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions() Thierry Reding
@ 2019-09-02 13:38   ` Rob Herring
  2019-09-02 13:54   ` Robin Murphy
  1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2019-09-02 13:38 UTC (permalink / raw)
  To: Thierry Reding; +Cc: , devicetree, linux-kernel, iommu, Robin Murphy

On Thu, 29 Aug 2019 13:14:06 +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> This is an implementation that IOMMU drivers can use to obtain reserved
> memory regions from a device tree node. It uses the reserved-memory DT
> bindings to find the regions associated with a given device. These
> regions will be used to create 1:1 mappings in the IOMMU domain that
> the devices will be attached to.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/iommu/of_iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
>  include/linux/of_iommu.h |  8 ++++++++
>  2 files changed, 47 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions()
  2019-08-29 11:14 ` [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions() Thierry Reding
  2019-09-02 13:38   ` Rob Herring
@ 2019-09-02 13:54   ` Robin Murphy
  2019-09-02 15:00     ` Thierry Reding
  1 sibling, 1 reply; 10+ messages in thread
From: Robin Murphy @ 2019-09-02 13:54 UTC (permalink / raw)
  To: Thierry Reding, Joerg Roedel
  Cc: devicetree, Frank Rowand, linux-kernel, iommu, Rob Herring, Will Deacon

On 29/08/2019 12:14, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> This is an implementation that IOMMU drivers can use to obtain reserved
> memory regions from a device tree node. It uses the reserved-memory DT
> bindings to find the regions associated with a given device. These
> regions will be used to create 1:1 mappings in the IOMMU domain that
> the devices will be attached to.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>   drivers/iommu/of_iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
>   include/linux/of_iommu.h |  8 ++++++++
>   2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 614a93aa5305..0d47f626b854 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -9,6 +9,7 @@
>   #include <linux/iommu.h>
>   #include <linux/limits.h>
>   #include <linux/of.h>
> +#include <linux/of_address.h>
>   #include <linux/of_iommu.h>
>   #include <linux/of_pci.h>
>   #include <linux/slab.h>
> @@ -225,3 +226,41 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
>   
>   	return ops;
>   }
> +
> +/**
> + * of_iommu_get_resv_regions - reserved region driver helper for device tree
> + * @dev: device for which to get reserved regions
> + * @list: reserved region list
> + *
> + * IOMMU drivers can use this to implement their .get_resv_regions() callback
> + * for memory regions attached to a device tree node. See the reserved-memory
> + * device tree bindings on how to use these:
> + *
> + *   Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> + */
> +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
> +{
> +	struct of_phandle_iterator it;
> +	int err;
> +
> +	of_for_each_phandle(&it, err, dev->of_node, "memory-region", NULL, 0) {
> +		struct iommu_resv_region *region;
> +		struct resource res;
> +
> +		err = of_address_to_resource(it.node, 0, &res);
> +		if (err < 0) {
> +			dev_err(dev, "failed to parse memory region %pOF: %d\n",
> +				it.node, err);
> +			continue;
> +		}

What if the device node has memory regions for other purposes, like 
private CMA carveouts? We wouldn't want to force mappings of those (and 
in the very worst case doing so could even render them unusable).

Robin.

> +
> +		region = iommu_alloc_resv_region(res.start, resource_size(&res),
> +						 IOMMU_READ | IOMMU_WRITE,
> +						 IOMMU_RESV_DIRECT_RELAXABLE);
> +		if (!region)
> +			continue;
> +
> +		list_add_tail(&region->list, list);
> +	}
> +}
> +EXPORT_SYMBOL(of_iommu_get_resv_regions);
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index f3d40dd7bb66..fa16b26f55bc 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -15,6 +15,9 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
>   extern const struct iommu_ops *of_iommu_configure(struct device *dev,
>   					struct device_node *master_np);
>   
> +extern void of_iommu_get_resv_regions(struct device *dev,
> +				      struct list_head *list);
> +
>   #else
>   
>   static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
> @@ -30,6 +33,11 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>   	return NULL;
>   }
>   
> +static inline void of_iommu_get_resv_regions(struct device *dev,
> +					     struct list_head *list)
> +{
> +}
> +
>   #endif	/* CONFIG_OF_IOMMU */
>   
>   #endif /* __OF_IOMMU_H */
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions()
  2019-08-29 11:14 ` [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions() Thierry Reding
@ 2019-09-02 14:22   ` Robin Murphy
  2019-09-02 14:52     ` Thierry Reding
  0 siblings, 1 reply; 10+ messages in thread
From: Robin Murphy @ 2019-09-02 14:22 UTC (permalink / raw)
  To: Thierry Reding, Joerg Roedel
  Cc: devicetree, Frank Rowand, linux-kernel, iommu, Rob Herring, Will Deacon

On 29/08/2019 12:14, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> For device tree nodes, use the standard of_iommu_get_resv_regions()
> implementation to obtain the reserved memory regions associated with a
> device.

This covers the window between iommu_probe_device() setting up a default 
domain and the device's driver finally probing and taking control, but 
iommu_probe_device() represents the point that the IOMMU driver first 
knows about this device - there's still a window from whenever the IOMMU 
driver itself probed up to here where the "unidentified" traffic may 
have already been disrupted. Some IOMMU drivers have no option but to 
make the necessary configuration during their own probe routine, at 
which point a struct device for the display/etc. endpoint may not even 
exist yet.

Robin.

> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>   drivers/iommu/dma-iommu.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index de68b4a02aea..31d48e55ab55 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -19,6 +19,7 @@
>   #include <linux/iova.h>
>   #include <linux/irq.h>
>   #include <linux/mm.h>
> +#include <linux/of_iommu.h>
>   #include <linux/pci.h>
>   #include <linux/scatterlist.h>
>   #include <linux/vmalloc.h>
> @@ -164,6 +165,8 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
>   	if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
>   		iort_iommu_msi_get_resv_regions(dev, list);
>   
> +	if (dev->of_node)
> +		of_iommu_get_resv_regions(dev, list);
>   }
>   EXPORT_SYMBOL(iommu_dma_get_resv_regions);
>   
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions()
  2019-09-02 14:22   ` Robin Murphy
@ 2019-09-02 14:52     ` Thierry Reding
  2019-09-17 17:59       ` Will Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2019-09-02 14:52 UTC (permalink / raw)
  To: Robin Murphy
  Cc: devicetree, Frank Rowand, linux-kernel, iommu, Rob Herring, Will Deacon

[-- Attachment #1.1: Type: text/plain, Size: 2661 bytes --]

On Mon, Sep 02, 2019 at 03:22:35PM +0100, Robin Murphy wrote:
> On 29/08/2019 12:14, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > For device tree nodes, use the standard of_iommu_get_resv_regions()
> > implementation to obtain the reserved memory regions associated with a
> > device.
> 
> This covers the window between iommu_probe_device() setting up a default
> domain and the device's driver finally probing and taking control, but
> iommu_probe_device() represents the point that the IOMMU driver first knows
> about this device - there's still a window from whenever the IOMMU driver
> itself probed up to here where the "unidentified" traffic may have already
> been disrupted. Some IOMMU drivers have no option but to make the necessary
> configuration during their own probe routine, at which point a struct device
> for the display/etc. endpoint may not even exist yet.

Yeah, I think I'm actually running into this issue with the ARM SMMU
driver. The above works fine with the Tegra SMMU driver, though, because
it doesn't touch the SMMU configuration until a device is attached to a
domain.

For anything earlier than iommu_probe_device(), I don't see a way of
doing this generically. I've been working on a prototype to make these
reserved memory regions early on for ARM SMMU but I've been failing so
far. I think it would possibly work if we just switched the default for
stream IDs to be "bypass" if they have any devices that have reserved
memory regions, but again, this isn't quite working (yet).

Thierry

> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Frank Rowand <frowand.list@gmail.com>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >   drivers/iommu/dma-iommu.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index de68b4a02aea..31d48e55ab55 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -19,6 +19,7 @@
> >   #include <linux/iova.h>
> >   #include <linux/irq.h>
> >   #include <linux/mm.h>
> > +#include <linux/of_iommu.h>
> >   #include <linux/pci.h>
> >   #include <linux/scatterlist.h>
> >   #include <linux/vmalloc.h>
> > @@ -164,6 +165,8 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
> >   	if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
> >   		iort_iommu_msi_get_resv_regions(dev, list);
> > +	if (dev->of_node)
> > +		of_iommu_get_resv_regions(dev, list);
> >   }
> >   EXPORT_SYMBOL(iommu_dma_get_resv_regions);
> > 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions()
  2019-09-02 13:54   ` Robin Murphy
@ 2019-09-02 15:00     ` Thierry Reding
  2019-09-09 14:04       ` Will Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2019-09-02 15:00 UTC (permalink / raw)
  To: Robin Murphy
  Cc: devicetree, Frank Rowand, linux-kernel, iommu, Rob Herring, Will Deacon

[-- Attachment #1.1: Type: text/plain, Size: 3374 bytes --]

On Mon, Sep 02, 2019 at 02:54:23PM +0100, Robin Murphy wrote:
> On 29/08/2019 12:14, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > This is an implementation that IOMMU drivers can use to obtain reserved
> > memory regions from a device tree node. It uses the reserved-memory DT
> > bindings to find the regions associated with a given device. These
> > regions will be used to create 1:1 mappings in the IOMMU domain that
> > the devices will be attached to.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Frank Rowand <frowand.list@gmail.com>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >   drivers/iommu/of_iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
> >   include/linux/of_iommu.h |  8 ++++++++
> >   2 files changed, 47 insertions(+)
> > 
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 614a93aa5305..0d47f626b854 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -9,6 +9,7 @@
> >   #include <linux/iommu.h>
> >   #include <linux/limits.h>
> >   #include <linux/of.h>
> > +#include <linux/of_address.h>
> >   #include <linux/of_iommu.h>
> >   #include <linux/of_pci.h>
> >   #include <linux/slab.h>
> > @@ -225,3 +226,41 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> >   	return ops;
> >   }
> > +
> > +/**
> > + * of_iommu_get_resv_regions - reserved region driver helper for device tree
> > + * @dev: device for which to get reserved regions
> > + * @list: reserved region list
> > + *
> > + * IOMMU drivers can use this to implement their .get_resv_regions() callback
> > + * for memory regions attached to a device tree node. See the reserved-memory
> > + * device tree bindings on how to use these:
> > + *
> > + *   Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > + */
> > +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
> > +{
> > +	struct of_phandle_iterator it;
> > +	int err;
> > +
> > +	of_for_each_phandle(&it, err, dev->of_node, "memory-region", NULL, 0) {
> > +		struct iommu_resv_region *region;
> > +		struct resource res;
> > +
> > +		err = of_address_to_resource(it.node, 0, &res);
> > +		if (err < 0) {
> > +			dev_err(dev, "failed to parse memory region %pOF: %d\n",
> > +				it.node, err);
> > +			continue;
> > +		}
> 
> What if the device node has memory regions for other purposes, like private
> CMA carveouts? We wouldn't want to force mappings of those (and in the very
> worst case doing so could even render them unusable).

I suppose we could come up with additional properties to mark such
memory regions and skip them here.

One other alternative might be to make sure that the driver claims
the memory regions that have been mapped and then tells the IOMMU to
undo the mappings for them. That way the driver could set up the new
mappings, reprogram the hardware and then have the old mappings torn
down. I'm not sure that could always be done in a race-free way. For
example, what if the new mappings need to be in a region, such as a
private CMA carveout, that's already mapped. Can we temporarily map
one physical address to two DMA addresses?

The details here probably depend on the IOMMU hardware.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions()
  2019-09-02 15:00     ` Thierry Reding
@ 2019-09-09 14:04       ` Will Deacon
  0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2019-09-09 14:04 UTC (permalink / raw)
  To: Thierry Reding
  Cc: devicetree, Robin Murphy, linux-kernel, iommu, Rob Herring, Frank Rowand

On Mon, Sep 02, 2019 at 05:00:56PM +0200, Thierry Reding wrote:
> On Mon, Sep 02, 2019 at 02:54:23PM +0100, Robin Murphy wrote:
> > On 29/08/2019 12:14, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > > 
> > > This is an implementation that IOMMU drivers can use to obtain reserved
> > > memory regions from a device tree node. It uses the reserved-memory DT
> > > bindings to find the regions associated with a given device. These
> > > regions will be used to create 1:1 mappings in the IOMMU domain that
> > > the devices will be attached to.
> > > 
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Frank Rowand <frowand.list@gmail.com>
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >   drivers/iommu/of_iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
> > >   include/linux/of_iommu.h |  8 ++++++++
> > >   2 files changed, 47 insertions(+)
> > > 
> > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > > index 614a93aa5305..0d47f626b854 100644
> > > --- a/drivers/iommu/of_iommu.c
> > > +++ b/drivers/iommu/of_iommu.c
> > > @@ -9,6 +9,7 @@
> > >   #include <linux/iommu.h>
> > >   #include <linux/limits.h>
> > >   #include <linux/of.h>
> > > +#include <linux/of_address.h>
> > >   #include <linux/of_iommu.h>
> > >   #include <linux/of_pci.h>
> > >   #include <linux/slab.h>
> > > @@ -225,3 +226,41 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> > >   	return ops;
> > >   }
> > > +
> > > +/**
> > > + * of_iommu_get_resv_regions - reserved region driver helper for device tree
> > > + * @dev: device for which to get reserved regions
> > > + * @list: reserved region list
> > > + *
> > > + * IOMMU drivers can use this to implement their .get_resv_regions() callback
> > > + * for memory regions attached to a device tree node. See the reserved-memory
> > > + * device tree bindings on how to use these:
> > > + *
> > > + *   Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > > + */
> > > +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
> > > +{
> > > +	struct of_phandle_iterator it;
> > > +	int err;
> > > +
> > > +	of_for_each_phandle(&it, err, dev->of_node, "memory-region", NULL, 0) {
> > > +		struct iommu_resv_region *region;
> > > +		struct resource res;
> > > +
> > > +		err = of_address_to_resource(it.node, 0, &res);
> > > +		if (err < 0) {
> > > +			dev_err(dev, "failed to parse memory region %pOF: %d\n",
> > > +				it.node, err);
> > > +			continue;
> > > +		}
> > 
> > What if the device node has memory regions for other purposes, like private
> > CMA carveouts? We wouldn't want to force mappings of those (and in the very
> > worst case doing so could even render them unusable).
> 
> I suppose we could come up with additional properties to mark such
> memory regions and skip them here.

I think we need /something/ like that, both so that we can identify these
memory regions as requiring an identity mapping in the SMMU but also so
that we can place additional requirements on them, such as being 64k-aligned
and mandating properties of the mapping, such as cacheability based on the
device coherency.

I defer to the devicetree folks as to whether this should be an additional
property, or a phandle or whatever.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions()
  2019-09-02 14:52     ` Thierry Reding
@ 2019-09-17 17:59       ` Will Deacon
  0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2019-09-17 17:59 UTC (permalink / raw)
  To: Thierry Reding
  Cc: devicetree, Robin Murphy, linux-kernel, iommu, Rob Herring, Frank Rowand

On Mon, Sep 02, 2019 at 04:52:45PM +0200, Thierry Reding wrote:
> On Mon, Sep 02, 2019 at 03:22:35PM +0100, Robin Murphy wrote:
> > On 29/08/2019 12:14, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > > 
> > > For device tree nodes, use the standard of_iommu_get_resv_regions()
> > > implementation to obtain the reserved memory regions associated with a
> > > device.
> > 
> > This covers the window between iommu_probe_device() setting up a default
> > domain and the device's driver finally probing and taking control, but
> > iommu_probe_device() represents the point that the IOMMU driver first knows
> > about this device - there's still a window from whenever the IOMMU driver
> > itself probed up to here where the "unidentified" traffic may have already
> > been disrupted. Some IOMMU drivers have no option but to make the necessary
> > configuration during their own probe routine, at which point a struct device
> > for the display/etc. endpoint may not even exist yet.
> 
> Yeah, I think I'm actually running into this issue with the ARM SMMU
> driver. The above works fine with the Tegra SMMU driver, though, because
> it doesn't touch the SMMU configuration until a device is attached to a
> domain.
> 
> For anything earlier than iommu_probe_device(), I don't see a way of
> doing this generically. I've been working on a prototype to make these
> reserved memory regions early on for ARM SMMU but I've been failing so
> far. I think it would possibly work if we just switched the default for
> stream IDs to be "bypass" if they have any devices that have reserved
> memory regions, but again, this isn't quite working (yet).

I think we should avoid the use of "bypass" outside of the IOMMU probe()
routine if at all possible, since it leaves the thing wide open if we don't
subsequently probe the master.

Why can't we initialise a page-table early for StreamIDs with these
reserved regions, and then join that up later on if we see a device with
one of those StreamIDs attaching to a DMA domain? I suppose the nasty
case would be attaching to a domain that already has other masters
attached to it. Can we forbid that somehow for these devices? Otherwise,
I think we'd have to transiently switch to bypass whilst switching page
table.

What problems did you run into trying to implement this?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 11:14 [PATCH 0/2] iommu: Support reserved-memory regions Thierry Reding
2019-08-29 11:14 ` [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions() Thierry Reding
2019-09-02 13:38   ` Rob Herring
2019-09-02 13:54   ` Robin Murphy
2019-09-02 15:00     ` Thierry Reding
2019-09-09 14:04       ` Will Deacon
2019-08-29 11:14 ` [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions() Thierry Reding
2019-09-02 14:22   ` Robin Murphy
2019-09-02 14:52     ` Thierry Reding
2019-09-17 17:59       ` Will Deacon

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org iommu@archiver.kernel.org
	public-inbox-index linux-iommu


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/ public-inbox