[RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds

[RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds

[linucherian-Re5JQEeQqe8AvxtiuMwx3w]

From: Linu Cherian <linu.cherian-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Cavium CN99xx SMMUv3 implementation has two Silicon Erratas.
1. Errata ID #74
   SMMU register alias Page 1 is not implemented
2. Errata ID #126
   SMMU doesnt support unique IRQ lines for gerror, eventq and cmdq-sync

The following patchset does software workaround for these two erratas.
An option flag is introduced for each errata which will enable/disable
the errata workarounds.
For device tree based probing, option flags can be turned on by passing 
relevant options along with the smmuv3 device node. For ACPI, option flags
are turned on when Cavium CN99xx SMMuv3 model is identified in the IORT.

Note: We are in the process of getting necessary IORT SMMUv3 model ID 
      for Cavium CN99xx SMMUv3 implementation and it is yet to get
      allocated. We have assumed model ID 3 for this.

Geetha (1):
  iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon
    errata

Linu Cherian (6):
  iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon
    errata.
  iommu/arm-smmu-v3: Do resource size checks based on smmu option
    PAGE0_REGS_ONLY
  ACPICA: IORT: Add SMMuV3 model definitions.
  iommu/arm-smmu-v3: For ACPI based device probing, set relevant options
    for different SMMUv3 implementations.
  ACPI/IORT: Fixup SMMUv3 resource size for Cavium 99xx SMMUv3 model.
  arm64: Documentation: Add Cavium ThunderX2 SMMUv3 erratas.

 Documentation/arm64/silicon-errata.txt |   2 +
 drivers/acpi/arm64/iort.c              |  10 ++-
 drivers/iommu/arm-smmu-v3.c            | 115 ++++++++++++++++++++++++++-------
 include/acpi/actbl2.h                  |   5 ++
 4 files changed, 107 insertions(+), 25 deletions(-)

-- 
1.9.1

[RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds

[linucherian at gmail.com]

From: Linu Cherian <linu.cherian@cavium.com>

Cavium CN99xx SMMUv3 implementation has two Silicon Erratas.
1. Errata ID #74
   SMMU register alias Page 1 is not implemented
2. Errata ID #126
   SMMU doesnt support unique IRQ lines for gerror, eventq and cmdq-sync

The following patchset does software workaround for these two erratas.
An option flag is introduced for each errata which will enable/disable
the errata workarounds.
For device tree based probing, option flags can be turned on by passing 
relevant options along with the smmuv3 device node. For ACPI, option flags
are turned on when Cavium CN99xx SMMuv3 model is identified in the IORT.

Note: We are in the process of getting necessary IORT SMMUv3 model ID 
      for Cavium CN99xx SMMUv3 implementation and it is yet to get
      allocated. We have assumed model ID 3 for this.

Geetha (1):
  iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon
    errata

Linu Cherian (6):
  iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon
    errata.
  iommu/arm-smmu-v3: Do resource size checks based on smmu option
    PAGE0_REGS_ONLY
  ACPICA: IORT: Add SMMuV3 model definitions.
  iommu/arm-smmu-v3: For ACPI based device probing, set relevant options
    for different SMMUv3 implementations.
  ACPI/IORT: Fixup SMMUv3 resource size for Cavium 99xx SMMUv3 model.
  arm64: Documentation: Add Cavium ThunderX2 SMMUv3 erratas.

 Documentation/arm64/silicon-errata.txt |   2 +
 drivers/acpi/arm64/iort.c              |  10 ++-
 drivers/iommu/arm-smmu-v3.c            | 115 ++++++++++++++++++++++++++-------
 include/acpi/actbl2.h                  |   5 ++
 4 files changed, 107 insertions(+), 25 deletions(-)

-- 
1.9.1

[RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata.

[linucherian]

From: Linu Cherian <linu.cherian@cavium.com>

Cavium 99xx SMMU implementation doesn't support page 1 register space
and PAGE0_REGS_ONLY option will be enabled as an errata workaround.

This option when turned on, replaces all page 1 offsets used for
EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 05b4592..df9f27b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -176,15 +176,15 @@
 #define ARM_SMMU_CMDQ_CONS		0x9c
 
 #define ARM_SMMU_EVTQ_BASE		0xa0
-#define ARM_SMMU_EVTQ_PROD		0x100a8
-#define ARM_SMMU_EVTQ_CONS		0x100ac
+#define ARM_SMMU_EVTQ_PROD(s)	        (page1_offset_adjust(0x100a8, s))
+#define ARM_SMMU_EVTQ_CONS(s)		(page1_offset_adjust(0x100ac, s))
 #define ARM_SMMU_EVTQ_IRQ_CFG0		0xb0
 #define ARM_SMMU_EVTQ_IRQ_CFG1		0xb8
 #define ARM_SMMU_EVTQ_IRQ_CFG2		0xbc
 
 #define ARM_SMMU_PRIQ_BASE		0xc0
-#define ARM_SMMU_PRIQ_PROD		0x100c8
-#define ARM_SMMU_PRIQ_CONS		0x100cc
+#define ARM_SMMU_PRIQ_PROD(s)		(page1_offset_adjust(0x100c8, s))
+#define ARM_SMMU_PRIQ_CONS(s)		(page1_offset_adjust(0x100cc, s))
 #define ARM_SMMU_PRIQ_IRQ_CFG0		0xd0
 #define ARM_SMMU_PRIQ_IRQ_CFG1		0xd8
 #define ARM_SMMU_PRIQ_IRQ_CFG2		0xdc
@@ -412,6 +412,9 @@
 #define MSI_IOVA_BASE			0x8000000
 #define MSI_IOVA_LENGTH			0x100000
 
+#define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
+	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
+
 static bool disable_bypass;
 module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
@@ -597,6 +600,7 @@ struct arm_smmu_device {
 	u32				features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
+#define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
 	u32				options;
 
 	struct arm_smmu_cmdq		cmdq;
@@ -663,9 +667,19 @@ struct arm_smmu_option_prop {
 
 static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
+	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
 	{ 0, NULL},
 };
 
+static inline unsigned long page1_offset_adjust(
+	unsigned long off, struct arm_smmu_device *smmu)
+{
+	if (!ARM_SMMU_PAGE0_REGS_ONLY(smmu))
+		return off;
+	else
+		return (off - SZ_64K);
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct arm_smmu_domain, domain);
@@ -1983,8 +1997,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 		return ret;
 
 	/* evtq */
-	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q, ARM_SMMU_EVTQ_PROD,
-				      ARM_SMMU_EVTQ_CONS, EVTQ_ENT_DWORDS);
+	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q,
+				      ARM_SMMU_EVTQ_PROD(smmu),
+				      ARM_SMMU_EVTQ_CONS(smmu),
+				      EVTQ_ENT_DWORDS);
 	if (ret)
 		return ret;
 
@@ -1992,8 +2008,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 	if (!(smmu->features & ARM_SMMU_FEAT_PRI))
 		return 0;
 
-	return arm_smmu_init_one_queue(smmu, &smmu->priq.q, ARM_SMMU_PRIQ_PROD,
-				       ARM_SMMU_PRIQ_CONS, PRIQ_ENT_DWORDS);
+	return arm_smmu_init_one_queue(smmu, &smmu->priq.q,
+				       ARM_SMMU_PRIQ_PROD(smmu),
+				       ARM_SMMU_PRIQ_CONS(smmu),
+				       PRIQ_ENT_DWORDS);
 }
 
 static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu)
@@ -2360,8 +2378,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
 
 	/* Event queue */
 	writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
-	writel_relaxed(smmu->evtq.q.prod, smmu->base + ARM_SMMU_EVTQ_PROD);
-	writel_relaxed(smmu->evtq.q.cons, smmu->base + ARM_SMMU_EVTQ_CONS);
+	writel_relaxed(smmu->evtq.q.prod, smmu->base +
+		       ARM_SMMU_EVTQ_PROD(smmu));
+	writel_relaxed(smmu->evtq.q.cons, smmu->base +
+		       ARM_SMMU_EVTQ_CONS(smmu));
 
 	enables |= CR0_EVTQEN;
 	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
@@ -2376,9 +2396,9 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
 		writeq_relaxed(smmu->priq.q.q_base,
 			       smmu->base + ARM_SMMU_PRIQ_BASE);
 		writel_relaxed(smmu->priq.q.prod,
-			       smmu->base + ARM_SMMU_PRIQ_PROD);
+			       smmu->base + ARM_SMMU_PRIQ_PROD(smmu));
 		writel_relaxed(smmu->priq.q.cons,
-			       smmu->base + ARM_SMMU_PRIQ_CONS);
+			       smmu->base + ARM_SMMU_PRIQ_CONS(smmu));
 
 		enables |= CR0_PRIQEN;
 		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
-- 
1.9.1

[RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata.

[linucherian at gmail.com]

From: Linu Cherian <linu.cherian@cavium.com>

Cavium 99xx SMMU implementation doesn't support page 1 register space
and PAGE0_REGS_ONLY option will be enabled as an errata workaround.

This option when turned on, replaces all page 1 offsets used for
EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 05b4592..df9f27b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -176,15 +176,15 @@
 #define ARM_SMMU_CMDQ_CONS		0x9c
 
 #define ARM_SMMU_EVTQ_BASE		0xa0
-#define ARM_SMMU_EVTQ_PROD		0x100a8
-#define ARM_SMMU_EVTQ_CONS		0x100ac
+#define ARM_SMMU_EVTQ_PROD(s)	        (page1_offset_adjust(0x100a8, s))
+#define ARM_SMMU_EVTQ_CONS(s)		(page1_offset_adjust(0x100ac, s))
 #define ARM_SMMU_EVTQ_IRQ_CFG0		0xb0
 #define ARM_SMMU_EVTQ_IRQ_CFG1		0xb8
 #define ARM_SMMU_EVTQ_IRQ_CFG2		0xbc
 
 #define ARM_SMMU_PRIQ_BASE		0xc0
-#define ARM_SMMU_PRIQ_PROD		0x100c8
-#define ARM_SMMU_PRIQ_CONS		0x100cc
+#define ARM_SMMU_PRIQ_PROD(s)		(page1_offset_adjust(0x100c8, s))
+#define ARM_SMMU_PRIQ_CONS(s)		(page1_offset_adjust(0x100cc, s))
 #define ARM_SMMU_PRIQ_IRQ_CFG0		0xd0
 #define ARM_SMMU_PRIQ_IRQ_CFG1		0xd8
 #define ARM_SMMU_PRIQ_IRQ_CFG2		0xdc
@@ -412,6 +412,9 @@
 #define MSI_IOVA_BASE			0x8000000
 #define MSI_IOVA_LENGTH			0x100000
 
+#define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
+	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
+
 static bool disable_bypass;
 module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
@@ -597,6 +600,7 @@ struct arm_smmu_device {
 	u32				features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
+#define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
 	u32				options;
 
 	struct arm_smmu_cmdq		cmdq;
@@ -663,9 +667,19 @@ struct arm_smmu_option_prop {
 
 static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
+	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
 	{ 0, NULL},
 };
 
+static inline unsigned long page1_offset_adjust(
+	unsigned long off, struct arm_smmu_device *smmu)
+{
+	if (!ARM_SMMU_PAGE0_REGS_ONLY(smmu))
+		return off;
+	else
+		return (off - SZ_64K);
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct arm_smmu_domain, domain);
@@ -1983,8 +1997,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 		return ret;
 
 	/* evtq */
-	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q, ARM_SMMU_EVTQ_PROD,
-				      ARM_SMMU_EVTQ_CONS, EVTQ_ENT_DWORDS);
+	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q,
+				      ARM_SMMU_EVTQ_PROD(smmu),
+				      ARM_SMMU_EVTQ_CONS(smmu),
+				      EVTQ_ENT_DWORDS);
 	if (ret)
 		return ret;
 
