IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported

PATCH Superseded pmallapp at broadcom.com 3 participants 5 messages Nov 16, 2015 – Dec 12, 2015 IOMMU

lore

Share & tools

lore

pmallapp at broadcom.com

author

2015-11-16 08:48 UTC #0 lore raw

PATCH 1/1 IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported

From: Prem Mallappa <[email protected]>

Also fix the STALLD check.

Signed-off-by Prem Mallappa <[email protected]>

---
 drivers/iommu/arm-smmu-v3.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

drivers/iommu/arm-smmu-v3.c +13 -3

index ed409cb..804671c 100644
@@ -38,7 +38,9 @@
 #define IDR0_ST_LVL_SHIFT		27
 #define IDR0_ST_LVL_MASK		0x3
 #define IDR0_ST_LVL_2LVL		(1 << IDR0_ST_LVL_SHIFT)
-#define IDR0_STALL_MODEL		(3 << 24)
+#define IDR0_STALL_SHIFT		24
+#define IDR0_STALL_MODEL_STALL		0x0
+#define IDR0_STALL_MODEL_FORCE		0x2
 #define IDR0_TTENDIAN_SHIFT		21
 #define IDR0_TTENDIAN_MASK		0x3
 #define IDR0_TTENDIAN_LE		(2 << IDR0_TTENDIAN_SHIFT)

@@ -1051,12 +1053,14 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
 			 STRTAB_STE_1_S1C_CACHE_WBRA
 			 << STRTAB_STE_1_S1COR_SHIFT |
 			 STRTAB_STE_1_S1C_SH_ISH << STRTAB_STE_1_S1CSH_SHIFT |
-			 STRTAB_STE_1_S1STALLD |
 #ifdef CONFIG_PCI_ATS
 			 STRTAB_STE_1_EATS_TRANS << STRTAB_STE_1_EATS_SHIFT |
 #endif
 			 STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT);
 
+		if (smmu->features & ARM_SMMU_FEAT_STALLS)
+			dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
+
 		val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK
 		        << STRTAB_STE_0_S1CTXPTR_SHIFT) |
 			STRTAB_STE_0_CFG_S1_TRANS;

@@ -2483,8 +2487,14 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
 		dev_warn(smmu->dev, "IDR0.COHACC overridden by dma-coherent property (%s)\n",
 			 coherent ? "true" : "false");
 
-	if (reg & IDR0_STALL_MODEL)
+	switch ((reg >> IDR0_STALL_SHIFT) & 0x3) {
+	case IDR0_STALL_MODEL_STALL:
+	case IDR0_STALL_MODEL_FORCE:
 		smmu->features |= ARM_SMMU_FEAT_STALLS;
+		break;
+	default:
+		break;
+	}
 
 	if (reg & IDR0_S1P)
 		smmu->features |= ARM_SMMU_FEAT_TRANS_S1;

Signature · 1 line

2.6.0

Will Deacon

2015-11-17 17:46 UTC #1 lore raw

PATCH 1/1 IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported

Hi Prem,

Thanks for the patch. Just a few minor comments...

Quoted 3 lines · pmallapp at broadcom.com

From: Prem Mallappa <[email protected]>
Also fix the STALLD check.

Can you extend the commit message please to explain that it is ILLEGAL to set STE.S1STALLD to 1 if stage 1 is enabled and either the stall or terminate models are not supported?

Quoted 10 lines

Signed-off-by Prem Mallappa <[email protected]>
---
 drivers/iommu/arm-smmu-v3.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
drivers/iommu/arm-smmu-v3.c +0 -0
index ed409cb..804671c 100644

drivers/iommu/arm-smmu-v3.c line 41

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c +1 -1

@@ -38,7 +38,9 @@
 #define IDR0_ST_LVL_SHIFT		27
 #define IDR0_ST_LVL_MASK		0x3
 #define IDR0_ST_LVL_2LVL		(1 << IDR0_ST_LVL_SHIFT)
-#define IDR0_STALL_MODEL		(3 << 24)
+#define IDR0_STALL_SHIFT		24

To keep the style consistent, can you also add IDR0_STALL_MASK please...

drivers/iommu/arm-smmu-v3.c line 43

Quoted 2 lines

drivers/iommu/arm-smmu-v3.c +2 -0

+#define IDR0_STALL_MODEL_STALL		0x0
+#define IDR0_STALL_MODEL_FORCE		0x2

... and shift these guys left by IDR0_STALL_SHIFT?

Quoted 21 lines

