Hi Will, Alexandre, Daniel,

Am Mittwoch, 14. Januar 2015, 10:46:10 schrieb Will Deacon:
> Hi Alex,
> 
> On Wed, Jan 14, 2015 at 09:00:24AM +0000, Alexandre Courbot wrote:
> > On 12/02/2014 01:57 AM, Will Deacon wrote:
> > > This patch plumbs the existing ARM IOMMU DMA infrastructure (which isn't
> > > actually called outside of a few drivers) into arch_setup_dma_ops, so
> > > that we can use IOMMUs for DMA transfers in a more generic fashion.
> > > 
> > > Since this significantly complicates the arch_setup_dma_ops function,
> > > it is moved out of line into dma-mapping.c. If CONFIG_ARM_DMA_USE_IOMMU
> > > is not set, the iommu parameter is ignored and the normal ops are used
> > > instead.
> > 
> > A series for IOMMU support with Tegra/Nouveau ceased to work after this
> > patch.
> 
> Which series? This code shouldn't even be executed unless you start using
> of_xlate and the generic bindings.
> 
> > The Tegra IOMMU is not registered by the time the DT is parsed,
> > and thus all devices end up without the proper DMA ops set up because
> > the phandle to the IOMMU cannot be resolved.
> 
> You might want to look at the patches posted for the exynos, renesas and ARM
> SMMUs for some hints in how to use the new API.
> 
> > Subsequently calling arm_iommu_create_mapping() and
> > arm_iommu_attach_device() from the driver (as I used to do until 3.18)
> > does not help since the call to set_dma_ops() has been moved out of
> > arm_iommu_attach_device(). Therefore there seems to be no way for a device
> > to gets its correct DMA ops unless the IOMMU is ready by the time the DT
> > is parsed.
> > 
> > Also potentially affected by this are the Rockchip DRM and OMAP3 ISP
> > drivers, which follow the same pattern.
> 
> I don't understand why any code currently in mainline should be affected.
> Please can you elaborate on the failure case?

As Alexandre suspected the new Rockchip drm code seems to be affected by
this. I hadn't played with the drm code before last weekend and was then
stumbling over different iommu related issues. As I hadn't to much contact
with iommus till now I didn't get very far.

But with Alexandre's bandaid patch of adding
	set_dma_ops(dev, &iommu_ops);
to arm_iommu_attach_device both problems go away.


So to elaborate on the two failure cases:

When attaching either hdmi or vga connectors at runtime, I get

[drm:drm_mode_debug_printmodeline] Modeline 26:"1024x768" 85 94500 1024 1072 1168 1376 768 769 772 808 0x40 0x5
[drm:drm_crtc_helper_set_mode] [CRTC:14]
[drm:vop_crtc_dpms] crtc[14] mode[0]
rockchip-vop ff930000.vop: Attached to iommu domain
[drm] processing encoder TMDS-18
[drm] processing encoder DAC-22
[drm] processing connector HDMI-A-1
[drm] processing connector VGA-1
[drm:vop_win_update] [PLANE:12] [FB:-1->24] update
rk_iommu ff930300.iommu: Page fault at 0x2d400500 of type read
rk_iommu ff930300.iommu: iova = 0x2d400500: dte_index: 0xb5 pte_index: 0x0 page_offset: 0x500
rk_iommu ff930300.iommu: mmu_dte_addr: 0x2e3b3000 dte@0x2e3b32d4: 0x000000 valid: 0 pte@0x00000000: 0x000000 valid: 0 page@0x00000000 flags: 0x0
[drm:drm_crtc_helper_set_mode] [ENCODER:22:DAC-22] set [MODE:26:1024x768]

===============
When my wip vga-connector is plugged in at boot, I get


