All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <treding@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	<iommu@lists.linux-foundation.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Christoph Hellwig <hch@lst.de>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Ben Skeggs <bskeggs@redhat.com>,
	Tobias Jakobi <tjakobi@math.uni-bielefeld.de>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH] Revert "ARM: dma-mapping: Set proper DMA ops in arm_iommu_detach_device()"
Date: Tue, 15 Jan 2019 16:29:11 +0100	[thread overview]
Message-ID: <20190115152910.GA12652@ulmo> (raw)
In-Reply-To: <4f7fd8db-33e4-9c8d-2fee-1ca9c98a9276@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

On Mon, Jan 14, 2019 at 04:38:20PM +0000, Robin Murphy wrote:
> On 14/01/2019 16:09, Thierry Reding wrote:
> > On Mon, Jan 14, 2019 at 02:22:40PM +0100, Marek Szyprowski wrote:
> > > This reverts commit 1874619a7df4b14b23b14877e705ae15325773e3.
> > > 
> > > That patch broke IOMMU support for devices, which fails to probe for the
> > > first time and use deferred probe approach. When non-NULL dma_ops is set
> > > in arm_iommu_detach_device(), the given device later ignored by
> > > arch_setup_dma_ops() and stays with non-IOMMU dma_ops.
> > > 
> > > Reported-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> > > Fixes: 1874619a7df4 "ARM: dma-mapping: Set proper DMA ops in arm_iommu_detach_device()"
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >   arch/arm/mm/dma-mapping.c | 12 ++++++------
> > >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > Can you point out exactly what drivers break because of this change? We
> > need to find a solution that works for everyone. Reverting is only
> > marginally useful because somebody will just end up wanting to revert
> > the revert because a different driver is now broken.
> 
> At first glance, it sounds like what we really want is for
> arch_teardown_iommu_ops() to completely clear any ops that
> arch_setup_dma_ops() installed - does the below suffice?
> 
> Robin.
> 
> ----->8-----
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index f1e2922e447c..1e3e08a1c456 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2390,4 +2390,6 @@ void arch_teardown_dma_ops(struct device *dev)
>  		return;
> 
>  	arm_teardown_iommu_dma_ops(dev);
> +	/* Let arch_setup_dma_ops() start again from scratch upon re-probe */
> +	set_dma_ops(dev, NULL);
>  }

Seems to work fine on Tegra:

Tested-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID
From: Thierry Reding <treding@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Christoph Hellwig <hch@lst.de>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Ben Skeggs <bskeggs@redhat.com>,
	Tobias Jakobi <tjakobi@math.uni-bielefeld.de>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH] Revert "ARM: dma-mapping: Set proper DMA ops in arm_iommu_detach_device()"
Date: Tue, 15 Jan 2019 16:29:11 +0100	[thread overview]
Message-ID: <20190115152910.GA12652@ulmo> (raw)
In-Reply-To: <4f7fd8db-33e4-9c8d-2fee-1ca9c98a9276@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

On Mon, Jan 14, 2019 at 04:38:20PM +0000, Robin Murphy wrote:
> On 14/01/2019 16:09, Thierry Reding wrote:
> > On Mon, Jan 14, 2019 at 02:22:40PM +0100, Marek Szyprowski wrote:
> > > This reverts commit 1874619a7df4b14b23b14877e705ae15325773e3.
> > > 
> > > That patch broke IOMMU support for devices, which fails to probe for the
> > > first time and use deferred probe approach. When non-NULL dma_ops is set
> > > in arm_iommu_detach_device(), the given device later ignored by
> > > arch_setup_dma_ops() and stays with non-IOMMU dma_ops.
> > > 
> > > Reported-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> > > Fixes: 1874619a7df4 "ARM: dma-mapping: Set proper DMA ops in arm_iommu_detach_device()"
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >   arch/arm/mm/dma-mapping.c | 12 ++++++------
> > >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > Can you point out exactly what drivers break because of this change? We
> > need to find a solution that works for everyone. Reverting is only
> > marginally useful because somebody will just end up wanting to revert
> > the revert because a different driver is now broken.
> 
> At first glance, it sounds like what we really want is for
> arch_teardown_iommu_ops() to completely clear any ops that
> arch_setup_dma_ops() installed - does the below suffice?
> 
> Robin.
> 
> ----->8-----
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index f1e2922e447c..1e3e08a1c456 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2390,4 +2390,6 @@ void arch_teardown_dma_ops(struct device *dev)
>  		return;
> 
>  	arm_teardown_iommu_dma_ops(dev);
> +	/* Let arch_setup_dma_ops() start again from scratch upon re-probe */
> +	set_dma_ops(dev, NULL);
>  }