drivers/iommu/arm-smmu-v3.c +3 -1

 #define IDR0_TTENDIAN_SHIFT		21
 #define IDR0_TTENDIAN_MASK		0x3
 #define IDR0_TTENDIAN_LE		(2 << IDR0_TTENDIAN_SHIFT)

@@ -1051,12 +1053,14 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
 			 STRTAB_STE_1_S1C_CACHE_WBRA
 			 << STRTAB_STE_1_S1COR_SHIFT |
 			 STRTAB_STE_1_S1C_SH_ISH << STRTAB_STE_1_S1CSH_SHIFT |
-			 STRTAB_STE_1_S1STALLD |
 #ifdef CONFIG_PCI_ATS
 			 STRTAB_STE_1_EATS_TRANS << STRTAB_STE_1_EATS_SHIFT |
 #endif
 			 STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT);
 
+		if (smmu->features & ARM_SMMU_FEAT_STALLS)
+			dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
+
 		val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK
 		        << STRTAB_STE_0_S1CTXPTR_SHIFT) |
 			STRTAB_STE_0_CFG_S1_TRANS;

drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_probe(struct arm_smmu_device *smmu) · line 2490

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c +1 -1

@@ -2483,8 +2487,14 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
 		dev_warn(smmu->dev, "IDR0.COHACC overridden by dma-coherent property (%s)\n",
 			 coherent ? "true" : "false");
 
-	if (reg & IDR0_STALL_MODEL)
+	switch ((reg >> IDR0_STALL_SHIFT) & 0x3) {

Then structure this more like the TTENDIAN code:

  switch (reg & IDR0_STALL_MASK << IDR0_STALL_SHIFT) {

drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_probe(struct arm_smmu_device *smmu) · line 2497

Quoted 7 lines

drivers/iommu/arm-smmu-v3.c +6 -0

+	case IDR0_STALL_MODEL_STALL:
+	case IDR0_STALL_MODEL_FORCE:
 		smmu->features |= ARM_SMMU_FEAT_STALLS;
+		break;
+	default:
+		break;
+	}

I think you can drop the default case, or does GCC complain?

Will

Prem (Premachandra) Mallappa

2015-11-24 05:30 UTC #2 lore raw

PATCH 1/1 IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported

Quoted 94 lines

-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com]
Sent: 17 November 2015 11:17 PM
To: Prem (Premachandra) Mallappa
Cc: iommu at lists.linux-foundation.org; linux-arm-kernel at lists.infradead.org; Jayachandran Chandrashekaran Nair
Subject: Re: [PATCH] IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported
Hi Prem,
Thanks for the patch. Just a few minor comments...
Quoted 3 lines · pmallapp at broadcom.com
From: Prem Mallappa <[email protected]>
Also fix the STALLD check.
Can you extend the commit message please to explain that it is ILLEGAL to set STE.S1STALLD to 1 if stage 1 is enabled and either the stall or terminate models are not supported?
drivers/iommu/arm-smmu-
Quoted 6 lines
Signed-off-by Prem Mallappa <[email protected]>
---
 drivers/iommu/arm-smmu-v3.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
drivers/iommu/arm-smmu- renamed from drivers/iommu/arm-smmu-v3.c +0 -0
v3.c
drivers/iommu/arm-smmu-v3.c line 41
Quoted 9 lines
index ed409cb..804671c 100644
drivers/iommu/arm-smmu-v3.c +1 -1
@@ -38,7 +38,9 @@
 #define IDR0_ST_LVL_SHIFT		27
 #define IDR0_ST_LVL_MASK		0x3
 #define IDR0_ST_LVL_2LVL		(1 << IDR0_ST_LVL_SHIFT)
