Re: [PATCH 08/11] drm/nouveau: tegra: Skip IOMMU initialization if already attached

From: Thierry Reding <thierry.reding@gmail.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: linux-tegra@vger.kernel.org, nouveau@lists.freedesktop.org,
	Ben Skeggs <bskeggs@redhat.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 08/11] drm/nouveau: tegra: Skip IOMMU initialization if already attached
Date: Mon, 16 Sep 2019 17:57:24 +0200	[thread overview]
Message-ID: <20190916155724.GG7488@ulmo> (raw)
In-Reply-To: <156993ea-ece6-9eb5-b664-a55c631e2600@arm.com>

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

On Mon, Sep 16, 2019 at 04:29:18PM +0100, Robin Murphy wrote:
> Hi Thierry,
> 
> On 16/09/2019 16:04, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > If the GPU is already attached to an IOMMU, don't detach it and setup an
> > explicit IOMMU domain. Since Nouveau can now properly handle the case of
> > the DMA API being backed by an IOMMU, just continue using the DMA API.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >   .../drm/nouveau/nvkm/engine/device/tegra.c    | 19 +++++++------------
> >   1 file changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c
> > index d0d52c1d4aee..fc652aaa41c7 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c
> > @@ -23,10 +23,6 @@
> >   #ifdef CONFIG_NOUVEAU_PLATFORM_DRIVER
> >   #include "priv.h"
> > -#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> > -#include <asm/dma-iommu.h>
> > -#endif
> > -
> >   static int
> >   nvkm_device_tegra_power_up(struct nvkm_device_tegra *tdev)
> >   {
> > @@ -109,14 +105,13 @@ nvkm_device_tegra_probe_iommu(struct nvkm_device_tegra *tdev)
> >   	unsigned long pgsize_bitmap;
> >   	int ret;
> > -#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> > -	if (dev->archdata.mapping) {
> > -		struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
> > -
> > -		arm_iommu_detach_device(dev);
> > -		arm_iommu_release_mapping(mapping);
> > -	}
> > -#endif
> > +	/*
> > +	 * Skip explicit IOMMU initialization if the GPU is already attached
> > +	 * to an IOMMU domain. This can happen if the DMA API is backed by an
> > +	 * IOMMU.
> > +	 */
> > +	if (iommu_get_domain_for_dev(dev))
> > +		return;
> 
> Beware of "iommu.passthrough=1" - you could get a valid default domain here
> yet still have direct/SWIOTLB DMA ops. I guess you probably want to
> double-check the domain type as well.

Good point. An earlier version of this patch had an additional check for
IOMMU_DOMAIN_DMA, but then that failed on 32-bit ARM because there the
DMA API can also use IOMMU_DOMAIN_UNMANAGED type domains. Checking for
IOMMU_DOMAIN_IDENTIFY should be safe, though. That doesn't seem to
appear in arch/arm, arch/arm64 or drivers/iommu/dma-iommu.c.

Thierry

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

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel