All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Joerg Roedel <joro@8bytes.org>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Krishna Reddy <vdumpa@nvidia.com>, Will Deacon <will@kernel.org>,
	iommu@lists.linux-foundation.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients
Date: Thu, 8 Apr 2021 02:42:42 -0700	[thread overview]
Message-ID: <20210408094241.GA31714@Asurada-Nvidia> (raw)
In-Reply-To: <20210328233256.20494-1-digetx@gmail.com>

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

On Mon, Mar 29, 2021 at 02:32:55AM +0300, Dmitry Osipenko wrote:
> All consumer-grade Android and Chromebook devices show a splash screen
> on boot and then display is left enabled when kernel is booted. This
> behaviour is unacceptable in a case of implicit IOMMU domains to which
> devices are attached during kernel boot since devices, like display
> controller, may perform DMA at that time. We can work around this problem
> by deferring the enable of SMMU translation for a specific devices,
> like a display controller, until the first IOMMU mapping is created,
> which works good enough in practice because by that time h/w is already
> stopped.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

For both patches:
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Nicolin Chen <nicoleotsuka@gmail.com>

The WAR looks good to me. Perhaps Thierry would give some input.

Another topic:
I think this may help work around the mc-errors, which we have
been facing on Tegra210 also when we enable IOMMU_DOMAIN_DMA.
(attached a test patch rebasing on these two)

However, GPU would also report errors using DMA domain:

 nouveau 57000000.gpu: acr: firmware unavailable
 nouveau 57000000.gpu: pmu: firmware unavailable
 nouveau 57000000.gpu: gr: firmware unavailable
 tegra-mc 70019000.memory-controller: gpusrd: read @0x00000000fffbe200: Security violation (TrustZone violation)
 nouveau 57000000.gpu: DRM: failed to create kernel channel, -22
 tegra-mc 70019000.memory-controller: gpusrd: read @0x00000000fffad000: Security violation (TrustZone violation)
 nouveau 57000000.gpu: fifo: SCHED_ERROR 20 []
 nouveau 57000000.gpu: fifo: SCHED_ERROR 20 []

Looking at the address, seems that GPU allocated memory in 32-bit
physical address space behind SMMU, so a violation happened after
turning on DMA domain I guess... 

[-- Attachment #2: dma_domain.patch --]
[-- Type: text/x-diff, Size: 2395 bytes --]

From 20b58a74fee0c7b961b92f9118ad69a12199e6a5 Mon Sep 17 00:00:00 2001
From: Nicolin Chen <nicolinc@nvidia.com>
Date: Thu, 12 Dec 2019 17:46:50 -0800
Subject: [PATCH 6/7] iommu/tegra-smmu: Add IOMMU_DOMAIN_DMA

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 39 ++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 8104f001e679..eff10d1ec568 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-iommu.h>
 
 #include <soc/tegra/ahb.h>
 #include <soc/tegra/mc.h>
@@ -297,35 +298,29 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
 {
 	struct tegra_smmu_as *as;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED)
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
 
 	as = kzalloc(sizeof(*as), GFP_KERNEL);
 	if (!as)
 		return NULL;
 
+	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(&as->domain))
+		goto free_as;
+
 	as->attr = SMMU_PD_READABLE | SMMU_PD_WRITABLE | SMMU_PD_NONSECURE;
 
 	as->pd = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO);
-	if (!as->pd) {
-		kfree(as);
-		return NULL;
-	}
+	if (!as->pd)
+		goto put_dma_cookie;
 
 	as->count = kcalloc(SMMU_NUM_PDE, sizeof(u32), GFP_KERNEL);
-	if (!as->count) {
-		__free_page(as->pd);
-		kfree(as);
-		return NULL;
-	}
+	if (!as->count)
+		goto free_pd_range;
 
 	as->pts = kcalloc(SMMU_NUM_PDE, sizeof(*as->pts), GFP_KERNEL);
-	if (!as->pts) {
-		kfree(as->count);
-		__free_page(as->pd);
-		kfree(as);
-		return NULL;
-	}
+	if (!as->pts)
+		goto free_pts;
 
 	spin_lock_init(&as->lock);
 