[drm:drm_target_preferred] found mode 1280x1024
[drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
[drm:drm_setup_crtcs] desired mode 1280x1024 set on crtc 14 (0,0)
------------[ cut here ]------------
WARNING: CPU: 1 PID: 33 at mm/page_alloc.c:2645 __alloc_pages_nodemask+0x18c/0x6a8()
Modules linked in:
CPU: 1 PID: 33 Comm: kworker/u8:1 Not tainted 3.19.0-rc1+ #1512
Hardware name: Rockchip Cortex-A9 (Device Tree)
Workqueue: deferwq deferred_probe_work_func
[<c00148e4>] (unwind_backtrace) from [<c00111e0>] (show_stack+0x10/0x14)
[<c00111e0>] (show_stack) from [<c0426564>] (dump_stack+0x6c/0x84)
[<c0426564>] (dump_stack) from [<c0021f34>] (warn_slowpath_common+0x80/0xac)
[<c0021f34>] (warn_slowpath_common) from [<c0021f78>] (warn_slowpath_null+0x18/0x1c)
[<c0021f78>] (warn_slowpath_null) from [<c008b114>] (__alloc_pages_nodemask+0x18c/0x6a8)
[<c008b114>] (__alloc_pages_nodemask) from [<c001a274>] (__dma_alloc_buffer.isra.18+0x2c/0x80)
[<c001a274>] (__dma_alloc_buffer.isra.18) from [<c001a2dc>] (__alloc_remap_buffer.isra.22+0x14/0x5c)
[<c001a2dc>] (__alloc_remap_buffer.isra.22) from [<c001a490>] (__dma_alloc+0x16c/0x1d8)
[<c001a490>] (__dma_alloc) from [<c001a614>] (arm_dma_alloc+0x84/0x90)
[<c001a614>] (arm_dma_alloc) from [<c022f610>] (rockchip_gem_create_object+0x8c/0xc4)
[<c022f610>] (rockchip_gem_create_object) from [<c022f158>] (rockchip_drm_fbdev_create+0x6c/0x1ec)
[<c022f158>] (rockchip_drm_fbdev_create) from [<c021499c>] (drm_fb_helper_initial_config+0x230/0x328)
[<c021499c>] (drm_fb_helper_initial_config) from [<c022f388>] (rockchip_drm_fbdev_init+0xa4/0xc0)
[<c022f388>] (rockchip_drm_fbdev_init) from [<c022ea18>] (rockchip_drm_load+0x1b8/0x1f4)
[<c022ea18>] (rockchip_drm_load) from [<c021c218>] (drm_dev_register+0x80/0x100)
[<c021c218>] (drm_dev_register) from [<c022e77c>] (rockchip_drm_bind+0x48/0x74)
[<c022e77c>] (rockchip_drm_bind) from [<c0234a58>] (try_to_bring_up_master.part.2+0xa4/0xf4)
[<c0234a58>] (try_to_bring_up_master.part.2) from [<c0234c58>] (component_add+0x9c/0x104)
[<c0234c58>] (component_add) from [<c023a1e4>] (platform_drv_probe+0x48/0x90)
[<c023a1e4>] (platform_drv_probe) from [<c0238c30>] (driver_probe_device+0x130/0x340)
[<c0238c30>] (driver_probe_device) from [<c0237270>] (bus_for_each_drv+0x70/0x84)
[<c0237270>] (bus_for_each_drv) from [<c0238a8c>] (device_attach+0x64/0x88)
[<c0238a8c>] (device_attach) from [<c0238090>] (bus_probe_device+0x28/0x98)
[<c0238090>] (bus_probe_device) from [<c0238548>] (deferred_probe_work_func+0x78/0xa4)
[<c0238548>] (deferred_probe_work_func) from [<c0034008>] (process_one_work+0x1c8/0x2f4)
[<c0034008>] (process_one_work) from [<c0034448>] (worker_thread+0x2e8/0x450)
[<c0034448>] (worker_thread) from [<c00380a8>] (kthread+0xdc/0xf0)
[<c00380a8>] (kthread) from [<c000e8b8>] (ret_from_fork+0x14/0x3c)
---[ end trace ce23b3730f8c0d32 ]---
[drm:rockchip_gem_create_object] *ERROR* failed to allocate 0x500000 byte dma buffer


where Daniel Kurtz already deducted in private:

"But, more importantly, this call stack has "arm_dma_alloc", which
suggests that are not actually using the iommu dma allocators.
The allocation should have been handled by arm_iommu_alloc_attrs(),
but for some reason, it is not.
The iommu allocator should have been installed as the allocator for
the drm device by the call to arm_iommu_attach_device() in
rockchip_drm_load."


> > This raises the following questions:
> > 
> > 1) Why are arm_iommu_create_mapping() and arm_iommu_attach_device()
> > still public since they cannot set the DMA ops and thus seem to be
> > useless outside of arch_setup_dma_ops()?
> 
> It has callers outside of the file. I'd like to make it static, but that
> means doing some non-trivial porting of all the callers, which I'm also
> unable to test.
> 
> > 2) Say you want to use the IOMMU API in your driver, and have an iommu
> > property in your device's DT node. If by chance your IOMMU is registered
> > early, you will already have a mapping automatically created even before
> > your probe function is called. Can this be avoided? Is it even safe?
> 
> Currently, I think you have to either teardown the ops manually or return
> an error from of_xlate. Thierry was also looking at this sort of thing,
> so it might be worth talking to him.
> 
> Will