@@ -1992,8 +2008,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 	if (!(smmu->features & ARM_SMMU_FEAT_PRI))
 		return 0;
 
-	return arm_smmu_init_one_queue(smmu, &smmu->priq.q, ARM_SMMU_PRIQ_PROD,
-				       ARM_SMMU_PRIQ_CONS, PRIQ_ENT_DWORDS);
+	return arm_smmu_init_one_queue(smmu, &smmu->priq.q,
+				       ARM_SMMU_PRIQ_PROD(smmu),
+				       ARM_SMMU_PRIQ_CONS(smmu),
+				       PRIQ_ENT_DWORDS);
 }
 
 static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu)
@@ -2360,8 +2378,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
 
 	/* Event queue */
 	writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
-	writel_relaxed(smmu->evtq.q.prod, smmu->base + ARM_SMMU_EVTQ_PROD);
-	writel_relaxed(smmu->evtq.q.cons, smmu->base + ARM_SMMU_EVTQ_CONS);
+	writel_relaxed(smmu->evtq.q.prod, smmu->base +
+		       ARM_SMMU_EVTQ_PROD(smmu));
+	writel_relaxed(smmu->evtq.q.cons, smmu->base +
+		       ARM_SMMU_EVTQ_CONS(smmu));
 
 	enables |= CR0_EVTQEN;
 	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
@@ -2376,9 +2396,9 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
 		writeq_relaxed(smmu->priq.q.q_base,
 			       smmu->base + ARM_SMMU_PRIQ_BASE);
 		writel_relaxed(smmu->priq.q.prod,
-			       smmu->base + ARM_SMMU_PRIQ_PROD);
+			       smmu->base + ARM_SMMU_PRIQ_PROD(smmu));
 		writel_relaxed(smmu->priq.q.cons,
-			       smmu->base + ARM_SMMU_PRIQ_CONS);
+			       smmu->base + ARM_SMMU_PRIQ_CONS(smmu));
 
 		enables |= CR0_PRIQEN;
 		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
-- 
1.9.1

[RFC PATCH 2/7] iommu/arm-smmu-v3: Do resource size checks based on smmu option PAGE0_REGS_ONLY

[linucherian]

From: Linu Cherian <linu.cherian@cavium.com>

With implementations supporting only page 0 of register space,
resource size can be 64k as well and hence perform size checks
based on smmu option PAGE0_REGS_ONLY.

For this, arm_smmu_device_dt_probe/acpi_probe has been moved before
platform_get_resource call, so that smmu options are set beforehand.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index df9f27b..b326195 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2669,6 +2669,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	return ret;
 }
 
+static unsigned long arm_smmu_resource_size(struct arm_smmu_device *smmu)
+{
+	if (ARM_SMMU_PAGE0_REGS_ONLY(smmu))
+		return SZ_64K;
+	else
+		return SZ_128K;
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;
@@ -2685,9 +2693,17 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	}
 	smmu->dev = dev;
 
+	if (dev->of_node) {
+		ret = arm_smmu_device_dt_probe(pdev, smmu);
+	} else {
+		ret = arm_smmu_device_acpi_probe(pdev, smmu);
+		if (ret == -ENODEV)
+			return ret;
+	}
+
 	/* Base address */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (resource_size(res) + 1 < SZ_128K) {
+	if (resource_size(res) + 1 < arm_smmu_resource_size(smmu)) {
 		dev_err(dev, "MMIO region too small (%pr)\n", res);
 		return -EINVAL;
 	}
@@ -2714,14 +2730,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	if (irq > 0)
 		smmu->gerr_irq = irq;
 
-	if (dev->of_node) {
-		ret = arm_smmu_device_dt_probe(pdev, smmu);
-	} else {
-		ret = arm_smmu_device_acpi_probe(pdev, smmu);
-		if (ret == -ENODEV)
-			return ret;
-	}
-
 	/* Set bypass mode according to firmware probing result */
 	bypass = !!ret;
 
-- 
1.9.1

[RFC PATCH 2/7] iommu/arm-smmu-v3: Do resource size checks based on smmu option PAGE0_REGS_ONLY

[linucherian at gmail.com]

From: Linu Cherian <linu.cherian@cavium.com>

With implementations supporting only page 0 of register space,
resource size can be 64k as well and hence perform size checks
based on smmu option PAGE0_REGS_ONLY.

For this, arm_smmu_device_dt_probe/acpi_probe has been moved before
platform_get_resource call, so that smmu options are set beforehand.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index df9f27b..b326195 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2669,6 +2669,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	return ret;
 }
 
+static unsigned long arm_smmu_resource_size(struct arm_smmu_device *smmu)
+{
+	if (ARM_SMMU_PAGE0_REGS_ONLY(smmu))
+		return SZ_64K;
+	else
+		return SZ_128K;
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;
@@ -2685,9 +2693,17 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	}
 	smmu->dev = dev;
 
