[PATCH v1 0/2] arm-smmu fixes for CBn_TCR and S2CR/SMR programming

[PATCH v1 0/2] arm-smmu fixes for CBn_TCR and S2CR/SMR programming

[Olav Haugan]

Here are a couple fixes for the arm smmu driver. The first one deals with
ensuring that we program CBn_TCR correctly when we are programming a stage-1
context bank.
The second patch ensures that the S2CR and SMR registers are programmed
correctly for either stream id matching or stream id indexing. When stream
id matching is not supported by the hardware the SMR registers does not
exists. However, even if they are UNK/SBZP we prefer not to write to more
registers than needed.

Olav Haugan (2):
  iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1
  iommu/arm-smmu: Correctly program S2CR and SMR registers

 drivers/iommu/arm-smmu.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v1 0/2] arm-smmu fixes for CBn_TCR and S2CR/SMR programming

[Olav Haugan]

Here are a couple fixes for the arm smmu driver. The first one deals with
ensuring that we program CBn_TCR correctly when we are programming a stage-1
context bank.
The second patch ensures that the S2CR and SMR registers are programmed
correctly for either stream id matching or stream id indexing. When stream
id matching is not supported by the hardware the SMR registers does not
exists. However, even if they are UNK/SBZP we prefer not to write to more
registers than needed.

Olav Haugan (2):
  iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1
  iommu/arm-smmu: Correctly program S2CR and SMR registers

 drivers/iommu/arm-smmu.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v1 1/2] iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1

[Olav Haugan]

Stage-1 context bank does not have SMMU_CBn_TCR[SL0] field. SL0 field
is only applicable to stage-2 context banks.

Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
---
 drivers/iommu/arm-smmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ff6633d..a83ca6a 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -833,6 +833,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 				reg |= (TTBCR2_ADDR_48 << TTBCR_PASIZE_SHIFT);
 				break;
 			}
+			reg |= (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
+
 		} else {
 			reg |= (64 - smmu->input_size) << TTBCR_T0SZ_SHIFT;
 		}
@@ -843,8 +845,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 	reg |= TTBCR_EAE |
 	      (TTBCR_SH_IS << TTBCR_SH0_SHIFT) |
 	      (TTBCR_RGN_WBWA << TTBCR_ORGN0_SHIFT) |
-	      (TTBCR_RGN_WBWA << TTBCR_IRGN0_SHIFT) |
-	      (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
+	      (TTBCR_RGN_WBWA << TTBCR_IRGN0_SHIFT);
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBCR);
 
 	/* MAIR0 (stage-1 only) */
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v1 1/2] iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1

[Olav Haugan]

Stage-1 context bank does not have SMMU_CBn_TCR[SL0] field. SL0 field
is only applicable to stage-2 context banks.

Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
---
 drivers/iommu/arm-smmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ff6633d..a83ca6a 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -833,6 +833,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 				reg |= (TTBCR2_ADDR_48 << TTBCR_PASIZE_SHIFT);
 				break;
 			}
+			reg |= (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
+
 		} else {
 			reg |= (64 - smmu->input_size) << TTBCR_T0SZ_SHIFT;
 		}
@@ -843,8 +845,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 	reg |= TTBCR_EAE |
 	      (TTBCR_SH_IS << TTBCR_SH0_SHIFT) |
 	      (TTBCR_RGN_WBWA << TTBCR_ORGN0_SHIFT) |
-	      (TTBCR_RGN_WBWA << TTBCR_IRGN0_SHIFT) |
-	      (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
+	      (TTBCR_RGN_WBWA << TTBCR_IRGN0_SHIFT);
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBCR);
 
 	/* MAIR0 (stage-1 only) */
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v1 1/2] iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1

[Will Deacon]

Hi Olav,

On Mon, Aug 04, 2014 at 01:47:43AM +0100, Olav Haugan wrote:
> Stage-1 context bank does not have SMMU_CBn_TCR[SL0] field. SL0 field
> is only applicable to stage-2 context banks.

Ha, I actually noticed this last week but didn't get around to writing a
a fix. Thanks for taking a look!

> Signed-off-by: Olav Haugan <ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index ff6633d..a83ca6a 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -833,6 +833,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
>  				reg |= (TTBCR2_ADDR_48 << TTBCR_PASIZE_SHIFT);
>  				break;
>  			}
> +			reg |= (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
> +

I think this block is all predicated on smmu->version > 1, so we need
something for stage2 CBs on SMMUv1 too.

Will

[PATCH v1 1/2] iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1

[Will Deacon]

Hi Olav,

On Mon, Aug 04, 2014 at 01:47:43AM +0100, Olav Haugan wrote:
> Stage-1 context bank does not have SMMU_CBn_TCR[SL0] field. SL0 field
> is only applicable to stage-2 context banks.

Ha, I actually noticed this last week but didn't get around to writing a
a fix. Thanks for taking a look!

> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
> ---
>  drivers/iommu/arm-smmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index ff6633d..a83ca6a 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -833,6 +833,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
>  				reg |= (TTBCR2_ADDR_48 << TTBCR_PASIZE_SHIFT);
>  				break;
>  			}
> +			reg |= (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
> +

I think this block is all predicated on smmu->version > 1, so we need
something for stage2 CBs on SMMUv1 too.

Will

Re: [PATCH v1 1/2] iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1

[Olav Haugan]

On 8/4/2014 2:41 AM, Will Deacon wrote:
> Hi Olav,
> 
> On Mon, Aug 04, 2014 at 01:47:43AM +0100, Olav Haugan wrote:
>> Stage-1 context bank does not have SMMU_CBn_TCR[SL0] field. SL0 field
>> is only applicable to stage-2 context banks.
> 
> Ha, I actually noticed this last week but didn't get around to writing a
> a fix. Thanks for taking a look!
> 
>> Signed-off-by: Olav Haugan <ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  drivers/iommu/arm-smmu.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index ff6633d..a83ca6a 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -833,6 +833,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
>>  				reg |= (TTBCR2_ADDR_48 << TTBCR_PASIZE_SHIFT);
>>  				break;
>>  			}
>> +			reg |= (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
>> +
> 
> I think this block is all predicated on smmu->version > 1, so we need
> something for stage2 CBs on SMMUv1 too.

Oh, I see. I will fix that.


Olav

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v1 1/2] iommu/arm-smmu: Fix programming of SMMU_CBn_TCR for stage 1

[Olav Haugan]

On 8/4/2014 2:41 AM, Will Deacon wrote:
> Hi Olav,
> 
> On Mon, Aug 04, 2014 at 01:47:43AM +0100, Olav Haugan wrote:
>> Stage-1 context bank does not have SMMU_CBn_TCR[SL0] field. SL0 field
>> is only applicable to stage-2 context banks.
> 
> Ha, I actually noticed this last week but didn't get around to writing a
> a fix. Thanks for taking a look!
> 
>> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
>> ---
>>  drivers/iommu/arm-smmu.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index ff6633d..a83ca6a 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -833,6 +833,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
>>  				reg |= (TTBCR2_ADDR_48 << TTBCR_PASIZE_SHIFT);
>>  				break;
>>  			}
>> +			reg |= (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
>> +
> 
> I think this block is all predicated on smmu->version > 1, so we need
> something for stage2 CBs on SMMUv1 too.

Oh, I see. I will fix that.


Olav

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v1 2/2] iommu/arm-smmu: Correctly program S2CR and SMR registers

[Olav Haugan]

When stream matching is supported by the hardware the S2CR is indexed
by the stream matching register group index and not the stream id.
Fix the programming of the S2CR register to ensure it is programmed
correctly in both cases.

In addition, the SMR registers does not exist when stream matching is not
supported by the hardware. Avoid writing to this register if not needed.

Signed-off-by: Olav Haugan <ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a83ca6a..19cca83 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1115,6 +1115,9 @@ static void arm_smmu_master_free_smrs(struct arm_smmu_device *smmu,
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 	struct arm_smmu_smr *smrs = cfg->smrs;
 
+	if (!smrs)
+		return;
+
 	/* Invalidate the SMRs before freeing back to the allocator */
 	for (i = 0; i < cfg->num_streamids; ++i) {
 		u8 idx = smrs[i].idx;
@@ -1134,10 +1137,10 @@ static void arm_smmu_bypass_stream_mapping(struct arm_smmu_device *smmu,
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 
 	for (i = 0; i < cfg->num_streamids; ++i) {
-		u16 sid = cfg->streamids[i];
-
+		u32 idx;
+		idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
 		writel_relaxed(S2CR_TYPE_BYPASS,
-			       gr0_base + ARM_SMMU_GR0_S2CR(sid));
+			       gr0_base + ARM_SMMU_GR0_S2CR(idx));
 	}
 }
 
@@ -1630,7 +1633,10 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
 
 	/* Mark all SMRn as invalid and all S2CRn as bypass */
 	for (i = 0; i < smmu->num_mapping_groups; ++i) {
-		writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(i));
+		if (smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) {
+			writel_relaxed(~SMR_VALID,
+					gr0_base + ARM_SMMU_GR0_SMR(i));
+		}
 		writel_relaxed(S2CR_TYPE_BYPASS,
 			gr0_base + ARM_SMMU_GR0_S2CR(i));
 	}
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v1 2/2] iommu/arm-smmu: Correctly program S2CR and SMR registers