-#define IDR0_STALL_MODEL		(3 << 24)
+#define IDR0_STALL_SHIFT		24
To keep the style consistent, can you also add IDR0_STALL_MASK please...
drivers/iommu/arm-smmu-v3.c line 43
Quoted 2 lines
drivers/iommu/arm-smmu-v3.c +2 -0
+#define IDR0_STALL_MODEL_STALL		0x0
+#define IDR0_STALL_MODEL_FORCE		0x2
... and shift these guys left by IDR0_STALL_SHIFT?
drivers/iommu/arm-smmu-v3.c static void arm_smmu_write_strtab_ent(struct
Quoted 5 lines
drivers/iommu/arm-smmu-v3.c +0 -0
 #define IDR0_TTENDIAN_SHIFT		21
 #define IDR0_TTENDIAN_MASK		0x3
 #define IDR0_TTENDIAN_LE		(2 << IDR0_TTENDIAN_SHIFT)
@@ -1051,12 +1053,14 @@ static void arm_smmu_write_strtab_ent(struct
arm_smmu_device *smmu, u32 sid,
drivers/iommu/arm-smmu-v3.c static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
Quoted 3 lines
drivers/iommu/arm-smmu-v3.c +0 -0
 			 STRTAB_STE_1_S1C_CACHE_WBRA
 			 << STRTAB_STE_1_S1COR_SHIFT |
 			 STRTAB_STE_1_S1C_SH_ISH <<
STRTAB_STE_1_S1CSH_SHIFT |
drivers/iommu/arm-smmu-v3.c static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
Quoted 3 lines
drivers/iommu/arm-smmu-v3.c +0 -1
-			 STRTAB_STE_1_S1STALLD |
 #ifdef CONFIG_PCI_ATS
 			 STRTAB_STE_1_EATS_TRANS <<
STRTAB_STE_1_EATS_SHIFT | #endif
drivers/iommu/arm-smmu-v3.c static void arm_smmu_write_strtab_ent(struct · line 1058
Quoted 1 line
drivers/iommu/arm-smmu-v3.c +0 -0
 			 STRTAB_STE_1_STRW_NSEL1 <<
STRTAB_STE_1_STRW_SHIFT);
drivers/iommu/arm-smmu-v3.c static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
Quoted 5 lines
drivers/iommu/arm-smmu-v3.c +3 -0
+		if (smmu->features & ARM_SMMU_FEAT_STALLS)
+			dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
+
 		val |= (ste->s1_cfg->cdptr_dma &
STRTAB_STE_0_S1CTXPTR_MASK
drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_probe(struct
Quoted 3 lines
drivers/iommu/arm-smmu-v3.c +0 -0
 		        << STRTAB_STE_0_S1CTXPTR_SHIFT) |
 			STRTAB_STE_0_CFG_S1_TRANS;
@@ -2483,8 +2487,14 @@ static int arm_smmu_device_probe(struct
arm_smmu_device *smmu)
drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_probe(struct · line 2487
Quoted 1 line
drivers/iommu/arm-smmu-v3.c +0 -0
 		dev_warn(smmu->dev, "IDR0.COHACC overridden by dma-
coherent property (%s)\n",
drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_probe(struct arm_smmu_device *smmu) · line 2490
Quoted 4 lines
drivers/iommu/arm-smmu-v3.c +1 -1
 			 coherent ? "true" : "false");
 
-	if (reg & IDR0_STALL_MODEL)
+	switch ((reg >> IDR0_STALL_SHIFT) & 0x3) {
Then structure this more like the TTENDIAN code:
  switch (reg & IDR0_STALL_MASK << IDR0_STALL_SHIFT) {
drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_probe(struct arm_smmu_device *smmu) · line 2497
Quoted 7 lines
drivers/iommu/arm-smmu-v3.c +6 -0
+	case IDR0_STALL_MODEL_STALL:
+	case IDR0_STALL_MODEL_FORCE:
 		smmu->features |= ARM_SMMU_FEAT_STALLS;
+		break;
+	default:
+		break;
+	}
I think you can drop the default case, or does GCC complain?

Quoted 1 line

Will

Thanks for comments, I was on holiday, I'll send updated patch along with 2 more soon.

/Prem

Will Deacon

2015-12-11 19:53 UTC #3 lore raw

PATCH 1/1 IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported

Hi Prem,

Quoted 2 lines · Prem (Premachandra) Mallappa

Thanks for comments, I was on holiday, I'll send updated patch along with 2 more soon.

Any update on this patch?

Cheers,

Will

Prem (Premachandra) Mallappa

2015-12-12 05:31 UTC #4 lore raw

PATCH 1/1 IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported

Hi Will,
It is ready, in internal review, will be sending it in a day.

/Prem

Quoted 21 lines

-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com]
Sent: 12 December 2015 01:23 AM
To: Prem (Premachandra) Mallappa
Cc: iommu at lists.linux-foundation.org; linux-arm-kernel at lists.infradead.org; Jayachandran Chandrashekaran Nair
Subject: Re: [PATCH] IOMMU: arm-smmu-v3: Use STE.S1STALLD only when supported
Hi Prem,
Quoted 2 lines · Prem (Premachandra) Mallappa
Thanks for comments, I was on holiday, I'll send updated patch along with 2 more soon.
Any update on this patch?
Cheers,
Will