[PATCH v6 8/8] arm: dma-mapping: plumb our iommu mapping ops into arch_setup_dma_ops

[Laurent Pinchart]

Hi Will,

On Tuesday 20 January 2015 15:19:11 Will Deacon wrote:
> On Tue, Jan 20, 2015 at 03:14:01PM +0000, Laurent Pinchart wrote:
> > On Monday 19 January 2015 13:31:00 Thierry Reding wrote:
> >> On Mon, Jan 19, 2015 at 01:34:24PM +0200, Laurent Pinchart wrote:
> >>> On Monday 19 January 2015 11:12:02 Will Deacon wrote:
> >>>> On Sun, Jan 18, 2015 at 11:18:51AM +0000, Laurent Pinchart wrote:
> >>>>> On Sunday 18 January 2015 15:54:34 Alexandre Courbot wrote:
> >>>>>> On 01/16/2015 08:18 AM, Laurent Pinchart wrote:
> >>>>>>> On Thursday 15 January 2015 11:12:17 Will Deacon wrote:
> >
> > [snip]
> > 
> >>>>>> I am arriving late in this discussion, but what is wrong with
> >>>>>> asking drivers to explicitly state that they want the DMA API to be
> >>>>>> backed by the IOMMU instead of forcibly making it work that way?
> >>>>> 
> >>>>> The vast majority of the drivers are not IOMMU-aware. We would thus
> >>>>> need to add a call at the beginning of the probe function of nearly
> >>>>> every driver that can perform DMA to state that the driver doesn't
> >>>>> need to handle any IOMMU that might be present in the system itself.
> >>>>> I don't think that's a better solution.
> >>>>> 
> >>>>> Explicitly tearing down mappings in drivers that want to manage
> >>>>> IOMMUs isn't a solution I like either. A possibly better solution
> >>>>> would be to call a function to state that the DMA mapping API
> >>>>> shouldn't not handle IOMMUs. Something like
> >>>>> 
> >>>>> 	dma_mapping_ignore_iommu(dev);
> >>>>> 
> >>>>> at the beginning of the probe function of such drivers could do. The
> >>>>> function would perform behind the scene all operations needed to
> >>>>> tear down everything that shouldn't have been set up.
> >>>> 
> >>>> An alternative would be to add a flag to platform_driver, like we
> >>>> have for "prevent_deferred_probe" which is something like
> >>>> "prevent_dma_configure".
> >>> 
> >>> That's a solution I have proposed (albeit as a struct device_driver
> >>> field, but that's a small detail), so I'm fine with it :-)
> >> 
> >> I think Marek had proposed something similar initially as well. I don't
> >> see an immediate downside to that solution. It's still somewhat ugly in
> >> that a lot of stuff is set up before it's known to actually be used at
> >> all, but it seems like there's some consensus that this can be improved
> >> later on, so I have no objections to such a patch.
> >> 
> >> Of course that doesn't solve the current breakage for the Rockchip DRM
> >> and OMAP ISP drivers.
> > 
> > And, as I came to realize after a long bisect yesternight, the Renesas
> > IPMMU driver :-/ Basically any platform that relied on
> > arm_iommu_attach_device() to set the IOMMU DMA ops is now broken.
> 
> We could restore the set_dma_ops call in arm_iommu_attach_device as a
> temporary hack (along with a big fat comment), since arch_setup_dma_ops
> actually sets the ops correct *after* the call to
> arm_get_iommu_dma_map_ops...
> 
> It doesn't provide any motivation for people to consider moving over to the
> new framework, but it fixes the current issues affecting mainline.

I'm all for incentives, but I think avoiding a major v3.19 regression would be 
good, too :-) I wanted to test your LPAE page table allocator yesterday with 
the Renesas IPMMU driver, and ended up spending the whole night bisecting the 
regression instead.

-- 
Regards,

Laurent Pinchart