@@ -335,6 +330,17 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
 	as->attached_devices_need_sync = true;
 
 	return &as->domain;
+
+free_pts:
+	kfree(as->pts);
+free_pd_range:
+	__free_page(as->pd);
+put_dma_cookie:
+	iommu_put_dma_cookie(&as->domain);
+free_as:
+	kfree(as);
+
+	return NULL;
 }
 
 static void tegra_smmu_domain_free(struct iommu_domain *domain)
@@ -346,6 +352,7 @@ static void tegra_smmu_domain_free(struct iommu_domain *domain)
 	WARN_ON_ONCE(as->use_count);
 	kfree(as->count);
 	kfree(as->pts);
+	iommu_put_dma_cookie(domain);
 	kfree(as);
 }
 
-- 
2.17.1


WARNING: multiple messages have this Message-ID (diff)
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	linux-tegra@vger.kernel.org, Will Deacon <will@kernel.org>
Subject: Re: [PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients
Date: Thu, 8 Apr 2021 02:42:42 -0700	[thread overview]
Message-ID: <20210408094241.GA31714@Asurada-Nvidia> (raw)
In-Reply-To: <20210328233256.20494-1-digetx@gmail.com>

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

On Mon, Mar 29, 2021 at 02:32:55AM +0300, Dmitry Osipenko wrote:
> All consumer-grade Android and Chromebook devices show a splash screen
> on boot and then display is left enabled when kernel is booted. This
> behaviour is unacceptable in a case of implicit IOMMU domains to which
> devices are attached during kernel boot since devices, like display
> controller, may perform DMA at that time. We can work around this problem
> by deferring the enable of SMMU translation for a specific devices,
> like a display controller, until the first IOMMU mapping is created,
> which works good enough in practice because by that time h/w is already
> stopped.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

For both patches:
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Nicolin Chen <nicoleotsuka@gmail.com>

The WAR looks good to me. Perhaps Thierry would give some input.

Another topic:
I think this may help work around the mc-errors, which we have
been facing on Tegra210 also when we enable IOMMU_DOMAIN_DMA.
(attached a test patch rebasing on these two)

However, GPU would also report errors using DMA domain:

 nouveau 57000000.gpu: acr: firmware unavailable
 nouveau 57000000.gpu: pmu: firmware unavailable
 nouveau 57000000.gpu: gr: firmware unavailable
 tegra-mc 70019000.memory-controller: gpusrd: read @0x00000000fffbe200: Security violation (TrustZone violation)
 nouveau 57000000.gpu: DRM: failed to create kernel channel, -22
 tegra-mc 70019000.memory-controller: gpusrd: read @0x00000000fffad000: Security violation (TrustZone violation)
 nouveau 57000000.gpu: fifo: SCHED_ERROR 20 []
 nouveau 57000000.gpu: fifo: SCHED_ERROR 20 []

Looking at the address, seems that GPU allocated memory in 32-bit
physical address space behind SMMU, so a violation happened after
turning on DMA domain I guess... 

[-- Attachment #2: dma_domain.patch --]
[-- Type: text/x-diff, Size: 2395 bytes --]

From 20b58a74fee0c7b961b92f9118ad69a12199e6a5 Mon Sep 17 00:00:00 2001
From: Nicolin Chen <nicolinc@nvidia.com>
Date: Thu, 12 Dec 2019 17:46:50 -0800
Subject: [PATCH 6/7] iommu/tegra-smmu: Add IOMMU_DOMAIN_DMA

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 39 ++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 8104f001e679..eff10d1ec568 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-iommu.h>
 
 #include <soc/tegra/ahb.h>
 #include <soc/tegra/mc.h>
@@ -297,35 +298,29 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
 {
 	struct tegra_smmu_as *as;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED)
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
 
 	as = kzalloc(sizeof(*as), GFP_KERNEL);
 	if (!as)
 		return NULL;
 
+	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(&as->domain))
+		goto free_as;
+
 	as->attr = SMMU_PD_READABLE | SMMU_PD_WRITABLE | SMMU_PD_NONSECURE;
 
 	as->pd = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO);
