Re: [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions()

[Thierry Reding]

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@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> > Cc: Frank Rowand <frowand.list@xxxxxxxxx>
> > Cc: devicetree@xxxxxxxxxxxxxxx
> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> >   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: signature.asc
Description: PGP signature