ACK: [PATCH 1/1][SRU][Cosmic][Bionic] driver core: Postpone DMA tear-down until after devres release

[Kleber Souza]

On 5/3/19 6:03 PM, dann frazier wrote:
> From: Geert Uytterhoeven <geert+renesas at glider.be>
> 
> BugLink: https://bugs.launchpad.net/bugs/1827437
> 
> When unbinding the (IOMMU-enabled) R-Car SATA device on Salvator-XS
> (R-Car H3 ES2.0), in preparation of rebinding against vfio-platform for
> device pass-through for virtualization:
> 
>     echo ee300000.sata > /sys/bus/platform/drivers/sata_rcar/unbind
> 
> the kernel crashes with:
> 
>     Unable to handle kernel paging request at virtual address ffffffbf029ffffc
>     Mem abort info:
>       ESR = 0x96000006
>       Exception class = DABT (current EL), IL = 32 bits
>       SET = 0, FnV = 0
>       EA = 0, S1PTW = 0
>     Data abort info:
>       ISV = 0, ISS = 0x00000006
>       CM = 0, WnR = 0
>     swapper pgtable: 4k pages, 39-bit VAs, pgdp = 000000007e8c586c
>     [ffffffbf029ffffc] pgd=000000073bfc6003, pud=000000073bfc6003, pmd=0000000000000000
>     Internal error: Oops: 96000006 [#1] SMP
>     Modules linked in:
>     CPU: 0 PID: 1098 Comm: bash Not tainted 5.0.0-rc5-salvator-x-00452-g37596f884f4318ef #287
>     Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ (DT)
>     pstate: 60400005 (nZCv daif +PAN -UAO)
>     pc : __free_pages+0x8/0x58
>     lr : __dma_direct_free_pages+0x50/0x5c
>     sp : ffffff801268baa0
>     x29: ffffff801268baa0 x28: 0000000000000000
>     x27: ffffffc6f9c60bf0 x26: ffffffc6f9c60bf0
>     x25: ffffffc6f9c60810 x24: 0000000000000000
>     x23: 00000000fffff000 x22: ffffff8012145000
>     x21: 0000000000000800 x20: ffffffbf029fffc8
>     x19: 0000000000000000 x18: ffffffc6f86c42c8
>     x17: 0000000000000000 x16: 0000000000000070
>     x15: 0000000000000003 x14: 0000000000000000
>     x13: ffffff801103d7f8 x12: 0000000000000028
>     x11: ffffff8011117604 x10: 0000000000009ad8
>     x9 : ffffff80110126d0 x8 : ffffffc6f7563000
>     x7 : 6b6b6b6b6b6b6b6b x6 : 0000000000000018
>     x5 : ffffff8011cf3cc8 x4 : 0000000000004000
>     x3 : 0000000000080000 x2 : 0000000000000001
>     x1 : 0000000000000000 x0 : ffffffbf029fffc8
>     Process bash (pid: 1098, stack limit = 0x00000000c38e3e32)
>     Call trace:
>      __free_pages+0x8/0x58
>      __dma_direct_free_pages+0x50/0x5c
>      arch_dma_free+0x1c/0x98
>      dma_direct_free+0x14/0x24
>      dma_free_attrs+0x9c/0xdc
>      dmam_release+0x18/0x20
>      release_nodes+0x25c/0x28c
>      devres_release_all+0x48/0x4c
>      device_release_driver_internal+0x184/0x1f0
>      device_release_driver+0x14/0x1c
>      unbind_store+0x70/0xb8
>      drv_attr_store+0x24/0x34
>      sysfs_kf_write+0x4c/0x64
>      kernfs_fop_write+0x154/0x1c4
>      __vfs_write+0x34/0x164
>      vfs_write+0xb4/0x16c
>      ksys_write+0x5c/0xbc
>      __arm64_sys_write+0x14/0x1c
>      el0_svc_common+0x98/0x114
>      el0_svc_handler+0x1c/0x24
>      el0_svc+0x8/0xc
>     Code: d51b4234 17fffffa a9bf7bfd 910003fd (b9403404)
>     ---[ end trace 8c564cdd3a1a840f ]---
> 
> While I've bisected this to commit e8e683ae9a736407 ("iommu/of: Fix
> probe-deferral"), and reverting that commit on post-v5.0-rc4 kernels
> does fix the problem, this turned out to be a red herring.
> 
> On arm64, arch_teardown_dma_ops() resets dev->dma_ops to NULL.
> Hence if a driver has used a managed DMA allocation API, the allocated
> DMA memory will be freed using the direct DMA ops, while it may have
> been allocated using a custom DMA ops (iommu_dma_ops in this case).
> 
> Fix this by reversing the order of the calls to devres_release_all() and
> arch_teardown_dma_ops().
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas at glider.be>
> Acked-by: Christoph Hellwig <hch at lst.de>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> Cc: stable <stable at vger.kernel.org>
> Reviewed-by: Robin Murphy <robin.murphy at arm.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> (backported from commit 376991db4b6464e906d699ef07681e2ffa8ab08c)
> [dannf: Based on 4.19.29 backport, with trivial offset fix]
> Signed-off-by: dann frazier <dann.frazier at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

> ---
>  drivers/base/dd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index a9e1865c3b16b..a206d04c4e33c 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -891,9 +891,9 @@ static void __device_release_driver(struct device *dev, struct device *parent)
>  			return;
>  
>  		device_links_driver_cleanup(dev);
> -		dma_deconfigure(dev);
>  
>  		devres_release_all(dev);
> +		dma_deconfigure(dev);
>  		dev->driver = NULL;
>  		dev_set_drvdata(dev, NULL);
>  		if (dev->pm_domain && dev->pm_domain->dismiss)
>