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

From: Robin Murphy <robin.murphy@arm.com>
To: Thierry Reding <thierry.reding@gmail.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:15:25 +0100	[thread overview]
Message-ID: <ee285d4f-e5ff-e043-35cd-1338b1bb238f@arm.com> (raw)
In-Reply-To: <20190916155724.GG7488@ulmo>

On 16/09/2019 16:57, Thierry Reding wrote:
> 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.

Right, "domain && domain->type != IOMMU_DOMAIN_IDENTITY" should be 
sufficient to answer "is the DMA layer managing my address space for 
me?" unless and until some massive API change happens (which I certainly 
don't foresee).

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