[Olav Haugan]

When stream matching is supported by the hardware the S2CR is indexed
by the stream matching register group index and not the stream id.
Fix the programming of the S2CR register to ensure it is programmed
correctly in both cases.

In addition, the SMR registers does not exist when stream matching is not
supported by the hardware. Avoid writing to this register if not needed.

Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
---
 drivers/iommu/arm-smmu.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a83ca6a..19cca83 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1115,6 +1115,9 @@ static void arm_smmu_master_free_smrs(struct arm_smmu_device *smmu,
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 	struct arm_smmu_smr *smrs = cfg->smrs;
 
+	if (!smrs)
+		return;
+
 	/* Invalidate the SMRs before freeing back to the allocator */
 	for (i = 0; i < cfg->num_streamids; ++i) {
 		u8 idx = smrs[i].idx;
@@ -1134,10 +1137,10 @@ static void arm_smmu_bypass_stream_mapping(struct arm_smmu_device *smmu,
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 
 	for (i = 0; i < cfg->num_streamids; ++i) {
-		u16 sid = cfg->streamids[i];
-
+		u32 idx;
+		idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
 		writel_relaxed(S2CR_TYPE_BYPASS,
-			       gr0_base + ARM_SMMU_GR0_S2CR(sid));
+			       gr0_base + ARM_SMMU_GR0_S2CR(idx));
 	}
 }
 
@@ -1630,7 +1633,10 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
 
 	/* Mark all SMRn as invalid and all S2CRn as bypass */
 	for (i = 0; i < smmu->num_mapping_groups; ++i) {
-		writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(i));
+		if (smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) {
+			writel_relaxed(~SMR_VALID,
+					gr0_base + ARM_SMMU_GR0_SMR(i));
+		}
 		writel_relaxed(S2CR_TYPE_BYPASS,
 			gr0_base + ARM_SMMU_GR0_S2CR(i));
 	}
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v1 2/2] iommu/arm-smmu: Correctly program S2CR and SMR registers

[Will Deacon]

On Mon, Aug 04, 2014 at 01:47:44AM +0100, Olav Haugan wrote:
> When stream matching is supported by the hardware the S2CR is indexed
> by the stream matching register group index and not the stream id.
> Fix the programming of the S2CR register to ensure it is programmed
> correctly in both cases.
> 
> In addition, the SMR registers does not exist when stream matching is not
> supported by the hardware. Avoid writing to this register if not needed.

I've already fixed some/all of this in my iommu/pci branch:

  https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/?h=iommu/pci&id=d4e46030b91367b50ecb544fee021b30e4ec4e9a

Can you try rebasing on that and see if there's anything left to do, please?

Once we get to -rc1, I'll sort out my branches so the iommu/devel branch
will contain stuff that I consider candidates for merging (since the
iommu/pci branch contains a bunch of WIP/RFC stuff too)

Cheers,

Will

[PATCH v1 2/2] iommu/arm-smmu: Correctly program S2CR and SMR registers

[Will Deacon]

On Mon, Aug 04, 2014 at 01:47:44AM +0100, Olav Haugan wrote:
> When stream matching is supported by the hardware the S2CR is indexed
> by the stream matching register group index and not the stream id.
> Fix the programming of the S2CR register to ensure it is programmed
> correctly in both cases.
> 
> In addition, the SMR registers does not exist when stream matching is not
> supported by the hardware. Avoid writing to this register if not needed.

I've already fixed some/all of this in my iommu/pci branch:

  https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/?h=iommu/pci&id=d4e46030b91367b50ecb544fee021b30e4ec4e9a

Can you try rebasing on that and see if there's anything left to do, please?

Once we get to -rc1, I'll sort out my branches so the iommu/devel branch
will contain stuff that I consider candidates for merging (since the
iommu/pci branch contains a bunch of WIP/RFC stuff too)

Cheers,

Will