+	if (dev->of_node) {
+		ret = arm_smmu_device_dt_probe(pdev, smmu);
+	} else {
+		ret = arm_smmu_device_acpi_probe(pdev, smmu);
+		if (ret == -ENODEV)
+			return ret;
+	}
+
 	/* Base address */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (resource_size(res) + 1 < SZ_128K) {
+	if (resource_size(res) + 1 < arm_smmu_resource_size(smmu)) {
 		dev_err(dev, "MMIO region too small (%pr)\n", res);
 		return -EINVAL;
 	}
@@ -2714,14 +2730,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	if (irq > 0)
 		smmu->gerr_irq = irq;
 
-	if (dev->of_node) {
-		ret = arm_smmu_device_dt_probe(pdev, smmu);
-	} else {
-		ret = arm_smmu_device_acpi_probe(pdev, smmu);
-		if (ret == -ENODEV)
-			return ret;
-	}
-
 	/* Set bypass mode according to firmware probing result */
 	bypass = !!ret;
 
-- 
1.9.1

[RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[linucherian]

From: Geetha <gakula@cavium.com>

Cavium 99xx SMMU implementation doesn't not support unique irq lines for
gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
irq line for all three interrupts.

Signed-off-by: Geetha Sowjanya <gakula@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index b326195..1475ad8 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -415,6 +415,9 @@
 #define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
 	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
 
+#define ARM_SMMU_USE_SHARED_IRQS(s)		\
+	((s)->options & ARM_SMMU_OPT_USE_SHARED_IRQS)
+
 static bool disable_bypass;
 module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
@@ -601,6 +604,7 @@ struct arm_smmu_device {
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
 #define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
+#define ARM_SMMU_OPT_USE_SHARED_IRQS    (1 << 2)
 	u32				options;
 
 	struct arm_smmu_cmdq		cmdq;
@@ -668,6 +672,7 @@ struct arm_smmu_option_prop {
 static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
 	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
+	{ ARM_SMMU_OPT_USE_SHARED_IRQS, "cavium-cn99xx,broken-unique-irqlines"},
 	{ 0, NULL},
 };
 
@@ -2237,6 +2242,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 {
 	int ret, irq;
 	u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
+	u32 irqflags = IRQF_ONESHOT | IRQF_SHARED;
 
 	/* Disable IRQs first */
 	ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
@@ -2251,9 +2257,11 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 	/* Request interrupt lines */
 	irq = smmu->evtq.q.irq;
 	if (irq) {
+		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
+			irqflags = IRQF_ONESHOT;
 		ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
 						arm_smmu_evtq_thread,
-						IRQF_ONESHOT,
+						irqflags,
 						"arm-smmu-v3-evtq", smmu);
 		if (ret < 0)
 			dev_warn(smmu->dev, "failed to enable evtq irq\n");
@@ -2261,8 +2269,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 
 	irq = smmu->cmdq.q.irq;
 	if (irq) {
+		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
+			irqflags = 0;
 		ret = devm_request_irq(smmu->dev, irq,
-				       arm_smmu_cmdq_sync_handler, 0,
+				       arm_smmu_cmdq_sync_handler, irqflags,
 				       "arm-smmu-v3-cmdq-sync", smmu);
 		if (ret < 0)
 			dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n");
@@ -2270,8 +2280,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 
 	irq = smmu->gerr_irq;
 	if (irq) {
+		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
+			irqflags = 0;
 		ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler,
-				       0, "arm-smmu-v3-gerror", smmu);
+				       irqflags, "arm-smmu-v3-gerror", smmu);
 		if (ret < 0)
 			dev_warn(smmu->dev, "failed to enable gerror irq\n");
 	}
-- 
1.9.1

[RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[linucherian at gmail.com]

From: Geetha <gakula@cavium.com>

Cavium 99xx SMMU implementation doesn't not support unique irq lines for
gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
irq line for all three interrupts.

Signed-off-by: Geetha Sowjanya <gakula@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index b326195..1475ad8 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -415,6 +415,9 @@
 #define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
 	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
 
+#define ARM_SMMU_USE_SHARED_IRQS(s)		\
+	((s)->options & ARM_SMMU_OPT_USE_SHARED_IRQS)
+
 static bool disable_bypass;
 module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
@@ -601,6 +604,7 @@ struct arm_smmu_device {
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
 #define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
+#define ARM_SMMU_OPT_USE_SHARED_IRQS    (1 << 2)
 	u32				options;
 
 	struct arm_smmu_cmdq		cmdq;
@@ -668,6 +672,7 @@ struct arm_smmu_option_prop {
 static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
 	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
+	{ ARM_SMMU_OPT_USE_SHARED_IRQS, "cavium-cn99xx,broken-unique-irqlines"},
 	{ 0, NULL},
 };
 
@@ -2237,6 +2242,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 {
 	int ret, irq;
 	u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
+	u32 irqflags = IRQF_ONESHOT | IRQF_SHARED;
 
 	/* Disable IRQs first */
 	ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
@@ -2251,9 +2257,11 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 	/* Request interrupt lines */
 	irq = smmu->evtq.q.irq;
 	if (irq) {
+		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
+			irqflags = IRQF_ONESHOT;
 		ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
 						arm_smmu_evtq_thread,
-						IRQF_ONESHOT,
+						irqflags,
 						"arm-smmu-v3-evtq", smmu);
 		if (ret < 0)
 			dev_warn(smmu->dev, "failed to enable evtq irq\n");
@@ -2261,8 +2269,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 
 	irq = smmu->cmdq.q.irq;
 	if (irq) {
+		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
+			irqflags = 0;
 		ret = devm_request_irq(smmu->dev, irq,
-				       arm_smmu_cmdq_sync_handler, 0,
+				       arm_smmu_cmdq_sync_handler, irqflags,
 				       "arm-smmu-v3-cmdq-sync", smmu);
 		if (ret < 0)
 			dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n");
@@ -2270,8 +2280,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 
 	irq = smmu->gerr_irq;
 	if (irq) {
+		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
+			irqflags = 0;
 		ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler,
-				       0, "arm-smmu-v3-gerror", smmu);
+				       irqflags, "arm-smmu-v3-gerror", smmu);
 		if (ret < 0)
 			dev_warn(smmu->dev, "failed to enable gerror irq\n");
 	}
-- 
1.9.1

[RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[linucherian]

From: Linu Cherian <linu.cherian@cavium.com>

Add SMMuV3 model definitions.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 include/acpi/actbl2.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 2b4af07..9db67d6 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -778,6 +778,11 @@ struct acpi_iort_smmu {
 #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002	/* ARM Corelink MMU-400 */
 #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003	/* ARM Corelink MMU-500 */
 
+#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
+#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
+#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
+#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
+
 /* Masks for Flags field above */
 
 #define ACPI_IORT_SMMU_DVM_SUPPORTED    (1)
-- 
1.9.1

[RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[linucherian at gmail.com]

From: Linu Cherian <linu.cherian@cavium.com>

Add SMMuV3 model definitions.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 include/acpi/actbl2.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 2b4af07..9db67d6 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -778,6 +778,11 @@ struct acpi_iort_smmu {
 #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002	/* ARM Corelink MMU-400 */
 #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003	/* ARM Corelink MMU-500 */
 
+#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
+#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
+#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
+#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
+
 /* Masks for Flags field above */
 
 #define ACPI_IORT_SMMU_DVM_SUPPORTED    (1)
-- 
1.9.1

[RFC PATCH 5/7] iommu/arm-smmu-v3: For ACPI based device probing, set relevant options for different SMMUv3 implementations.

[linucherian]

From: Linu Cherian <linu.cherian@cavium.com>

Enable SKIP_PREFETCH option for HiSilicon SMMUv3 model.
Enable PAGE0_REGS_ONLY and USE_SHARED_IRQS options for
Cavium 99xx SMMUv3 model.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1475ad8..00b48b4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2634,18 +2634,45 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 }
 
 #ifdef CONFIG_ACPI
+static int acpi_smmu_get_options(u32 model, struct arm_smmu_device *smmu)
+{
+	int ret = 0;
+
+	switch (model) {
+	case ACPI_IORT_SMMU_V3:
+	case ACPI_IORT_SMMU_CORELINK_MMU600:
+		break;
+	case ACPI_IORT_SMMU_V3_HISILICON:
+		smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
+		break;
+	case ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:
+		smmu->options |= (ARM_SMMU_OPT_PAGE0_REGS_ONLY |
+				  ARM_SMMU_OPT_USE_SHARED_IRQS);
+		break;
+	default:
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
 static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
 				      struct arm_smmu_device *smmu)
 {
 	struct acpi_iort_smmu_v3 *iort_smmu;
 	struct device *dev = smmu->dev;
 	struct acpi_iort_node *node;
+	int ret;
 
 	node = *(struct acpi_iort_node **)dev_get_platdata(dev);
 
 	/* Retrieve SMMUv3 specific data */
 	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
+	ret = acpi_smmu_get_options(iort_smmu->model, smmu);
+	if (ret < 0)
+		return ret;
+
 	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
-- 
1.9.1

[RFC PATCH 5/7] iommu/arm-smmu-v3: For ACPI based device probing, set relevant options for different SMMUv3 implementations.

[linucherian at gmail.com]

From: Linu Cherian <linu.cherian@cavium.com>

Enable SKIP_PREFETCH option for HiSilicon SMMUv3 model.
Enable PAGE0_REGS_ONLY and USE_SHARED_IRQS options for
Cavium 99xx SMMUv3 model.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/iommu/arm-smmu-v3.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1475ad8..00b48b4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2634,18 +2634,45 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 }
 
 #ifdef CONFIG_ACPI
+static int acpi_smmu_get_options(u32 model, struct arm_smmu_device *smmu)
+{
+	int ret = 0;
+
+	switch (model) {
+	case ACPI_IORT_SMMU_V3:
+	case ACPI_IORT_SMMU_CORELINK_MMU600:
+		break;
+	case ACPI_IORT_SMMU_V3_HISILICON:
+		smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
+		break;
+	case ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:
+		smmu->options |= (ARM_SMMU_OPT_PAGE0_REGS_ONLY |
+				  ARM_SMMU_OPT_USE_SHARED_IRQS);
+		break;
+	default:
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
 static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
 				      struct arm_smmu_device *smmu)
 {
 	struct acpi_iort_smmu_v3 *iort_smmu;
 	struct device *dev = smmu->dev;
 	struct acpi_iort_node *node;
+	int ret;
 
 	node = *(struct acpi_iort_node **)dev_get_platdata(dev);
 
 	/* Retrieve SMMUv3 specific data */
 	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
+	ret = acpi_smmu_get_options(iort_smmu->model, smmu);
+	if (ret < 0)
+		return ret;
+
 	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
-- 
1.9.1

[RFC PATCH 6/7] ACPI/IORT: Fixup SMMUv3 resource size for Cavium 99xx SMMUv3 model.

[linucherian]

From: Linu Cherian <linu.cherian@cavium.com>

Cavium 99xx implementation doesn't support second page in SMMU
register space. Hence, resource size is set as 64k for this model.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/acpi/arm64/iort.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 22e08d2..ac22280 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -765,12 +765,20 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
 {
 	struct acpi_iort_smmu_v3 *smmu;
 	int num_res = 0;
+	unsigned long size = SZ_128K;
 
 	/* Retrieve SMMUv3 specific data */
 	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
+	/*
+	 * Override the size, for Cavium CN99xx implementations
+	 * which doesn't support the page 1 SMMU register space.
+	 */
+	if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
+		size = SZ_64K;
+
 	res[num_res].start = smmu->base_address;
-	res[num_res].end = smmu->base_address + SZ_128K - 1;
+	res[num_res].end = smmu->base_address + size - 1;
 	res[num_res].flags = IORESOURCE_MEM;
 
 	num_res++;
-- 
1.9.1

[RFC PATCH 6/7] ACPI/IORT: Fixup SMMUv3 resource size for Cavium 99xx SMMUv3 model.

[linucherian at gmail.com]

From: Linu Cherian <linu.cherian@cavium.com>

Cavium 99xx implementation doesn't support second page in SMMU
register space. Hence, resource size is set as 64k for this model.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 drivers/acpi/arm64/iort.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 22e08d2..ac22280 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -765,12 +765,20 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
 {
 	struct acpi_iort_smmu_v3 *smmu;
 	int num_res = 0;
+	unsigned long size = SZ_128K;
 
 	/* Retrieve SMMUv3 specific data */
 	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
+	/*
+	 * Override the size, for Cavium CN99xx implementations
+	 * which doesn't support the page 1 SMMU register space.
+	 */
+	if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
+		size = SZ_64K;
+
 	res[num_res].start = smmu->base_address;
-	res[num_res].end = smmu->base_address + SZ_128K - 1;
+	res[num_res].end = smmu->base_address + size - 1;
 	res[num_res].flags = IORESOURCE_MEM;
 
 	num_res++;
-- 
1.9.1

[RFC PATCH 7/7] arm64: Documentation: Add Cavium ThunderX2 SMMUv3 erratas.

[linucherian]

From: Linu Cherian <linu.cherian@cavium.com>

Add Cavium ThunderX2 SMMUv3 erratas to the errata list.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 Documentation/arm64/silicon-errata.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 10f2ddd..42422f6 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -62,6 +62,8 @@ stable kernels.
 | Cavium         | ThunderX GICv3  | #23154          | CAVIUM_ERRATUM_23154        |
 | Cavium         | ThunderX Core   | #27456          | CAVIUM_ERRATUM_27456        |
 | Cavium         | ThunderX SMMUv2 | #27704          | N/A                         |
+| Cavium         | ThunderX2 SMMUv3| #74             | N/A                         |
+| Cavium         | ThunderX2 SMMUv3| #126            | N/A                         |
 |                |                 |                 |                             |
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585         |
 |                |                 |                 |                             |
-- 
1.9.1

[RFC PATCH 7/7] arm64: Documentation: Add Cavium ThunderX2 SMMUv3 erratas.

[linucherian at gmail.com]

From: Linu Cherian <linu.cherian@cavium.com>

Add Cavium ThunderX2 SMMUv3 erratas to the errata list.

Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
---
 Documentation/arm64/silicon-errata.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 10f2ddd..42422f6 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -62,6 +62,8 @@ stable kernels.
 | Cavium         | ThunderX GICv3  | #23154          | CAVIUM_ERRATUM_23154        |
 | Cavium         | ThunderX Core   | #27456          | CAVIUM_ERRATUM_27456        |
 | Cavium         | ThunderX SMMUv2 | #27704          | N/A                         |
+| Cavium         | ThunderX2 SMMUv3| #74             | N/A                         |
+| Cavium         | ThunderX2 SMMUv3| #126            | N/A                         |
 |                |                 |                 |                             |
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585         |
 |                |                 |                 |                             |
-- 
1.9.1

Re: [RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata.

[Robin Murphy]

On 11/04/17 15:42, linucherian@gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Cavium 99xx SMMU implementation doesn't support page 1 register space
> and PAGE0_REGS_ONLY option will be enabled as an errata workaround.

Ugh :(

> This option when turned on, replaces all page 1 offsets used for
> EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.

I think it might be neater to have something like:

arm_smmu_page1(smmu) {
	if (smmu->quirk)
		return smmu->base;
	return smmu->base + 64k;
}

and use it as the base in the appropriate places, rather than override
the individual registers. Much like ARM_SMMU_GR0_NS in the SMMUv2 driver.

Robin.

> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 44 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 05b4592..df9f27b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -176,15 +176,15 @@
>  #define ARM_SMMU_CMDQ_CONS		0x9c
>  
>  #define ARM_SMMU_EVTQ_BASE		0xa0
> -#define ARM_SMMU_EVTQ_PROD		0x100a8
> -#define ARM_SMMU_EVTQ_CONS		0x100ac
> +#define ARM_SMMU_EVTQ_PROD(s)	        (page1_offset_adjust(0x100a8, s))
> +#define ARM_SMMU_EVTQ_CONS(s)		(page1_offset_adjust(0x100ac, s))
>  #define ARM_SMMU_EVTQ_IRQ_CFG0		0xb0
>  #define ARM_SMMU_EVTQ_IRQ_CFG1		0xb8
>  #define ARM_SMMU_EVTQ_IRQ_CFG2		0xbc
>  
>  #define ARM_SMMU_PRIQ_BASE		0xc0
> -#define ARM_SMMU_PRIQ_PROD		0x100c8
> -#define ARM_SMMU_PRIQ_CONS		0x100cc
> +#define ARM_SMMU_PRIQ_PROD(s)		(page1_offset_adjust(0x100c8, s))
> +#define ARM_SMMU_PRIQ_CONS(s)		(page1_offset_adjust(0x100cc, s))
>  #define ARM_SMMU_PRIQ_IRQ_CFG0		0xd0
>  #define ARM_SMMU_PRIQ_IRQ_CFG1		0xd8
>  #define ARM_SMMU_PRIQ_IRQ_CFG2		0xdc
> @@ -412,6 +412,9 @@
>  #define MSI_IOVA_BASE			0x8000000
>  #define MSI_IOVA_LENGTH			0x100000
>  
> +#define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
> +	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
> +
>  static bool disable_bypass;
>  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
> @@ -597,6 +600,7 @@ struct arm_smmu_device {
>  	u32				features;
>  
>  #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
> +#define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
>  	u32				options;
>  
>  	struct arm_smmu_cmdq		cmdq;
> @@ -663,9 +667,19 @@ struct arm_smmu_option_prop {
>  
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>  	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
> +	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
>  	{ 0, NULL},
>  };
>  
> +static inline unsigned long page1_offset_adjust(
> +	unsigned long off, struct arm_smmu_device *smmu)
> +{
> +	if (!ARM_SMMU_PAGE0_REGS_ONLY(smmu))
> +		return off;
> +	else
> +		return (off - SZ_64K);
> +}
> +
>  static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>  {
>  	return container_of(dom, struct arm_smmu_domain, domain);
> @@ -1983,8 +1997,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
>  		return ret;
>  
>  	/* evtq */
> -	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q, ARM_SMMU_EVTQ_PROD,
> -				      ARM_SMMU_EVTQ_CONS, EVTQ_ENT_DWORDS);
> +	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q,
> +				      ARM_SMMU_EVTQ_PROD(smmu),
> +				      ARM_SMMU_EVTQ_CONS(smmu),
> +				      EVTQ_ENT_DWORDS);
>  	if (ret)
>  		return ret;
>  
> @@ -1992,8 +2008,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
>  	if (!(smmu->features & ARM_SMMU_FEAT_PRI))
>  		return 0;
>  
> -	return arm_smmu_init_one_queue(smmu, &smmu->priq.q, ARM_SMMU_PRIQ_PROD,
> -				       ARM_SMMU_PRIQ_CONS, PRIQ_ENT_DWORDS);
> +	return arm_smmu_init_one_queue(smmu, &smmu->priq.q,
> +				       ARM_SMMU_PRIQ_PROD(smmu),
> +				       ARM_SMMU_PRIQ_CONS(smmu),
> +				       PRIQ_ENT_DWORDS);
>  }
>  
>  static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu)
> @@ -2360,8 +2378,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
>  
>  	/* Event queue */
>  	writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
> -	writel_relaxed(smmu->evtq.q.prod, smmu->base + ARM_SMMU_EVTQ_PROD);
> -	writel_relaxed(smmu->evtq.q.cons, smmu->base + ARM_SMMU_EVTQ_CONS);
> +	writel_relaxed(smmu->evtq.q.prod, smmu->base +
> +		       ARM_SMMU_EVTQ_PROD(smmu));
> +	writel_relaxed(smmu->evtq.q.cons, smmu->base +
> +		       ARM_SMMU_EVTQ_CONS(smmu));
>  
>  	enables |= CR0_EVTQEN;
>  	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
> @@ -2376,9 +2396,9 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
>  		writeq_relaxed(smmu->priq.q.q_base,
>  			       smmu->base + ARM_SMMU_PRIQ_BASE);
>  		writel_relaxed(smmu->priq.q.prod,
> -			       smmu->base + ARM_SMMU_PRIQ_PROD);
> +			       smmu->base + ARM_SMMU_PRIQ_PROD(smmu));
>  		writel_relaxed(smmu->priq.q.cons,
> -			       smmu->base + ARM_SMMU_PRIQ_CONS);
> +			       smmu->base + ARM_SMMU_PRIQ_CONS(smmu));
>  
>  		enables |= CR0_PRIQEN;
>  		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
> 

[RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata.

[Robin Murphy]

On 11/04/17 15:42, linucherian at gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Cavium 99xx SMMU implementation doesn't support page 1 register space
> and PAGE0_REGS_ONLY option will be enabled as an errata workaround.

Ugh :(

> This option when turned on, replaces all page 1 offsets used for
> EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.

I think it might be neater to have something like:

arm_smmu_page1(smmu) {
	if (smmu->quirk)
		return smmu->base;
	return smmu->base + 64k;
}

and use it as the base in the appropriate places, rather than override
the individual registers. Much like ARM_SMMU_GR0_NS in the SMMUv2 driver.

Robin.

> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 44 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 05b4592..df9f27b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -176,15 +176,15 @@
>  #define ARM_SMMU_CMDQ_CONS		0x9c
>  
>  #define ARM_SMMU_EVTQ_BASE		0xa0
> -#define ARM_SMMU_EVTQ_PROD		0x100a8
> -#define ARM_SMMU_EVTQ_CONS		0x100ac
> +#define ARM_SMMU_EVTQ_PROD(s)	        (page1_offset_adjust(0x100a8, s))
> +#define ARM_SMMU_EVTQ_CONS(s)		(page1_offset_adjust(0x100ac, s))
>  #define ARM_SMMU_EVTQ_IRQ_CFG0		0xb0
>  #define ARM_SMMU_EVTQ_IRQ_CFG1		0xb8
>  #define ARM_SMMU_EVTQ_IRQ_CFG2		0xbc
>  
>  #define ARM_SMMU_PRIQ_BASE		0xc0
> -#define ARM_SMMU_PRIQ_PROD		0x100c8
> -#define ARM_SMMU_PRIQ_CONS		0x100cc
> +#define ARM_SMMU_PRIQ_PROD(s)		(page1_offset_adjust(0x100c8, s))
> +#define ARM_SMMU_PRIQ_CONS(s)		(page1_offset_adjust(0x100cc, s))
>  #define ARM_SMMU_PRIQ_IRQ_CFG0		0xd0
>  #define ARM_SMMU_PRIQ_IRQ_CFG1		0xd8
>  #define ARM_SMMU_PRIQ_IRQ_CFG2		0xdc
> @@ -412,6 +412,9 @@
>  #define MSI_IOVA_BASE			0x8000000
>  #define MSI_IOVA_LENGTH			0x100000
>  
> +#define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
> +	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
> +
>  static bool disable_bypass;
>  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
> @@ -597,6 +600,7 @@ struct arm_smmu_device {
>  	u32				features;
>  
>  #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
> +#define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
>  	u32				options;
>  
>  	struct arm_smmu_cmdq		cmdq;
> @@ -663,9 +667,19 @@ struct arm_smmu_option_prop {
>  
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>  	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
> +	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
>  	{ 0, NULL},
>  };
>  
> +static inline unsigned long page1_offset_adjust(
> +	unsigned long off, struct arm_smmu_device *smmu)
> +{
> +	if (!ARM_SMMU_PAGE0_REGS_ONLY(smmu))
> +		return off;
> +	else
> +		return (off - SZ_64K);
> +}
> +
>  static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>  {
>  	return container_of(dom, struct arm_smmu_domain, domain);
> @@ -1983,8 +1997,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
>  		return ret;
>  
>  	/* evtq */
> -	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q, ARM_SMMU_EVTQ_PROD,
> -				      ARM_SMMU_EVTQ_CONS, EVTQ_ENT_DWORDS);
> +	ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q,
> +				      ARM_SMMU_EVTQ_PROD(smmu),
> +				      ARM_SMMU_EVTQ_CONS(smmu),
> +				      EVTQ_ENT_DWORDS);
>  	if (ret)
>  		return ret;
>  
> @@ -1992,8 +2008,10 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
>  	if (!(smmu->features & ARM_SMMU_FEAT_PRI))
>  		return 0;
>  
> -	return arm_smmu_init_one_queue(smmu, &smmu->priq.q, ARM_SMMU_PRIQ_PROD,
> -				       ARM_SMMU_PRIQ_CONS, PRIQ_ENT_DWORDS);
> +	return arm_smmu_init_one_queue(smmu, &smmu->priq.q,
> +				       ARM_SMMU_PRIQ_PROD(smmu),
> +				       ARM_SMMU_PRIQ_CONS(smmu),
> +				       PRIQ_ENT_DWORDS);
>  }
>  
>  static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu)
> @@ -2360,8 +2378,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
>  
>  	/* Event queue */
>  	writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
> -	writel_relaxed(smmu->evtq.q.prod, smmu->base + ARM_SMMU_EVTQ_PROD);
> -	writel_relaxed(smmu->evtq.q.cons, smmu->base + ARM_SMMU_EVTQ_CONS);
> +	writel_relaxed(smmu->evtq.q.prod, smmu->base +
> +		       ARM_SMMU_EVTQ_PROD(smmu));
> +	writel_relaxed(smmu->evtq.q.cons, smmu->base +
> +		       ARM_SMMU_EVTQ_CONS(smmu));
>  
>  	enables |= CR0_EVTQEN;
>  	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
> @@ -2376,9 +2396,9 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
>  		writeq_relaxed(smmu->priq.q.q_base,
>  			       smmu->base + ARM_SMMU_PRIQ_BASE);
>  		writel_relaxed(smmu->priq.q.prod,
> -			       smmu->base + ARM_SMMU_PRIQ_PROD);
> +			       smmu->base + ARM_SMMU_PRIQ_PROD(smmu));
>  		writel_relaxed(smmu->priq.q.cons,
> -			       smmu->base + ARM_SMMU_PRIQ_CONS);
> +			       smmu->base + ARM_SMMU_PRIQ_CONS(smmu));
>  
>  		enables |= CR0_PRIQEN;
>  		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
> 

Re: [RFC PATCH 2/7] iommu/arm-smmu-v3: Do resource size checks based on smmu option PAGE0_REGS_ONLY

[Robin Murphy]

On 11/04/17 15:42, linucherian@gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> With implementations supporting only page 0 of register space,
> resource size can be 64k as well and hence perform size checks
> based on smmu option PAGE0_REGS_ONLY.

What harm comes of mapping page 1 if we don't access it?

Robin.

> For this, arm_smmu_device_dt_probe/acpi_probe has been moved before
> platform_get_resource call, so that smmu options are set beforehand.
> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index df9f27b..b326195 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2669,6 +2669,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  	return ret;
>  }
>  
> +static unsigned long arm_smmu_resource_size(struct arm_smmu_device *smmu)
> +{
> +	if (ARM_SMMU_PAGE0_REGS_ONLY(smmu))
> +		return SZ_64K;
> +	else
> +		return SZ_128K;
> +}
> +
>  static int arm_smmu_device_probe(struct platform_device *pdev)
>  {
>  	int irq, ret;
> @@ -2685,9 +2693,17 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	}
>  	smmu->dev = dev;
>  
> +	if (dev->of_node) {
> +		ret = arm_smmu_device_dt_probe(pdev, smmu);
> +	} else {
> +		ret = arm_smmu_device_acpi_probe(pdev, smmu);
> +		if (ret == -ENODEV)
> +			return ret;
> +	}
> +
>  	/* Base address */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (resource_size(res) + 1 < SZ_128K) {
> +	if (resource_size(res) + 1 < arm_smmu_resource_size(smmu)) {
>  		dev_err(dev, "MMIO region too small (%pr)\n", res);
>  		return -EINVAL;
>  	}
> @@ -2714,14 +2730,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	if (irq > 0)
>  		smmu->gerr_irq = irq;
>  
> -	if (dev->of_node) {
> -		ret = arm_smmu_device_dt_probe(pdev, smmu);
> -	} else {
> -		ret = arm_smmu_device_acpi_probe(pdev, smmu);
> -		if (ret == -ENODEV)
> -			return ret;
> -	}
> -
>  	/* Set bypass mode according to firmware probing result */
>  	bypass = !!ret;
>  
> 

[RFC PATCH 2/7] iommu/arm-smmu-v3: Do resource size checks based on smmu option PAGE0_REGS_ONLY

[Robin Murphy]

On 11/04/17 15:42, linucherian at gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> With implementations supporting only page 0 of register space,
> resource size can be 64k as well and hence perform size checks
> based on smmu option PAGE0_REGS_ONLY.

What harm comes of mapping page 1 if we don't access it?

Robin.

> For this, arm_smmu_device_dt_probe/acpi_probe has been moved before
> platform_get_resource call, so that smmu options are set beforehand.
> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index df9f27b..b326195 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2669,6 +2669,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  	return ret;
>  }
>  
> +static unsigned long arm_smmu_resource_size(struct arm_smmu_device *smmu)
> +{
> +	if (ARM_SMMU_PAGE0_REGS_ONLY(smmu))
> +		return SZ_64K;
> +	else
> +		return SZ_128K;
> +}
> +
>  static int arm_smmu_device_probe(struct platform_device *pdev)
>  {
>  	int irq, ret;
> @@ -2685,9 +2693,17 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	}
>  	smmu->dev = dev;
>  
> +	if (dev->of_node) {
> +		ret = arm_smmu_device_dt_probe(pdev, smmu);
> +	} else {
> +		ret = arm_smmu_device_acpi_probe(pdev, smmu);
> +		if (ret == -ENODEV)
> +			return ret;
> +	}
> +
>  	/* Base address */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (resource_size(res) + 1 < SZ_128K) {
> +	if (resource_size(res) + 1 < arm_smmu_resource_size(smmu)) {
>  		dev_err(dev, "MMIO region too small (%pr)\n", res);
>  		return -EINVAL;
>  	}
> @@ -2714,14 +2730,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	if (irq > 0)
>  		smmu->gerr_irq = irq;
>  
> -	if (dev->of_node) {
> -		ret = arm_smmu_device_dt_probe(pdev, smmu);
> -	} else {
> -		ret = arm_smmu_device_acpi_probe(pdev, smmu);
> -		if (ret == -ENODEV)
> -			return ret;
> -	}
> -
>  	/* Set bypass mode according to firmware probing result */
>  	bypass = !!ret;
>  
> 

Re: [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Robin Murphy]

On 11/04/17 15:42, linucherian@gmail.com wrote:
> From: Geetha <gakula@cavium.com>
> 
> Cavium 99xx SMMU implementation doesn't not support unique irq lines for
> gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
> irq line for all three interrupts.

AFAICS, there's nothing actually wrong with using shared wired IRQs -
the architecture spec doesn't appear to say anything about it. I think
it might suffice to simply add IRQF_SHARED if we can see the SMMU
doesn't support MSIs anyway - it doesn't really seem like something we
need to treat as a specific quirk.

> Signed-off-by: Geetha Sowjanya <gakula@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index b326195..1475ad8 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -415,6 +415,9 @@
>  #define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
>  	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
>  
> +#define ARM_SMMU_USE_SHARED_IRQS(s)		\
> +	((s)->options & ARM_SMMU_OPT_USE_SHARED_IRQS)
> +
>  static bool disable_bypass;
>  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
> @@ -601,6 +604,7 @@ struct arm_smmu_device {
>  
>  #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
>  #define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
> +#define ARM_SMMU_OPT_USE_SHARED_IRQS    (1 << 2)
>  	u32				options;
>  
>  	struct arm_smmu_cmdq		cmdq;
> @@ -668,6 +672,7 @@ struct arm_smmu_option_prop {
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>  	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
>  	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
> +	{ ARM_SMMU_OPT_USE_SHARED_IRQS, "cavium-cn99xx,broken-unique-irqlines"},
>  	{ 0, NULL},
>  };
>  
> @@ -2237,6 +2242,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  {
>  	int ret, irq;
>  	u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
> +	u32 irqflags = IRQF_ONESHOT | IRQF_SHARED;

Either way, this is a really ugly way to go about it - I'd much rather
initialise the common base value:

	u32 irqflags = (definitely not an MSI) ? IRQF_SHARED : 0;

>  
>  	/* Disable IRQs first */
>  	ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
> @@ -2251,9 +2257,11 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  	/* Request interrupt lines */
>  	irq = smmu->evtq.q.irq;
>  	if (irq) {
> +		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
> +			irqflags = IRQF_ONESHOT;
>  		ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
>  						arm_smmu_evtq_thread,
> -						IRQF_ONESHOT,

...and just pass irqflags | IRQF_ONESHOT here (and irqflags elsewhere),
without all the horrible copy-paste conditions.

Robin.

> +						irqflags,
>  						"arm-smmu-v3-evtq", smmu);
>  		if (ret < 0)
>  			dev_warn(smmu->dev, "failed to enable evtq irq\n");
> @@ -2261,8 +2269,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  
>  	irq = smmu->cmdq.q.irq;
>  	if (irq) {
> +		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
> +			irqflags = 0;
>  		ret = devm_request_irq(smmu->dev, irq,
> -				       arm_smmu_cmdq_sync_handler, 0,
> +				       arm_smmu_cmdq_sync_handler, irqflags,
>  				       "arm-smmu-v3-cmdq-sync", smmu);
>  		if (ret < 0)
>  			dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n");
> @@ -2270,8 +2280,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  
>  	irq = smmu->gerr_irq;
>  	if (irq) {
> +		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
> +			irqflags = 0;
>  		ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler,
> -				       0, "arm-smmu-v3-gerror", smmu);
> +				       irqflags, "arm-smmu-v3-gerror", smmu);
>  		if (ret < 0)
>  			dev_warn(smmu->dev, "failed to enable gerror irq\n");
>  	}
> 

[RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Robin Murphy]

On 11/04/17 15:42, linucherian at gmail.com wrote:
> From: Geetha <gakula@cavium.com>
> 
> Cavium 99xx SMMU implementation doesn't not support unique irq lines for
> gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
> irq line for all three interrupts.

AFAICS, there's nothing actually wrong with using shared wired IRQs -
the architecture spec doesn't appear to say anything about it. I think
it might suffice to simply add IRQF_SHARED if we can see the SMMU
doesn't support MSIs anyway - it doesn't really seem like something we
need to treat as a specific quirk.

> Signed-off-by: Geetha Sowjanya <gakula@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index b326195..1475ad8 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -415,6 +415,9 @@
>  #define ARM_SMMU_PAGE0_REGS_ONLY(s)		\
>  	((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
>  
> +#define ARM_SMMU_USE_SHARED_IRQS(s)		\
> +	((s)->options & ARM_SMMU_OPT_USE_SHARED_IRQS)
> +
>  static bool disable_bypass;
>  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
> @@ -601,6 +604,7 @@ struct arm_smmu_device {
>  
>  #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
>  #define ARM_SMMU_OPT_PAGE0_REGS_ONLY    (1 << 1)
> +#define ARM_SMMU_OPT_USE_SHARED_IRQS    (1 << 2)
>  	u32				options;
>  
>  	struct arm_smmu_cmdq		cmdq;
> @@ -668,6 +672,7 @@ struct arm_smmu_option_prop {
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>  	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
>  	{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
> +	{ ARM_SMMU_OPT_USE_SHARED_IRQS, "cavium-cn99xx,broken-unique-irqlines"},
>  	{ 0, NULL},
>  };
>  
> @@ -2237,6 +2242,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  {
>  	int ret, irq;
>  	u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
> +	u32 irqflags = IRQF_ONESHOT | IRQF_SHARED;

Either way, this is a really ugly way to go about it - I'd much rather
initialise the common base value:

	u32 irqflags = (definitely not an MSI) ? IRQF_SHARED : 0;

>  
>  	/* Disable IRQs first */
>  	ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
> @@ -2251,9 +2257,11 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  	/* Request interrupt lines */
>  	irq = smmu->evtq.q.irq;
>  	if (irq) {
> +		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
> +			irqflags = IRQF_ONESHOT;
>  		ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
>  						arm_smmu_evtq_thread,
> -						IRQF_ONESHOT,

...and just pass irqflags | IRQF_ONESHOT here (and irqflags elsewhere),
without all the horrible copy-paste conditions.

Robin.

> +						irqflags,
>  						"arm-smmu-v3-evtq", smmu);
>  		if (ret < 0)
>  			dev_warn(smmu->dev, "failed to enable evtq irq\n");
> @@ -2261,8 +2269,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  
>  	irq = smmu->cmdq.q.irq;
>  	if (irq) {
> +		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
> +			irqflags = 0;
>  		ret = devm_request_irq(smmu->dev, irq,
> -				       arm_smmu_cmdq_sync_handler, 0,
> +				       arm_smmu_cmdq_sync_handler, irqflags,
>  				       "arm-smmu-v3-cmdq-sync", smmu);
>  		if (ret < 0)
>  			dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n");
> @@ -2270,8 +2280,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
>  
>  	irq = smmu->gerr_irq;
>  	if (irq) {
> +		if (!ARM_SMMU_USE_SHARED_IRQS(smmu))
> +			irqflags = 0;
>  		ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler,
> -				       0, "arm-smmu-v3-gerror", smmu);
> +				       irqflags, "arm-smmu-v3-gerror", smmu);
>  		if (ret < 0)
>  			dev_warn(smmu->dev, "failed to enable gerror irq\n");
>  	}
> 

Re: [RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Robin Murphy]

On 11/04/17 15:42, linucherian@gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Add SMMuV3 model definitions.
> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  include/acpi/actbl2.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 2b4af07..9db67d6 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002	/* ARM Corelink MMU-400 */
>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003	/* ARM Corelink MMU-500 */
>  
> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */

None of those models are listed in the current IORT spec.

What's MMU-600?

Robin.

> +
>  /* Masks for Flags field above */
>  
>  #define ACPI_IORT_SMMU_DVM_SUPPORTED    (1)
> 

[RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Robin Murphy]

On 11/04/17 15:42, linucherian at gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Add SMMuV3 model definitions.
> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  include/acpi/actbl2.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 2b4af07..9db67d6 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002	/* ARM Corelink MMU-400 */
>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003	/* ARM Corelink MMU-500 */
>  
> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */

None of those models are listed in the current IORT spec.

What's MMU-600?

Robin.

> +
>  /* Masks for Flags field above */
>  
>  #define ACPI_IORT_SMMU_DVM_SUPPORTED    (1)
> 

Re: [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Will Deacon]

On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
> On 11/04/17 15:42, linucherian@gmail.com wrote:
> > From: Geetha <gakula@cavium.com>
> > 
> > Cavium 99xx SMMU implementation doesn't not support unique irq lines for
> > gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
> > irq line for all three interrupts.
> 
> AFAICS, there's nothing actually wrong with using shared wired IRQs -
> the architecture spec doesn't appear to say anything about it. I think
> it might suffice to simply add IRQF_SHARED if we can see the SMMU
> doesn't support MSIs anyway - it doesn't really seem like something we
> need to treat as a specific quirk.

No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
where it's clear that each source asserts a *unique* wired interrupt.

Geetha: does your implementation support MSIs?

Will

[RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Will Deacon]

On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
> On 11/04/17 15:42, linucherian at gmail.com wrote:
> > From: Geetha <gakula@cavium.com>
> > 
> > Cavium 99xx SMMU implementation doesn't not support unique irq lines for
> > gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
> > irq line for all three interrupts.
> 
> AFAICS, there's nothing actually wrong with using shared wired IRQs -
> the architecture spec doesn't appear to say anything about it. I think
> it might suffice to simply add IRQF_SHARED if we can see the SMMU
> doesn't support MSIs anyway - it doesn't really seem like something we
> need to treat as a specific quirk.

No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
where it's clear that each source asserts a *unique* wired interrupt.

Geetha: does your implementation support MSIs?

Will

Re: [RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds

[Will Deacon]

On Tue, Apr 11, 2017 at 08:12:38PM +0530, linucherian@gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Cavium CN99xx SMMUv3 implementation has two Silicon Erratas.
> 1. Errata ID #74
>    SMMU register alias Page 1 is not implemented
> 2. Errata ID #126
>    SMMU doesnt support unique IRQ lines for gerror, eventq and cmdq-sync

Is this device in production, or just part of a test chip?

Will

[RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds

[Will Deacon]

On Tue, Apr 11, 2017 at 08:12:38PM +0530, linucherian at gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Cavium CN99xx SMMUv3 implementation has two Silicon Erratas.
> 1. Errata ID #74
>    SMMU register alias Page 1 is not implemented
> 2. Errata ID #126
>    SMMU doesnt support unique IRQ lines for gerror, eventq and cmdq-sync

Is this device in production, or just part of a test chip?

Will

Re: [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Sunil Kovvuri]

On Tue, Apr 11, 2017 at 9:51 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
>> On 11/04/17 15:42, linucherian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> > From: Geetha <gakula-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> >
>> > Cavium 99xx SMMU implementation doesn't not support unique irq lines for
>> > gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
>> > irq line for all three interrupts.
>>
>> AFAICS, there's nothing actually wrong with using shared wired IRQs -
>> the architecture spec doesn't appear to say anything about it. I think
>> it might suffice to simply add IRQF_SHARED if we can see the SMMU
>> doesn't support MSIs anyway - it doesn't really seem like something we
>> need to treat as a specific quirk.
>
> No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
> where it's clear that each source asserts a *unique* wired interrupt.
>
> Geetha: does your implementation support MSIs?
>
> Will

No, this silicon doesn't support MSIs.

Thanks,
Sunil.

[RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Sunil Kovvuri]

On Tue, Apr 11, 2017 at 9:51 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
>> On 11/04/17 15:42, linucherian at gmail.com wrote:
>> > From: Geetha <gakula@cavium.com>
>> >
>> > Cavium 99xx SMMU implementation doesn't not support unique irq lines for
>> > gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
>> > irq line for all three interrupts.
>>
>> AFAICS, there's nothing actually wrong with using shared wired IRQs -
>> the architecture spec doesn't appear to say anything about it. I think
>> it might suffice to simply add IRQF_SHARED if we can see the SMMU
>> doesn't support MSIs anyway - it doesn't really seem like something we
>> need to treat as a specific quirk.
>
> No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
> where it's clear that each source asserts a *unique* wired interrupt.
>
> Geetha: does your implementation support MSIs?
>
> Will

No, this silicon doesn't support MSIs.

Thanks,
Sunil.

Re: [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Robin Murphy]

On 11/04/17 17:21, Will Deacon wrote:
> On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
>> On 11/04/17 15:42, linucherian@gmail.com wrote:
>>> From: Geetha <gakula@cavium.com>
>>>
>>> Cavium 99xx SMMU implementation doesn't not support unique irq lines for
>>> gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
>>> irq line for all three interrupts.
>>
>> AFAICS, there's nothing actually wrong with using shared wired IRQs -
>> the architecture spec doesn't appear to say anything about it. I think
>> it might suffice to simply add IRQF_SHARED if we can see the SMMU
>> doesn't support MSIs anyway - it doesn't really seem like something we
>> need to treat as a specific quirk.
> 
> No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
> where it's clear that each source asserts a *unique* wired interrupt.

Perhaps I'm reading it too generously; it does indeed specify that the
*implementation* has to provide a unique output for each source, but
other than suggesting a particular mode of operation based on that I
don't see anything actually forbidding the *integration* from then just
munging those lines together externally, as integrators so often like to
do. That's the case I had in mind.

Robin.

> 
> Geetha: does your implementation support MSIs?
> 
> Will
> 

[RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Robin Murphy]

On 11/04/17 17:21, Will Deacon wrote:
> On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
>> On 11/04/17 15:42, linucherian at gmail.com wrote:
>>> From: Geetha <gakula@cavium.com>
>>>
>>> Cavium 99xx SMMU implementation doesn't not support unique irq lines for
>>> gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
>>> irq line for all three interrupts.
>>
>> AFAICS, there's nothing actually wrong with using shared wired IRQs -
>> the architecture spec doesn't appear to say anything about it. I think
>> it might suffice to simply add IRQF_SHARED if we can see the SMMU
>> doesn't support MSIs anyway - it doesn't really seem like something we
>> need to treat as a specific quirk.
> 
> No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
> where it's clear that each source asserts a *unique* wired interrupt.

Perhaps I'm reading it too generously; it does indeed specify that the
*implementation* has to provide a unique output for each source, but
other than suggesting a particular mode of operation based on that I
don't see anything actually forbidding the *integration* from then just
munging those lines together externally, as integrators so often like to
do. That's the case I had in mind.

Robin.

> 
> Geetha: does your implementation support MSIs?
> 
> Will
> 

Re: [RFC PATCH 2/7] iommu/arm-smmu-v3: Do resource size checks based on smmu option PAGE0_REGS_ONLY

[Sunil Kovvuri]

On Tue, Apr 11, 2017 at 9:13 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 11/04/17 15:42, linucherian@gmail.com wrote:
>> From: Linu Cherian <linu.cherian@cavium.com>
>>
>> With implementations supporting only page 0 of register space,
>> resource size can be 64k as well and hence perform size checks
>> based on smmu option PAGE0_REGS_ONLY.
>
> What harm comes of mapping page 1 if we don't access it?
>
> Robin.
>

There are multiple SMMUs on the silicon and CSRs of each SMMU are
64K apart. Hence can't map page-1 even though it's not accessed.

Thanks,
Sunil.

[RFC PATCH 2/7] iommu/arm-smmu-v3: Do resource size checks based on smmu option PAGE0_REGS_ONLY

[Sunil Kovvuri]

On Tue, Apr 11, 2017 at 9:13 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 11/04/17 15:42, linucherian at gmail.com wrote:
>> From: Linu Cherian <linu.cherian@cavium.com>
>>
>> With implementations supporting only page 0 of register space,
>> resource size can be 64k as well and hence perform size checks
>> based on smmu option PAGE0_REGS_ONLY.
>
> What harm comes of mapping page 1 if we don't access it?
>
> Robin.
>

There are multiple SMMUs on the silicon and CSRs of each SMMU are
64K apart. Hence can't map page-1 even though it's not accessed.

Thanks,
Sunil.

Re: [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Will Deacon]

On Tue, Apr 11, 2017 at 05:38:21PM +0100, Robin Murphy wrote:
> On 11/04/17 17:21, Will Deacon wrote:
> > On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
> >> On 11/04/17 15:42, linucherian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> >>> From: Geetha <gakula-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> >>>
> >>> Cavium 99xx SMMU implementation doesn't not support unique irq lines for
> >>> gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
> >>> irq line for all three interrupts.
> >>
> >> AFAICS, there's nothing actually wrong with using shared wired IRQs -
> >> the architecture spec doesn't appear to say anything about it. I think
> >> it might suffice to simply add IRQF_SHARED if we can see the SMMU
> >> doesn't support MSIs anyway - it doesn't really seem like something we
> >> need to treat as a specific quirk.
> > 
> > No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
> > where it's clear that each source asserts a *unique* wired interrupt.
> 
> Perhaps I'm reading it too generously; it does indeed specify that the
> *implementation* has to provide a unique output for each source, but
> other than suggesting a particular mode of operation based on that I
> don't see anything actually forbidding the *integration* from then just
> munging those lines together externally, as integrators so often like to
> do. That's the case I had in mind.

Sure, but then there wouldn't be any point in the architecture mandating
a unique source, would there? What next, OR all the address lines together
too?

Will

[RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata

[Will Deacon]

On Tue, Apr 11, 2017 at 05:38:21PM +0100, Robin Murphy wrote:
> On 11/04/17 17:21, Will Deacon wrote:
> > On Tue, Apr 11, 2017 at 04:54:26PM +0100, Robin Murphy wrote:
> >> On 11/04/17 15:42, linucherian at gmail.com wrote:
> >>> From: Geetha <gakula@cavium.com>
> >>>
> >>> Cavium 99xx SMMU implementation doesn't not support unique irq lines for
> >>> gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single
> >>> irq line for all three interrupts.
> >>
> >> AFAICS, there's nothing actually wrong with using shared wired IRQs -
> >> the architecture spec doesn't appear to say anything about it. I think
> >> it might suffice to simply add IRQF_SHARED if we can see the SMMU
> >> doesn't support MSIs anyway - it doesn't really seem like something we
> >> need to treat as a specific quirk.
> > 
> > No, this is not permitted by the spec. See 3.18.2 ("Interrupt sources"),
> > where it's clear that each source asserts a *unique* wired interrupt.
> 
> Perhaps I'm reading it too generously; it does indeed specify that the
> *implementation* has to provide a unique output for each source, but
> other than suggesting a particular mode of operation based on that I
> don't see anything actually forbidding the *integration* from then just
> munging those lines together externally, as integrators so often like to
> do. That's the case I had in mind.

Sure, but then there wouldn't be any point in the architecture mandating
a unique source, would there? What next, OR all the address lines together
too?

Will

Re: [RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Sunil Kovvuri]

On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 11/04/17 15:42, linucherian@gmail.com wrote:
>> From: Linu Cherian <linu.cherian@cavium.com>
>>
>> Add SMMuV3 model definitions.
>>
>> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
>> ---
>>  include/acpi/actbl2.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>> index 2b4af07..9db67d6 100644
>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002   /* ARM Corelink MMU-400 */
>>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003   /* ARM Corelink MMU-500 */
>>
>> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
>> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
>> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
>> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
>
> None of those models are listed in the current IORT spec.

As mentioned in the cover letter, we are in the process of getting
model no added for
our silicon in the soon to be published updated IORT spec. Meanwhile
we wanted to take
feedback on the errata patches from experts. Hence patches were
submitted as RFC.

Thanks,
Sunil.

>
> What's MMU-600?
>
> Robin.
>
>> +
>>  /* Masks for Flags field above */
>>
>>  #define ACPI_IORT_SMMU_DVM_SUPPORTED    (1)
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Sunil Kovvuri]

On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 11/04/17 15:42, linucherian at gmail.com wrote:
>> From: Linu Cherian <linu.cherian@cavium.com>
>>
>> Add SMMuV3 model definitions.
>>
>> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
>> ---
>>  include/acpi/actbl2.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>> index 2b4af07..9db67d6 100644
>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002   /* ARM Corelink MMU-400 */
>>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003   /* ARM Corelink MMU-500 */
>>
>> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
>> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
>> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
>> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
>
> None of those models are listed in the current IORT spec.

As mentioned in the cover letter, we are in the process of getting
model no added for
our silicon in the soon to be published updated IORT spec. Meanwhile
we wanted to take
feedback on the errata patches from experts. Hence patches were
submitted as RFC.

Thanks,
Sunil.

>
> What's MMU-600?
>
> Robin.
>
>> +
>>  /* Masks for Flags field above */
>>
>>  #define ACPI_IORT_SMMU_DVM_SUPPORTED    (1)
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Hanjun Guo]

On 2017/4/12 0:57, Sunil Kovvuri wrote:
> On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 11/04/17 15:42, linucherian@gmail.com wrote:
>>> From: Linu Cherian <linu.cherian@cavium.com>
>>>
>>> Add SMMuV3 model definitions.
>>>
>>> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
>>> ---
>>>  include/acpi/actbl2.h | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>> index 2b4af07..9db67d6 100644
>>> --- a/include/acpi/actbl2.h
>>> +++ b/include/acpi/actbl2.h
>>> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>>>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002   /* ARM Corelink MMU-400 */
>>>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003   /* ARM Corelink MMU-500 */
>>>
>>> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
>>> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
>>> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
>>> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
>>
>> None of those models are listed in the current IORT spec.
>
> As mentioned in the cover letter, we are in the process of getting
> model no added for
> our silicon in the soon to be published updated IORT spec. Meanwhile
> we wanted to take
> feedback on the errata patches from experts. Hence patches were
> submitted as RFC.

Thanks. The name for Hisilicon SMMUv3 might be changed, we need to wait
for the released IORT spec.

Thanks
Hanjun

[RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Hanjun Guo]

On 2017/4/12 0:57, Sunil Kovvuri wrote:
> On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 11/04/17 15:42, linucherian at gmail.com wrote:
>>> From: Linu Cherian <linu.cherian@cavium.com>
>>>
>>> Add SMMuV3 model definitions.
>>>
>>> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
>>> ---
>>>  include/acpi/actbl2.h | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>> index 2b4af07..9db67d6 100644
>>> --- a/include/acpi/actbl2.h
>>> +++ b/include/acpi/actbl2.h
>>> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>>>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002   /* ARM Corelink MMU-400 */
>>>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003   /* ARM Corelink MMU-500 */
>>>
>>> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
>>> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
>>> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
>>> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
>>
>> None of those models are listed in the current IORT spec.
>
> As mentioned in the cover letter, we are in the process of getting
> model no added for
> our silicon in the soon to be published updated IORT spec. Meanwhile
> we wanted to take
> feedback on the errata patches from experts. Hence patches were
> submitted as RFC.

Thanks. The name for Hisilicon SMMUv3 might be changed, we need to wait
for the released IORT spec.

Thanks
Hanjun

Re: [Devel] [RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Hanjun Guo]

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

On 2017/4/12 0:57, Sunil Kovvuri wrote:
> On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy(a)arm.com> wrote:
>> On 11/04/17 15:42, linucherian(a)gmail.com wrote:
>>> From: Linu Cherian <linu.cherian(a)cavium.com>
>>>
>>> Add SMMuV3 model definitions.
>>>
>>> Signed-off-by: Linu Cherian <linu.cherian(a)cavium.com>
>>> ---
>>>  include/acpi/actbl2.h | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>> index 2b4af07..9db67d6 100644
>>> --- a/include/acpi/actbl2.h
>>> +++ b/include/acpi/actbl2.h
>>> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>>>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002   /* ARM Corelink MMU-400 */
>>>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003   /* ARM Corelink MMU-500 */
>>>
>>> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
>>> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
>>> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
>>> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
>>
>> None of those models are listed in the current IORT spec.
>
> As mentioned in the cover letter, we are in the process of getting
> model no added for
> our silicon in the soon to be published updated IORT spec. Meanwhile
> we wanted to take
> feedback on the errata patches from experts. Hence patches were
> submitted as RFC.

Thanks. The name for Hisilicon SMMUv3 might be changed, we need to wait
for the released IORT spec.

Thanks
Hanjun

Re: [RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata.

[Linu Cherian]

On Tue, Apr 11, 2017 at 9:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 11/04/17 15:42, linucherian@gmail.com wrote:
>> From: Linu Cherian <linu.cherian@cavium.com>
>>
>> Cavium 99xx SMMU implementation doesn't support page 1 register space
>> and PAGE0_REGS_ONLY option will be enabled as an errata workaround.
>
> Ugh :(
>
>> This option when turned on, replaces all page 1 offsets used for
>> EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.
>
> I think it might be neater to have something like:
>
> arm_smmu_page1(smmu) {
>         if (smmu->quirk)
>                 return smmu->base;
>         return smmu->base + 64k;
> }
>
> and use it as the base in the appropriate places, rather than override
> the individual registers. Much like ARM_SMMU_GR0_NS in the SMMUv2 driver.
>

IIUC, we need to change the offsets as well for this,
Like,
 #define ARM_SMMU_EVTQ_BASE             0xa0
-#define ARM_SMMU_EVTQ_PROD             0x100a8
-#define ARM_SMMU_EVTQ_CONS             0x100ac
+#define ARM_SMMU_EVTQ_PROD             0xa8
+#define ARM_SMMU_EVTQ_CONS             0xac
 #define ARM_SMMU_EVTQ_IRQ_CFG0         0xb0
 #define ARM_SMMU_EVTQ_IRQ_CFG1         0xb8
 #define ARM_SMMU_EVTQ_IRQ_CFG2         0xbc

 #define ARM_SMMU_PRIQ_BASE             0xc0
-#define ARM_SMMU_PRIQ_PROD             0x100c8
-#define ARM_SMMU_PRIQ_CONS             0x100cc
+#define ARM_SMMU_PRIQ_PROD             0xc8
+#define ARM_SMMU_PRIQ_CONS             0xcc


But, it appears difficult to take this approach, at least with
arm_smmu_init_one_queue function.
This function takes both page0 register offset and and page1 register
offset as an argument.
So, we might need to do additional checks in this function to decide,
whether to use
a pag0 base or page1 base.


Thanks.

[RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata.

[Linu Cherian]

On Tue, Apr 11, 2017 at 9:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 11/04/17 15:42, linucherian at gmail.com wrote:
>> From: Linu Cherian <linu.cherian@cavium.com>
>>
>> Cavium 99xx SMMU implementation doesn't support page 1 register space
>> and PAGE0_REGS_ONLY option will be enabled as an errata workaround.
>
> Ugh :(
>
>> This option when turned on, replaces all page 1 offsets used for
>> EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.
>
> I think it might be neater to have something like:
>
> arm_smmu_page1(smmu) {
>         if (smmu->quirk)
>                 return smmu->base;
>         return smmu->base + 64k;
> }
>
> and use it as the base in the appropriate places, rather than override
> the individual registers. Much like ARM_SMMU_GR0_NS in the SMMUv2 driver.
>

IIUC, we need to change the offsets as well for this,
Like,
 #define ARM_SMMU_EVTQ_BASE             0xa0
-#define ARM_SMMU_EVTQ_PROD             0x100a8
-#define ARM_SMMU_EVTQ_CONS             0x100ac
+#define ARM_SMMU_EVTQ_PROD             0xa8
+#define ARM_SMMU_EVTQ_CONS             0xac
 #define ARM_SMMU_EVTQ_IRQ_CFG0         0xb0
 #define ARM_SMMU_EVTQ_IRQ_CFG1         0xb8
 #define ARM_SMMU_EVTQ_IRQ_CFG2         0xbc

 #define ARM_SMMU_PRIQ_BASE             0xc0
-#define ARM_SMMU_PRIQ_PROD             0x100c8
-#define ARM_SMMU_PRIQ_CONS             0x100cc
+#define ARM_SMMU_PRIQ_PROD             0xc8
+#define ARM_SMMU_PRIQ_CONS             0xcc


But, it appears difficult to take this approach, at least with
arm_smmu_init_one_queue function.
This function takes both page0 register offset and and page1 register
offset as an argument.
So, we might need to do additional checks in this function to decide,
whether to use
a pag0 base or page1 base.


Thanks.

Re: [RFC PATCH 5/7] iommu/arm-smmu-v3: For ACPI based device probing, set relevant options for different SMMUv3 implementations.

[Robert Richter]

Linu,

On 11.04.17 20:12:43, linucherian@gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Enable SKIP_PREFETCH option for HiSilicon SMMUv3 model.
> Enable PAGE0_REGS_ONLY and USE_SHARED_IRQS options for
> Cavium 99xx SMMUv3 model.
> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1475ad8..00b48b4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2634,18 +2634,45 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
>  }
>  
>  #ifdef CONFIG_ACPI
> +static int acpi_smmu_get_options(u32 model, struct arm_smmu_device *smmu)
> +{
> +	int ret = 0;
> +
> +	switch (model) {
> +	case ACPI_IORT_SMMU_V3:
> +	case ACPI_IORT_SMMU_CORELINK_MMU600:
> +		break;
> +	case ACPI_IORT_SMMU_V3_HISILICON:
> +		smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> +		break;
> +	case ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:
> +		smmu->options |= (ARM_SMMU_OPT_PAGE0_REGS_ONLY |
> +				  ARM_SMMU_OPT_USE_SHARED_IRQS);
> +		break;
> +	default:
> +		ret = -ENODEV;
> +	}
> +
> +	return ret;
> +}
> +
>  static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
>  				      struct arm_smmu_device *smmu)
>  {
>  	struct acpi_iort_smmu_v3 *iort_smmu;
>  	struct device *dev = smmu->dev;
>  	struct acpi_iort_node *node;
> +	int ret;
>  
>  	node = *(struct acpi_iort_node **)dev_get_platdata(dev);
>  
>  	/* Retrieve SMMUv3 specific data */
>  	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>  
> +	ret = acpi_smmu_get_options(iort_smmu->model, smmu);
> +	if (ret < 0)
> +		return ret;
> +

could we add at least:

	dev_notice(smmu->dev, "option mask 0x%x\n", smmu->options);

We need a note to see which options have been enabled.

It could be even more fancy and maybe unified with DT code to see the
same messages for DT and ACPI.

Thanks,

-Robert

>  	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
>  		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
>  
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 5/7] iommu/arm-smmu-v3: For ACPI based device probing, set relevant options for different SMMUv3 implementations.

[Robert Richter]

Linu,

On 11.04.17 20:12:43, linucherian at gmail.com wrote:
> From: Linu Cherian <linu.cherian@cavium.com>
> 
> Enable SKIP_PREFETCH option for HiSilicon SMMUv3 model.
> Enable PAGE0_REGS_ONLY and USE_SHARED_IRQS options for
> Cavium 99xx SMMUv3 model.
> 
> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1475ad8..00b48b4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2634,18 +2634,45 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
>  }
>  
>  #ifdef CONFIG_ACPI
> +static int acpi_smmu_get_options(u32 model, struct arm_smmu_device *smmu)
> +{
> +	int ret = 0;
> +
> +	switch (model) {
> +	case ACPI_IORT_SMMU_V3:
> +	case ACPI_IORT_SMMU_CORELINK_MMU600:
> +		break;
> +	case ACPI_IORT_SMMU_V3_HISILICON:
> +		smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> +		break;
> +	case ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:
> +		smmu->options |= (ARM_SMMU_OPT_PAGE0_REGS_ONLY |
> +				  ARM_SMMU_OPT_USE_SHARED_IRQS);
> +		break;
> +	default:
> +		ret = -ENODEV;
> +	}
> +
> +	return ret;
> +}
> +
>  static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
>  				      struct arm_smmu_device *smmu)
>  {
>  	struct acpi_iort_smmu_v3 *iort_smmu;
>  	struct device *dev = smmu->dev;
>  	struct acpi_iort_node *node;
> +	int ret;
>  
>  	node = *(struct acpi_iort_node **)dev_get_platdata(dev);
>  
>  	/* Retrieve SMMUv3 specific data */
>  	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>  
> +	ret = acpi_smmu_get_options(iort_smmu->model, smmu);
> +	if (ret < 0)
> +		return ret;
> +

could we add at least:

	dev_notice(smmu->dev, "option mask 0x%x\n", smmu->options);

We need a note to see which options have been enabled.

It could be even more fancy and maybe unified with DT code to see the
same messages for DT and ACPI.

Thanks,

-Robert

>  	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
>  		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
>  
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 5/7] iommu/arm-smmu-v3: For ACPI based device probing, set relevant options for different SMMUv3 implementations.

[Linu Cherian]

>> +     switch (model) {
>> +     case ACPI_IORT_SMMU_V3:
>> +     case ACPI_IORT_SMMU_CORELINK_MMU600:
>> +             break;
>> +     case ACPI_IORT_SMMU_V3_HISILICON:
>> +             smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
>> +             break;
>> +     case ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:
>> +             smmu->options |= (ARM_SMMU_OPT_PAGE0_REGS_ONLY |
>> +                               ARM_SMMU_OPT_USE_SHARED_IRQS);
>> +             break;
>> +     default:
>> +             ret = -ENODEV;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>>  static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
>>                                     struct arm_smmu_device *smmu)
>>  {
>>       struct acpi_iort_smmu_v3 *iort_smmu;
>>       struct device *dev = smmu->dev;
>>       struct acpi_iort_node *node;
>> +     int ret;
>>
>>       node = *(struct acpi_iort_node **)dev_get_platdata(dev);
>>
>>       /* Retrieve SMMUv3 specific data */
>>       iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>>
>> +     ret = acpi_smmu_get_options(iort_smmu->model, smmu);
>> +     if (ret < 0)
>> +             return ret;
>> +
>
> could we add at least:
>
>         dev_notice(smmu->dev, "option mask 0x%x\n", smmu->options);
>
> We need a note to see which options have been enabled.
>

Yeah. Will add that.

Thanks.

[RFC PATCH 5/7] iommu/arm-smmu-v3: For ACPI based device probing, set relevant options for different SMMUv3 implementations.

[Linu Cherian]

>> +     switch (model) {
>> +     case ACPI_IORT_SMMU_V3:
>> +     case ACPI_IORT_SMMU_CORELINK_MMU600:
>> +             break;
>> +     case ACPI_IORT_SMMU_V3_HISILICON:
>> +             smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
>> +             break;
>> +     case ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:
>> +             smmu->options |= (ARM_SMMU_OPT_PAGE0_REGS_ONLY |
>> +                               ARM_SMMU_OPT_USE_SHARED_IRQS);
>> +             break;
>> +     default:
>> +             ret = -ENODEV;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>>  static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
>>                                     struct arm_smmu_device *smmu)
>>  {
>>       struct acpi_iort_smmu_v3 *iort_smmu;
>>       struct device *dev = smmu->dev;
>>       struct acpi_iort_node *node;
>> +     int ret;
>>
>>       node = *(struct acpi_iort_node **)dev_get_platdata(dev);
>>
>>       /* Retrieve SMMUv3 specific data */
>>       iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>>
>> +     ret = acpi_smmu_get_options(iort_smmu->model, smmu);
>> +     if (ret < 0)
>> +             return ret;
>> +
>
> could we add at least:
>
>         dev_notice(smmu->dev, "option mask 0x%x\n", smmu->options);
>
> We need a note to see which options have been enabled.
>

Yeah. Will add that.

Thanks.

Re: [RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Lorenzo Pieralisi]

On Tue, Apr 11, 2017 at 10:27:55PM +0530, Sunil Kovvuri wrote:
> On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> > On 11/04/17 15:42, linucherian@gmail.com wrote:
> >> From: Linu Cherian <linu.cherian@cavium.com>
> >>
> >> Add SMMuV3 model definitions.
> >>
> >> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> >> ---
> >>  include/acpi/actbl2.h | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> >> index 2b4af07..9db67d6 100644
> >> --- a/include/acpi/actbl2.h
> >> +++ b/include/acpi/actbl2.h
> >> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
> >>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002   /* ARM Corelink MMU-400 */
> >>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003   /* ARM Corelink MMU-500 */
> >>
> >> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
> >> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
> >> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
> >> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
> >
> > None of those models are listed in the current IORT spec.
> 
> As mentioned in the cover letter, we are in the process of getting
> model no added for our silicon in the soon to be published updated
> IORT spec.

Good, so first get the IORT firmware bindings updated and then post
patches to manage them, there is really no point in commenting on
patches based on FW that can't be written.

Thanks,
Lorenzo

[RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.

[Lorenzo Pieralisi]

On Tue, Apr 11, 2017 at 10:27:55PM +0530, Sunil Kovvuri wrote:
> On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> > On 11/04/17 15:42, linucherian at gmail.com wrote:
> >> From: Linu Cherian <linu.cherian@cavium.com>
> >>
> >> Add SMMuV3 model definitions.
> >>
> >> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
> >> ---
> >>  include/acpi/actbl2.h | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> >> index 2b4af07..9db67d6 100644
> >> --- a/include/acpi/actbl2.h
> >> +++ b/include/acpi/actbl2.h
> >> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
> >>  #define ACPI_IORT_SMMU_CORELINK_MMU400  0x00000002   /* ARM Corelink MMU-400 */
> >>  #define ACPI_IORT_SMMU_CORELINK_MMU500  0x00000003   /* ARM Corelink MMU-500 */
> >>
> >> +#define ACPI_IORT_SMMU_V3               0x00000000      /* Generic SMMUv3 */
> >> +#define ACPI_IORT_SMMU_CORELINK_MMU600  0x00000001      /* ARM Corelink MMU-600 */
> >> +#define ACPI_IORT_SMMU_V3_HISILICON     0x00000002      /* HiSilicon SMMUv3 */
> >> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003      /* Cavium CN99xx SMMUv3 */
> >
> > None of those models are listed in the current IORT spec.
> 
> As mentioned in the cover letter, we are in the process of getting
> model no added for our silicon in the soon to be published updated
> IORT spec.

Good, so first get the IORT firmware bindings updated and then post
patches to manage them, there is really no point in commenting on
patches based on FW that can't be written.

Thanks,
Lorenzo