[PATCH v1 0/3] IOMMU: Tegra SMMU fixes

[PATCH v1 0/3] IOMMU: Tegra SMMU fixes

[Dmitry Osipenko]

Hello,

This small series primarily fixes a bug that affects Terga30 and
Terga114 platforms, it also carries two patches that improve SMMU
functionality and clean up code a tad.

Dmitry Osipenko (3):
  iommu/tegra-smmu: Fix invalid ASID bits on Tegra30/114
  iommu/tegra-smmu: Properly release domain resources
  iommu/tegra-smmu: Respect IOMMU API read-write protections

 drivers/iommu/tegra-smmu.c | 41 ++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH v1 1/3] iommu/tegra-smmu: Fix invalid ASID bits on Tegra30/114

[Dmitry Osipenko]

Both Tegra30 and Tegra114 have 4 ASID's and the corresponding bitfield of
the TLB_FLUSH register differs from later Tegra generations that have 128
ASID's.

In a result the PTE's are now flushed correctly from TLB and this fixes
problems with graphics (randomly failing tests) on Tegra30.

Cc: stable <stable@vger.kernel.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/iommu/tegra-smmu.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 5182c7d6171e..8d30653cd13a 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -102,7 +102,6 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu, unsigned long offset)
 #define  SMMU_TLB_FLUSH_VA_MATCH_ALL     (0 << 0)
 #define  SMMU_TLB_FLUSH_VA_MATCH_SECTION (2 << 0)
 #define  SMMU_TLB_FLUSH_VA_MATCH_GROUP   (3 << 0)
-#define  SMMU_TLB_FLUSH_ASID(x)          (((x) & 0x7f) << 24)
 #define  SMMU_TLB_FLUSH_VA_SECTION(addr) ((((addr) & 0xffc00000) >> 12) | \
 					  SMMU_TLB_FLUSH_VA_MATCH_SECTION)
 #define  SMMU_TLB_FLUSH_VA_GROUP(addr)   ((((addr) & 0xffffc000) >> 12) | \
@@ -205,8 +204,12 @@ static inline void smmu_flush_tlb_asid(struct tegra_smmu *smmu,
 {
 	u32 value;
 
-	value = SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_ASID(asid) |
-		SMMU_TLB_FLUSH_VA_MATCH_ALL;
+	if (smmu->soc->num_asids == 4)
+		value = (asid & 0x3) << 29;
+	else
+		value = (asid & 0x7f) << 24;
+
+	value |= SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_VA_MATCH_ALL;
 	smmu_writel(smmu, value, SMMU_TLB_FLUSH);
 }
 
@@ -216,8 +219,12 @@ static inline void smmu_flush_tlb_section(struct tegra_smmu *smmu,
 {
 	u32 value;
 
-	value = SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_ASID(asid) |
-		SMMU_TLB_FLUSH_VA_SECTION(iova);
+	if (smmu->soc->num_asids == 4)
+		value = (asid & 0x3) << 29;
+	else
+		value = (asid & 0x7f) << 24;
+
+	value |= SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_VA_SECTION(iova);
 	smmu_writel(smmu, value, SMMU_TLB_FLUSH);
 }
 
@@ -227,8 +234,12 @@ static inline void smmu_flush_tlb_group(struct tegra_smmu *smmu,
 {
 	u32 value;
 
-	value = SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_ASID(asid) |
-		SMMU_TLB_FLUSH_VA_GROUP(iova);
+	if (smmu->soc->num_asids == 4)
+		value = (asid & 0x3) << 29;
+	else
+		value = (asid & 0x7f) << 24;
+
+	value |= SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_VA_GROUP(iova);
 	smmu_writel(smmu, value, SMMU_TLB_FLUSH);
 }
 
-- 
2.20.1

[PATCH v1 2/3] iommu/tegra-smmu: Properly release domain resources

[Dmitry Osipenko]

Release all memory allocations associated with a released domain and emit
warning if domain is in-use at the time of destruction.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/iommu/tegra-smmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 8d30653cd13a..27b1249f0773 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -327,6 +327,9 @@ static void tegra_smmu_domain_free(struct iommu_domain *domain)
 
 	/* TODO: free page directory and page tables */
 
+	WARN_ON_ONCE(as->use_count);
+	kfree(as->count);
+	kfree(as->pts);
 	kfree(as);
 }
 
-- 
2.20.1

[PATCH v1 3/3] iommu/tegra-smmu: Respect IOMMU API read-write protections

[Dmitry Osipenko]

Set PTE read/write attributes accordingly to the the protections requested
by IOMMU API.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/iommu/tegra-smmu.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 27b1249f0773..463ee08f7d3a 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -145,8 +145,6 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu, unsigned long offset)
 
 #define SMMU_PDE_ATTR		(SMMU_PDE_READABLE | SMMU_PDE_WRITABLE | \
 				 SMMU_PDE_NONSECURE)
-#define SMMU_PTE_ATTR		(SMMU_PTE_READABLE | SMMU_PTE_WRITABLE | \
-				 SMMU_PTE_NONSECURE)
 
 static unsigned int iova_pd_index(unsigned long iova)
 {
@@ -659,6 +657,7 @@ static int tegra_smmu_map(struct iommu_domain *domain, unsigned long iova,
 {
 	struct tegra_smmu_as *as = to_smmu_as(domain);
 	dma_addr_t pte_dma;
+	u32 pte_attrs;
 	u32 *pte;
 
 	pte = as_get_pte(as, iova, &pte_dma);
@@ -669,8 +668,16 @@ static int tegra_smmu_map(struct iommu_domain *domain, unsigned long iova,
 	if (*pte == 0)
 		tegra_smmu_pte_get_use(as, iova);
 
+	pte_attrs = SMMU_PTE_NONSECURE;
+
+	if (prot & IOMMU_READ)
+		pte_attrs |= SMMU_PTE_READABLE;
+
+	if (prot & IOMMU_WRITE)
+		pte_attrs |= SMMU_PTE_WRITABLE;
+
 	tegra_smmu_set_pte(as, iova, pte, pte_dma,
-			   __phys_to_pfn(paddr) | SMMU_PTE_ATTR);
+			   __phys_to_pfn(paddr) | pte_attrs);
 
 	return 0;
 }
-- 
2.20.1

Re: [PATCH v1 0/3] IOMMU: Tegra SMMU fixes

[Dmitry Osipenko]

07.03.2019 1:50, Dmitry Osipenko пишет:
> Hello,
> 
> This small series primarily fixes a bug that affects Terga30 and
> Terga114 platforms, it also carries two patches that improve SMMU
> functionality and clean up code a tad.
> 
> Dmitry Osipenko (3):
>   iommu/tegra-smmu: Fix invalid ASID bits on Tegra30/114
>   iommu/tegra-smmu: Properly release domain resources
>   iommu/tegra-smmu: Respect IOMMU API read-write protections
> 
>  drivers/iommu/tegra-smmu.c | 41 ++++++++++++++++++++++++++++----------
>  1 file changed, 31 insertions(+), 10 deletions(-)
> 

Joerg, could you please apply this series?

Re: [PATCH v1 1/3] iommu/tegra-smmu: Fix invalid ASID bits on Tegra30/114

[Thierry Reding]

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

On Thu, Mar 07, 2019 at 01:50:07AM +0300, Dmitry Osipenko wrote:
> Both Tegra30 and Tegra114 have 4 ASID's and the corresponding bitfield of
> the TLB_FLUSH register differs from later Tegra generations that have 128
> ASID's.
> 
> In a result the PTE's are now flushed correctly from TLB and this fixes
> problems with graphics (randomly failing tests) on Tegra30.
> 
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/iommu/tegra-smmu.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 5182c7d6171e..8d30653cd13a 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -102,7 +102,6 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu, unsigned long offset)
>  #define  SMMU_TLB_FLUSH_VA_MATCH_ALL     (0 << 0)
>  #define  SMMU_TLB_FLUSH_VA_MATCH_SECTION (2 << 0)
>  #define  SMMU_TLB_FLUSH_VA_MATCH_GROUP   (3 << 0)
> -#define  SMMU_TLB_FLUSH_ASID(x)          (((x) & 0x7f) << 24)

Given that the same operation is repeated three times below, it might
have been worth to fold the conditional into the macro. That'd require
the macro to take an smmu parameter, but would otherwise leave the
individual instances shorter.

But either way, the fix is good, so:

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

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

Re: [PATCH v1 2/3] iommu/tegra-smmu: Properly release domain resources

[Thierry Reding]

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

On Thu, Mar 07, 2019 at 01:50:08AM +0300, Dmitry Osipenko wrote:
> Release all memory allocations associated with a released domain and emit
> warning if domain is in-use at the time of destruction.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/iommu/tegra-smmu.c | 3 +++
>  1 file changed, 3 insertions(+)

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

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

Re: [PATCH v1 3/3] iommu/tegra-smmu: Respect IOMMU API read-write protections

[Thierry Reding]

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

On Thu, Mar 07, 2019 at 01:50:09AM +0300, Dmitry Osipenko wrote:
> Set PTE read/write attributes accordingly to the the protections requested
> by IOMMU API.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/iommu/tegra-smmu.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)

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

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

Re: [PATCH v1 1/3] iommu/tegra-smmu: Fix invalid ASID bits on Tegra30/114

[Dmitry Osipenko]

03.04.2019 11:41, Thierry Reding пишет:
> On Thu, Mar 07, 2019 at 01:50:07AM +0300, Dmitry Osipenko wrote:
>> Both Tegra30 and Tegra114 have 4 ASID's and the corresponding bitfield of
>> the TLB_FLUSH register differs from later Tegra generations that have 128
>> ASID's.
>>
>> In a result the PTE's are now flushed correctly from TLB and this fixes
>> problems with graphics (randomly failing tests) on Tegra30.
>>
>> Cc: stable <stable@vger.kernel.org>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/iommu/tegra-smmu.c | 25 ++++++++++++++++++-------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>> index 5182c7d6171e..8d30653cd13a 100644
>> --- a/drivers/iommu/tegra-smmu.c
>> +++ b/drivers/iommu/tegra-smmu.c
>> @@ -102,7 +102,6 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu, unsigned long offset)
>>  #define  SMMU_TLB_FLUSH_VA_MATCH_ALL     (0 << 0)
>>  #define  SMMU_TLB_FLUSH_VA_MATCH_SECTION (2 << 0)
>>  #define  SMMU_TLB_FLUSH_VA_MATCH_GROUP   (3 << 0)
>> -#define  SMMU_TLB_FLUSH_ASID(x)          (((x) & 0x7f) << 24)
> 
> Given that the same operation is repeated three times below, it might
> have been worth to fold the conditional into the macro. That'd require
> the macro to take an smmu parameter, but would otherwise leave the
> individual instances shorter.

I had that variant initially and the result felt more clumsy to me.

> But either way, the fix is good, so:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>
> 

Thanks

Re: [PATCH v1 0/3] IOMMU: Tegra SMMU fixes

[Joerg Roedel]

On Wed, Apr 03, 2019 at 08:14:18AM +0300, Dmitry Osipenko wrote:

> Joerg, could you please apply this series?

Applied, thanks.