drm/tegra: Fix order of teardown in IOMMU case
PATCH 1/2 drm/tegra: Fix order of teardown in IOMMU case
From: Thierry Reding <[email protected]>
The original code works fine, this is merely a cosmetic change to make the teardown order the reverse of the setup order.
--- drivers/gpu/drm/tegra/drm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/tegra/drm.c +4 -4
@@ -207,10 +207,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)drm_mode_config_cleanup (drm);if (tegra->domain) { -iommu_domain_free (tegra->domain); -drm_mm_takedown (&tegra->mm);mutex_destroy (&tegra->mm_lock); +drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iommu_domain_free (tegra->domain); } free:kfree (tegra);
@@ -233,10 +233,10 @@ static void tegra_drm_unload(struct drm_device *drm)return ;if (tegra->domain) { -iommu_domain_free (tegra->domain); -drm_mm_takedown (&tegra->mm);mutex_destroy (&tegra->mm_lock); +drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iommu_domain_free (tegra->domain); }kfree (tegra);
Signature · 6 lines
2.17.0 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 2/2 drm/tegra: Acquire a reference to the IOVA cache
From: Thierry Reding <[email protected]>
The IOVA API uses a memory cache to allocate IOVA nodes from. To make sure that this cache is available, obtain a reference to it and release the reference when the cache is no longer needed.
On 64-bit ARM this is hidden by the fact that the DMA mapping API gets that reference and never releases it. On 32-bit ARM, however, the DMA mapping API doesn't do that, so allocation of IOVA nodes fails.
--- drivers/gpu/drm/tegra/drm.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/gpu/drm/tegra/drm.c +7 -1
@@ -115,6 +115,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags) goto free; } + err =iova_cache_get (); +if (err <0 ) + goto domain; + geometry = &tegra->domain->geometry; gem_start = geometry->aperture_start; gem_end = geometry->aperture_end -CARVEOUT_SZ ;
@@ -205,11 +209,12 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)tegra_drm_fb_free (drm); config:drm_mode_config_cleanup (drm); - +domain:if (tegra->domain) {mutex_destroy (&tegra->mm_lock);drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iova_cache_put ();iommu_domain_free (tegra->domain); } free:
@@ -236,6 +241,7 @@ static void tegra_drm_unload(struct drm_device *drm)mutex_destroy (&tegra->mm_lock);drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iova_cache_put ();iommu_domain_free (tegra->domain); }
Signature · 6 lines
2.17.0 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/tegra: Acquire a reference to the IOVA cache
Quoted 6 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The IOVA API uses a memory cache to allocate IOVA nodes from. To make sure that this cache is available, obtain a reference to it and release the reference when the cache is no longer needed.
Quoted 6 lines
On 64-bit ARM this is hidden by the fact that the DMA mapping API gets that reference and never releases it. On 32-bit ARM, however, the DMA mapping API doesn't do that, so allocation of IOVA nodes fails.
Signed-off-by Thierry Reding <[email protected]>---
CONFIG_TEGRA_IOMMU_SMMU is enabled in the default kernel configs and hence DRM should fail to probe on t124 since 4.11. What about to add stable tag for v4.11+ here to unbreak stable kernels as well?
Quoted 8 lines
drivers/gpu/drm/tegra/drm.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)drivers/gpu/drm/tegra/drm.c +0
-0
drivers/gpu/drm/tegra/drm.c static int tegra_drm_load(struct drm_device *drm, unsigned long flags) · line 121
Quoted 7 lines
drivers/gpu/drm/tegra/drm.c +4
-0@@ -115,6 +115,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags) goto free; } + err =iova_cache_get (); +if (err <0 ) + goto domain; +
Host1x also uses the alloc_iova(), though this allocation is actually invoked by the DRM driver on requesting a channel. It is fine right now because DRM driver is the only host1x user, but what about to add iova_cache_get() to the host1x driver as well for consistency?
Quoted 19 lines
drivers/gpu/drm/tegra/drm.c +2
-1geometry = &tegra->domain->geometry; gem_start = geometry->aperture_start; gem_end = geometry->aperture_end -CARVEOUT_SZ ;@@ -205,11 +209,12 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)tegra_drm_fb_free (drm); config:drm_mode_config_cleanup (drm); - +domain:if (tegra->domain) {mutex_destroy (&tegra->mm_lock);drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iova_cache_put ();iommu_domain_free (tegra->domain); } free:
drivers/gpu/drm/tegra/drm.c static void tegra_drm_unload(struct drm_device *drm) · line 247
Quoted 7 lines
drivers/gpu/drm/tegra/drm.c +1
-0@@ -236,6 +241,7 @@ static void tegra_drm_unload(struct drm_device *drm)mutex_destroy (&tegra->mm_lock);drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iova_cache_put ();iommu_domain_free (tegra->domain); }
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/tegra: Acquire a reference to the IOVA cache
Quoted 14 lines · Dmitry Osipenko
Quoted 6 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The IOVA API uses a memory cache to allocate IOVA nodes from. To make sure that this cache is available, obtain a reference to it and release the reference when the cache is no longer needed.
Quoted 6 lines
On 64-bit ARM this is hidden by the fact that the DMA mapping API gets that reference and never releases it. On 32-bit ARM, however, the DMA mapping API doesn't do that, so allocation of IOVA nodes fails.
Signed-off-by Thierry Reding <[email protected]>---
Quoted 6 lines
Reviewed-by Dmitry Osipenko <[email protected]>Tested-by Dmitry Osipenko <[email protected]>
CONFIG_TEGRA_IOMMU_SMMUis enabled in the default kernel configs and hence DRM should fail to probe on t124 since 4.11. What about to add stable tag for v4.11+ here to unbreak stable kernels as well?
IOMMU node for host1x was added to t124 DT in kernel v4.14, so s/4.11/4.14/.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/tegra: Acquire a reference to the IOVA cache
Quoted 15 lines · Dmitry Osipenko
Quoted 14 lines · Dmitry Osipenko
Quoted 12 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The IOVA API uses a memory cache to allocate IOVA nodes from. To make sure that this cache is available, obtain a reference to it and release the reference when the cache is no longer needed.
On 64-bit ARM this is hidden by the fact that the DMA mapping API gets that reference and never releases it. On 32-bit ARM, however, the DMA mapping API doesn't do that, so allocation of IOVA nodes fails.
Signed-off-by Thierry Reding <[email protected]>---
Quoted 8 lines
Quoted 6 lines
Reviewed-by Dmitry Osipenko <[email protected]>Tested-by Dmitry Osipenko <[email protected]>
CONFIG_TEGRA_IOMMU_SMMUis enabled in the default kernel configs and hence DRM should fail to probe on t124 since 4.11. What about to add stable tag for v4.11+ here to unbreak stable kernels as well?IOMMU node for host1x was added to t124 DT in kernel v4.14, so s/4.11/4.14/.
On the other hand nothing stops to use newer DT with older kernel.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/tegra: Acquire a reference to the IOVA cache
Quoted 19 lines · Dmitry Osipenko
Quoted 18 lines · Dmitry Osipenko
Quoted 17 lines · Dmitry Osipenko
Quoted 6 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The IOVA API uses a memory cache to allocate IOVA nodes from. To make sure that this cache is available, obtain a reference to it and release the reference when the cache is no longer needed.
Quoted 6 lines
On 64-bit ARM this is hidden by the fact that the DMA mapping API gets that reference and never releases it. On 32-bit ARM, however, the DMA mapping API doesn't do that, so allocation of IOVA nodes fails.
Signed-off-by Thierry Reding <[email protected]>---
Reviewed-by Dmitry Osipenko <[email protected]>Tested-by Dmitry Osipenko <[email protected]>
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
CONFIG_TEGRA_IOMMU_SMMUis enabled in the default kernel configs and hence DRM should fail to probe on t124 since 4.11. What about to add stable tag for v4.11+ here to unbreak stable kernels as well?IOMMU node for host1x was added to t124 DT in kernel v4.14, so s/4.11/4.14/.
On the other hand nothing stops to use newer DT with older kernel.
I've applied this and added:
Since that's the commit that introduced the iova API usage. It seems like we also need a fix in drivers/gpu/host1x to grab a reference to this IOVA cache because the host1x driver also makes use of that. It looks as if this patch currently papers over that bug, and there's very little chance that anyone will use the host1x driver without the Tegra DRM driver. However, it's probably best to still fix it to avoid future exposure.
I'll go type that patch up now.
Thierry
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/tegra: Acquire a reference to the IOVA cache
In reply to Thierry RedingQuoted 31 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The IOVA API uses a memory cache to allocate IOVA nodes from. To make sure that this cache is available, obtain a reference to it and release the reference when the cache is no longer needed.
On 64-bit ARM this is hidden by the fact that the DMA mapping API gets that reference and never releases it. On 32-bit ARM, however, the DMA mapping API doesn't do that, so allocation of IOVA nodes fails.
Signed-off-by Thierry Reding <[email protected]>--- drivers/gpu/drm/tegra/drm.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)drivers/gpu/drm/tegra/drm.c +4
-0@@ -115,6 +115,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags) goto free; } + err =iova_cache_get (); +if (err <0 ) + goto domain; + geometry = &tegra->domain->geometry; gem_start = geometry->aperture_start; gem_end = geometry->aperture_end -CARVEOUT_SZ ;
drivers/gpu/drm/tegra/drm.c static int tegra_drm_load(struct drm_device *drm, unsigned long flags) · line 217
Quoted 10 lines
drivers/gpu/drm/tegra/drm.c +2
-1@@ -205,11 +209,12 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)tegra_drm_fb_free (drm); config:drm_mode_config_cleanup (drm); - +domain:if (tegra->domain) {mutex_destroy (&tegra->mm_lock);drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iova_cache_put ();
I've spotted that this ^ is incorrect. This will put the iova_cache without getting it if iova_cache_get() failed.
drivers/gpu/drm/tegra/drm.c static void tegra_drm_unload(struct drm_device *drm) · line 247
Quoted 12 lines
drivers/gpu/drm/tegra/drm.c +1
-0iommu_domain_free (tegra->domain); } free:@@ -236,6 +241,7 @@ static void tegra_drm_unload(struct drm_device *drm)mutex_destroy (&tegra->mm_lock);drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iova_cache_put ();iommu_domain_free (tegra->domain); }
Thierry, please update the patch:
drivers/gpu/drm/tegra/drm.c +0 -0
@@ -98,6 +98,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
goto free;
}
+ err = iova_cache_get();
+ if (err < 0)
+ goto domain;
+
geometry = &tegra->domain->geometry;
gem_start = geometry->aperture_start;
gem_end = geometry->aperture_end - CARVEOUT_SZ;
@@ -194,8 +198,11 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
mutex_destroy(&tegra->mm_lock); drm_mm_takedown(&tegra->mm); put_iova_domain(&tegra->carveout.domain);
- iommu_domain_free(tegra->domain);
+ iova_cache_put();
}
+domain:
+ if (tegra->domain)
+ iommu_domain_free(tegra->domain);
free:
kfree(tegra);
return err;
@@ -220,6 +227,7 @@ static void tegra_drm_unload(struct drm_device *drm)
mutex_destroy(&tegra->mm_lock); drm_mm_takedown(&tegra->mm); put_iova_domain(&tegra->carveout.domain);
+ iova_cache_put();
iommu_domain_free(tegra->domain);
}
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/tegra: Acquire a reference to the IOVA cache
Quoted 37 lines · Dmitry Osipenko
Quoted 36 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The IOVA API uses a memory cache to allocate IOVA nodes from. To make sure that this cache is available, obtain a reference to it and release the reference when the cache is no longer needed.
On 64-bit ARM this is hidden by the fact that the DMA mapping API gets that reference and never releases it. On 32-bit ARM, however, the DMA mapping API doesn't do that, so allocation of IOVA nodes fails.
Signed-off-by Thierry Reding <[email protected]>--- drivers/gpu/drm/tegra/drm.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)drivers/gpu/drm/tegra/drm.c +5
-1@@ -115,6 +115,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags) goto free; } + err =iova_cache_get (); +if (err <0 ) + goto domain; + geometry = &tegra->domain->geometry; gem_start = geometry->aperture_start; gem_end = geometry->aperture_end -CARVEOUT_SZ ;@@ -205,11 +209,12 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)tegra_drm_fb_free (drm); config:drm_mode_config_cleanup (drm); - +domain:
Quoted 8 lines
drivers/gpu/drm/tegra/drm.c static int tegra_drm_load(struct drm_device *drm, unsigned long flags) · line 217
Quoted 5 lines
drivers/gpu/drm/tegra/drm.c +1
-0if (tegra->domain) {mutex_destroy (&tegra->mm_lock);drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iova_cache_put ();I've spotted that this ^ is incorrect. This will put the
iova_cachewithout getting it ifiova_cache_get()failed.
Good catch, updated the patch with your suggestion.
Thanks,
Thierry
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 6 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The original code works fine, this is merely a cosmetic change to make the teardown order the reverse of the setup order.
Signed-off-by Thierry Reding <[email protected]>
Quoted 25 lines
--- drivers/gpu/drm/tegra/drm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)drivers/gpu/drm/tegra/drm.c +2
-2@@ -207,10 +207,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)drm_mode_config_cleanup (drm);if (tegra->domain) { -iommu_domain_free (tegra->domain); -drm_mm_takedown (&tegra->mm);mutex_destroy (&tegra->mm_lock); +drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iommu_domain_free (tegra->domain); } free:kfree (tegra);@@ -233,10 +233,10 @@ static void tegra_drm_unload(struct drm_device *drm)return ;
drivers/gpu/drm/tegra/drm.c static void tegra_drm_unload(struct drm_device *drm) · line 242
Quoted 10 lines
drivers/gpu/drm/tegra/drm.c +2
-2if (tegra->domain) { -iommu_domain_free (tegra->domain); -drm_mm_takedown (&tegra->mm);mutex_destroy (&tegra->mm_lock); +drm_mm_takedown (&tegra->mm);put_iova_domain (&tegra->carveout.domain); +iommu_domain_free (tegra->domain); }kfree (tegra);
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thierry Reding
AppliedQuoted 8 lines · Dmitry Osipenko
Quoted 6 lines · Thierry Reding
From: Thierry Reding <[email protected]>
The original code works fine, this is merely a cosmetic change to make the teardown order the reverse of the setup order.
Signed-off-by Thierry Reding <[email protected]>Reviewed-by Dmitry Osipenko <[email protected]>
Applied, thanks.
Thierry
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel