Hi Will,
On Sunday 14 December 2014 17:49:34 Laurent Pinchart wrote:
> On Wednesday 10 December 2014 15:08:53 Will Deacon wrote:
> > On Wed, Dec 10, 2014 at 02:52:56PM +0000, Rob Clark wrote:
> > > On Mon, Dec 1, 2014 at 11:57 AM, Will Deacon wrote:
> > > > This patch extends of_dma_configure so that it sets up the IOMMU for a
> > > > device, as well as the coherent/non-coherent DMA mapping ops.
> > > >
> > > > Acked-by: Arnd Bergmann <a...@arndb.de>
> > > > Acked-by: Marek Szyprowski <m.szyprow...@samsung.com>
> > > > Tested-by: Robin Murphy <robin.mur...@arm.com>
> > > > Signed-off-by: Will Deacon <will.dea...@arm.com>
> > > > ---
> > > >
> > > > arch/arm/include/asm/dma-mapping.h | 4 +++-
> > > > drivers/of/platform.c | 21 ++++++++++++++-------
> > > > include/linux/dma-mapping.h | 8 +++++++-
> > > > 3 files changed, 24 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/arch/arm/include/asm/dma-mapping.h
> > > > b/arch/arm/include/asm/dma-mapping.h index dc3420e77758..f3c0d953f6a2
> > > > 100644
> > > > --- a/arch/arm/include/asm/dma-mapping.h
> > > > +++ b/arch/arm/include/asm/dma-mapping.h
> > > > @@ -121,7 +121,9 @@ static inline unsigned long dma_max_pfn(struct
> > > > device *dev)
> > > > }
> > > > #define dma_max_pfn(dev) dma_max_pfn(dev)
> > > >
> > > > -static inline void arch_setup_dma_ops(struct device *dev, bool
> > > > coherent)
> > > > +static inline void arch_setup_dma_ops(struct device *dev, u64
> > > > dma_base,
> > > > + u64 size, struct iommu_ops
> > > > *iommu,
> > > > + bool coherent)
> > > > {
> > > > if (coherent)
> > > > set_dma_ops(dev, &arm_coherent_dma_ops);
> > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > > index ff1f4e9afccb..b89caf8c7586 100644
> > > > --- a/drivers/of/platform.c
> > > > +++ b/drivers/of/platform.c
> > > > @@ -19,6 +19,7 @@
> > > > #include <linux/slab.h>
> > > > #include <linux/of_address.h>
> > > > #include <linux/of_device.h>
> > > > +#include <linux/of_iommu.h>
> > > > #include <linux/of_irq.h>
> > > > #include <linux/of_platform.h>
> > > > #include <linux/platform_device.h>
> > > > @@ -166,6 +167,7 @@ static void of_dma_configure(struct device *dev)
> > > > int ret;
> > > > bool coherent;
> > > > unsigned long offset;
> > > > + struct iommu_ops *iommu;
> > > > /*
> > > > * Set default dma-mask to 32 bit. Drivers are expected to
> > > > setup
> > > > @@ -194,7 +196,16 @@ static void of_dma_configure(struct device *dev)
> > > > dev_dbg(dev, "device is%sdma coherent\n",
> > > > coherent ? " " : " not ");
> > > > - arch_setup_dma_ops(dev, coherent);
> > > > + iommu = of_iommu_configure(dev);
> > > > + dev_dbg(dev, "device is%sbehind an iommu\n",
> > > > + iommu ? " " : " not ");
> > > > +
> > > > + arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
> > >
> > > so, what is the way for a driver that explicitly wants to manage it's
> > > own device virtual address space to opt out of this? I suspect that
> > > won't be the common case, but for a gpu, if dma layer all of a sudden
> > > thinks it is in control of the gpu's virtual address space, things are
> > > going to end in tears..
> >
> > I think you'll need to detach from the DMA domain, then have the driver
> > manage everything itself. As you say, it's not the common case, so you
> > may need to add some hooks for detaching from the default domain and
> > swizzling your DMA ops.
>
> I'm wondering if it's such an exotic case after all. I can see two reasons
> not to use the default domain. In addition to special requirements coming
> from the bus master side, the IOMMU itself might not support one domain per
> bus master (I'm of course raising the issue from a very selfish Renesas
> IPMMU point of view).
>
> In that case the IOMMU driver can still detach the device from the default
> domain in the .of_xlate() callback and attach it to its private domain.
I actually spoke too fast, the .of_xlate() callback is called before
arch_setup_dma_ops(). The operation could be performed in the .add_device()
callback. I'm having trouble understanding how you envision .add_device,
.remove_device and .of_xlate working together in the DT case. Could you please
detail what you think each callback should be responsible of exactly ?
> I'm however wondering whether this case isn't common enough to avoid
> creating default domains in common code in the first place. IOMMU drivers
> that support one domain per master would then create device domain in their
> .of_xlate() callback.
--
Regards,
Laurent Pinchart
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