-	if (!as->pd) {
-		kfree(as);
-		return NULL;
-	}
+	if (!as->pd)
+		goto put_dma_cookie;
 
 	as->count = kcalloc(SMMU_NUM_PDE, sizeof(u32), GFP_KERNEL);
-	if (!as->count) {
-		__free_page(as->pd);
-		kfree(as);
-		return NULL;
-	}
+	if (!as->count)
+		goto free_pd_range;
 
 	as->pts = kcalloc(SMMU_NUM_PDE, sizeof(*as->pts), GFP_KERNEL);
-	if (!as->pts) {
-		kfree(as->count);
-		__free_page(as->pd);
-		kfree(as);
-		return NULL;
-	}
+	if (!as->pts)
+		goto free_pts;
 
 	spin_lock_init(&as->lock);
 
@@ -335,6 +330,17 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
 	as->attached_devices_need_sync = true;
 
 	return &as->domain;
+
+free_pts:
+	kfree(as->pts);
+free_pd_range:
+	__free_page(as->pd);
+put_dma_cookie:
+	iommu_put_dma_cookie(&as->domain);
+free_as:
+	kfree(as);
+
+	return NULL;
 }
 
 static void tegra_smmu_domain_free(struct iommu_domain *domain)
@@ -346,6 +352,7 @@ static void tegra_smmu_domain_free(struct iommu_domain *domain)
 	WARN_ON_ONCE(as->use_count);
 	kfree(as->count);
 	kfree(as->pts);
+	iommu_put_dma_cookie(domain);
 	kfree(as);
 }
 
-- 
2.17.1


[-- Attachment #3: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  parent reply	other threads:[~2021-04-08  9:44 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-28 23:32 [PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients Dmitry Osipenko
2021-03-28 23:32 ` Dmitry Osipenko
2021-03-28 23:32 ` [PATCH v1 2/2] iommu/tegra-smmu: Revert workaround that was needed for Nyan Big Chromebook Dmitry Osipenko
2021-03-28 23:32   ` Dmitry Osipenko
2021-04-01  8:55   ` Nicolin Chen
2021-04-01  8:55     ` Nicolin Chen
2021-04-02 14:40     ` Dmitry Osipenko
2021-04-02 14:40       ` Dmitry Osipenko
2021-04-23 15:01       ` Guillaume Tucker
2021-04-23 15:01         ` Guillaume Tucker
2021-04-23 15:23         ` Dmitry Osipenko
2021-04-23 15:23           ` Dmitry Osipenko
2021-04-24 20:27           ` Dmitry Osipenko
2021-04-24 20:27             ` Dmitry Osipenko
2021-04-24 20:41             ` Dmitry Osipenko
2021-04-24 20:41               ` Dmitry Osipenko
2021-04-26  7:14             ` Thierry Reding
2021-04-26  7:14               ` Thierry Reding
2021-04-26  7:39               ` Dmitry Osipenko
2021-04-26  7:39                 ` Dmitry Osipenko
2021-04-08  9:42 ` Nicolin Chen [this message]
2021-04-08  9:42   ` [PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients Nicolin Chen
2021-04-08 13:26   ` Thierry Reding
2021-04-08 13:26     ` Thierry Reding
2021-04-08 14:20     ` Dmitry Osipenko
2021-04-08 14:20       ` Dmitry Osipenko
2021-04-08 12:40 ` Thierry Reding
2021-04-08 12:40   ` Thierry Reding
2021-04-08 14:07   ` Dmitry Osipenko
2021-04-08 14:07     ` Dmitry Osipenko
2021-04-09 12:55     ` Dmitry Osipenko
2021-04-09 12:55       ` Dmitry Osipenko

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=20210408094241.GA31714@Asurada-Nvidia \
    --to=nicoleotsuka@gmail.com \
    --cc=digetx@gmail.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jonathanh@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=vdumpa@nvidia.com \
    --cc=will@kernel.org \
    /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.