Seems to work fine on Tegra:

Tested-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID
From: Thierry Reding <treding@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	linux-kernel@vger.kernel.org,
	Tobias Jakobi <tjakobi@math.uni-bielefeld.de>,
	iommu@lists.linux-foundation.org, Ben Skeggs <bskeggs@redhat.com>,
	Christoph Hellwig <hch@lst.de>,
	linux-arm-kernel@lists.infradead.org,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH] Revert "ARM: dma-mapping: Set proper DMA ops in arm_iommu_detach_device()"
Date: Tue, 15 Jan 2019 16:29:11 +0100	[thread overview]
Message-ID: <20190115152910.GA12652@ulmo> (raw)
In-Reply-To: <4f7fd8db-33e4-9c8d-2fee-1ca9c98a9276@arm.com>


[-- Attachment #1.1: Type: text/plain, Size: 1919 bytes --]

On Mon, Jan 14, 2019 at 04:38:20PM +0000, Robin Murphy wrote:
> On 14/01/2019 16:09, Thierry Reding wrote:
> > On Mon, Jan 14, 2019 at 02:22:40PM +0100, Marek Szyprowski wrote:
> > > This reverts commit 1874619a7df4b14b23b14877e705ae15325773e3.
> > > 
> > > That patch broke IOMMU support for devices, which fails to probe for the
> > > first time and use deferred probe approach. When non-NULL dma_ops is set
> > > in arm_iommu_detach_device(), the given device later ignored by
> > > arch_setup_dma_ops() and stays with non-IOMMU dma_ops.
> > > 
> > > Reported-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> > > Fixes: 1874619a7df4 "ARM: dma-mapping: Set proper DMA ops in arm_iommu_detach_device()"
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >   arch/arm/mm/dma-mapping.c | 12 ++++++------
> > >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > Can you point out exactly what drivers break because of this change? We
> > need to find a solution that works for everyone. Reverting is only
> > marginally useful because somebody will just end up wanting to revert
> > the revert because a different driver is now broken.
> 
> At first glance, it sounds like what we really want is for
> arch_teardown_iommu_ops() to completely clear any ops that
> arch_setup_dma_ops() installed - does the below suffice?
> 
> Robin.
> 
> ----->8-----
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index f1e2922e447c..1e3e08a1c456 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2390,4 +2390,6 @@ void arch_teardown_dma_ops(struct device *dev)
>  		return;
> 
>  	arm_teardown_iommu_dma_ops(dev);
> +	/* Let arch_setup_dma_ops() start again from scratch upon re-probe */
> +	set_dma_ops(dev, NULL);
>  }

Seems to work fine on Tegra:

Tested-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2019-01-15 15:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20190114132250eucas1p2abdf2f36bad3554e37dfbf40e539f594@eucas1p2.samsung.com>
2019-01-14 13:22 ` [PATCH] Revert "ARM: dma-mapping: Set proper DMA ops in arm_iommu_detach_device()" Marek Szyprowski
2019-01-14 13:22   ` Marek Szyprowski
2019-01-14 16:09   ` Thierry Reding
2019-01-14 16:09     ` Thierry Reding
2019-01-14 16:09     ` Thierry Reding
2019-01-14 16:38     ` Robin Murphy
2019-01-14 16:38       ` Robin Murphy
2019-01-14 20:20       ` Tobias Jakobi
2019-01-14 20:20         ` Tobias Jakobi
2019-01-15 15:29       ` Thierry Reding [this message]
2019-01-15 15:29         ` Thierry Reding
2019-01-15 15:29         ` Thierry Reding
2019-01-16 10:36       ` Marek Szyprowski
2019-01-16 10:36         ` Marek Szyprowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190115152910.GA12652@ulmo \
    --to=treding@nvidia.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=bskeggs@redhat.com \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=robin.murphy@arm.com \
    --cc=tjakobi@math.uni-bielefeld.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.