[PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Geetha sowjanya]

Cavium ThunderX2 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 and also MSI for gerror,
   eventq and cmdq-sync

The following patchset does software workaround for these two erratas.

This series is based on patchset.
https://www.spinics.net/lists/arm-kernel/msg578443.html

Changes since v8:
    - Reworked patch #3 as suggested by Will. 
    - Corrected typo mistake in patch #2

Changes since v7:
    - Added new function "arm_smmu_v3_resource_size" in iort.c to get resource
      size.
    - Added new SMMU option "SHARED_IRQ" to enable errata #126 workaround.
    - Coding style issues fixed.
    - Suggested changes in arm_smmu_device_probe addressed.
    - Replaced ACPI_IORT_SMMU_CAVIUM_CN99XX macro with ACPI_IORT_SMMU_V3_CAVIUM_CN99XX

Changes since v6:
   - Changed device tree compatible string to vendor specific.
   - Rebased on Robin's latest "Update SMMU models for IORT rev. C" v2 patch.
     https://www.spinics.net/lists/arm-kernel/msg582809.html

Changes since v5:
   - Rebased on Robin's "Update SMMU models for IORT rev. C" patch.
     https://www.spinics.net/lists/arm-kernel/msg580728.html
   - Replaced ACPI_IORT_SMMU_V3_CAVIUM_CN99XX macro with ACPI_IORT_SMMU_CAVIUM_CN99XX

Changes since v4:
  - Replaced all page1 offset macros ARM_SMMU_EVTQ/PRIQ_PROD/CONS with
    arm_smmu_page1_fixup(ARM_SMMU_EVTQ/PRIQ_PROD/CONS, smmu)

Changes since v3:
  - Merged patches 1, 2 and 4 of Version 3.
  - Modified the page1_offset_adjust() and get_irq_flags() implementation as
    suggested by Robin.

Changes since v2:
  - Updated "Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt" document with
    new SMMU option used to enable errata workaround.

Changes since v1:
 - Since the use of MIDR register is rejected and SMMU_IIDR is broken on this
   silicon, as suggested by Will Deacon modified the patches to use ThunderX2
   SMMUv3 IORT model number to enable errata workaround.

Geetha Sowjanya (1):
  iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

Linu Cherian (2):
  ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3
    model
  iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74

 Documentation/arm64/silicon-errata.txt             |    2 +
 .../devicetree/bindings/iommu/arm,smmu-v3.txt      |    7 +
 drivers/acpi/arm64/iort.c                          |   69 ++++++---
 drivers/iommu/arm-smmu-v3.c                        |  171 +++++++++++++++-----
 4 files changed, 186 insertions(+), 63 deletions(-)

[PATCH v9 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

[Geetha sowjanya]

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

Cavium ThunderX2 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>
Signed-off-by: Geetha Sowjanya <geethasowjanya.akula@cavium.com>
---
 drivers/acpi/arm64/iort.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index c5fecf9..c166f3e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -828,6 +828,18 @@ static int __init arm_smmu_v3_count_resources(struct acpi_iort_node *node)
 	return num_res;
 }
 
+static unsigned long arm_smmu_v3_resource_size(struct acpi_iort_smmu_v3 *smmu)
+{
+	/*
+	 * Override the size, for Cavium ThunderX2 implementation
+	 * which doesn't support the page 1 SMMU register space.
+	 */
+	if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
+		return SZ_64K;
+
+	return SZ_128K;
+}
+
 static void __init arm_smmu_v3_init_resources(struct resource *res,
 					      struct acpi_iort_node *node)
 {
@@ -838,7 +850,8 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
 	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
 	res[num_res].start = smmu->base_address;
-	res[num_res].end = smmu->base_address + SZ_128K - 1;
+	res[num_res].end = smmu->base_address +
+				arm_smmu_v3_resource_size(smmu) - 1;
 	res[num_res].flags = IORESOURCE_MEM;
 
 	num_res++;
-- 
1.7.1

[PATCH v9 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74

[Geetha sowjanya]

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

Cavium ThunderX2 SMMU implementation doesn't support page 1 register space
and PAGE0_REGS_ONLY option is 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.

SMMU resource size checks are now based on SMMU option PAGE0_REGS_ONLY,
since resource size can be either 64k/128k.
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>
Signed-off-by: Geetha Sowjanya <geethasowjanya.akula@cavium.com>
---
 Documentation/arm64/silicon-errata.txt             |    1 +
 .../devicetree/bindings/iommu/arm,smmu-v3.txt      |    6 ++
 drivers/iommu/arm-smmu-v3.c                        |   68 ++++++++++++++-----
 3 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 10f2ddd..4693a32 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -62,6 +62,7 @@ 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                         |
 |                |                 |                 |                             |
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585         |
 |                |                 |                 |                             |
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
index be57550..6ecc48c 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -49,6 +49,12 @@ the PCIe specification.
 - hisilicon,broken-prefetch-cmd
                     : Avoid sending CMD_PREFETCH_* commands to the SMMU.
 
+- cavium,cn9900-broken-page1-regspace
+                    : Replaces all page 1 offsets used for EVTQ_PROD/CONS,
+		      PRIQ_PROD/CONS register access with page 0 offsets.
+		      Set for Cavium ThunderX2 silicon that doesn't support
+		      SMMU page1 register space.
+
 ** Example
 
         smmu@2b400000 {
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 380969a..2dea4a9 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -597,6 +597,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 +664,20 @@ 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,cn9900-broken-page1-regspace"},
 	{ 0, NULL},
 };
 
+static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset,
+						 struct arm_smmu_device *smmu)
+{
+	if ((offset > SZ_64K) &&
+	    (smmu->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY))
+		offset -= SZ_64K;
+
+	return smmu->base + offset;
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct arm_smmu_domain, domain);
@@ -1961,8 +1973,8 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
 		return -ENOMEM;
 	}
 
-	q->prod_reg	= smmu->base + prod_off;
-	q->cons_reg	= smmu->base + cons_off;
+	q->prod_reg	= arm_smmu_page1_fixup(prod_off, smmu);
+	q->cons_reg	= arm_smmu_page1_fixup(cons_off, smmu);
 	q->ent_dwords	= dwords;
 
 	q->q_base  = Q_BASE_RWA;
@@ -2363,8 +2375,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,
+		       arm_smmu_page1_fixup(ARM_SMMU_EVTQ_PROD, smmu));
+	writel_relaxed(smmu->evtq.q.cons,
+		       arm_smmu_page1_fixup(ARM_SMMU_EVTQ_CONS, smmu));
 
 	enables |= CR0_EVTQEN;
 	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
@@ -2379,9 +2393,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);
+			       arm_smmu_page1_fixup(ARM_SMMU_PRIQ_PROD, smmu));
 		writel_relaxed(smmu->priq.q.cons,
-			       smmu->base + ARM_SMMU_PRIQ_CONS);
+			       arm_smmu_page1_fixup(ARM_SMMU_PRIQ_CONS, smmu));
 
 		enables |= CR0_PRIQEN;
 		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
@@ -2605,6 +2619,14 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 }
 
 #ifdef CONFIG_ACPI
+static void acpi_smmu_get_options(u32 model, struct arm_smmu_device *smmu)
+{
+	if (model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
+		smmu->options |= ARM_SMMU_OPT_PAGE0_REGS_ONLY;
+
+	dev_notice(smmu->dev, "option mask 0x%x\n", smmu->options);
+}
+
 static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
 				      struct arm_smmu_device *smmu)
 {
@@ -2617,6 +2639,8 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
 	/* Retrieve SMMUv3 specific data */
 	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
+	acpi_smmu_get_options(iort_smmu->model, smmu);
+
 	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
@@ -2652,6 +2676,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 (smmu->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
+		return SZ_64K;
+	else
+		return SZ_128K;
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;
@@ -2668,9 +2700,20 @@ 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;
+	}
+
+	/* Set bypass mode according to firmware probing result */
+	bypass = !!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;
 	}
@@ -2697,17 +2740,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;
-
 	/* Probe the h/w */
 	ret = arm_smmu_device_hw_probe(smmu);
 	if (ret)
-- 
1.7.1

[PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

[Geetha sowjanya]

From: Geetha Sowjanya <geethasowjanya.akula@cavium.com>

Cavium ThunderX2 SMMU doesn't support MSI and also doesn't have unique irq
lines for gerror, eventq and cmdq-sync.

New named irq "combined" is set as a errata workaround, which allows to
share the irq line by register single irq handler for all the interrupts.

Signed-off-by: Geetha sowjanya <gakula@caviumnetworks.com>
---
 Documentation/arm64/silicon-errata.txt             |    1 +
 .../devicetree/bindings/iommu/arm,smmu-v3.txt      |    1 +
 drivers/acpi/arm64/iort.c                          |   54 +++++++----
 drivers/iommu/arm-smmu-v3.c                        |  105 +++++++++++++++-----
 4 files changed, 116 insertions(+), 45 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 4693a32..42422f6 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -63,6 +63,7 @@ stable kernels.
 | 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         |
 |                |                 |                 |                             |
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
index 6ecc48c..a5a1ca4 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -26,6 +26,7 @@ the PCIe specification.
                       * "priq"      - PRI Queue not empty
                       * "cmdq-sync" - CMD_SYNC complete
                       * "gerror"    - Global Error activated
+                      * "combined"  - Handles above all 4 interrupts.
 
 - #iommu-cells      : See the generic IOMMU binding described in
                         devicetree/bindings/pci/pci-iommu.txt
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index c166f3e..43e1f13 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -828,6 +828,18 @@ static int __init arm_smmu_v3_count_resources(struct acpi_iort_node *node)
 	return num_res;
 }
 
+static bool arm_smmu_v3_is_combined_irq(struct acpi_iort_smmu_v3 *smmu)
+{
+	/*
+	 * Cavium ThunderX2 implementation doesn't not support unique
+	 * irq line. Use single irq line for all the SMMUv3 interrupts.
+	 */
+	if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
+		return true;
+
+	return false;
+}
+
 static unsigned long arm_smmu_v3_resource_size(struct acpi_iort_smmu_v3 *smmu)
 {
 	/*
@@ -855,26 +867,32 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
 	res[num_res].flags = IORESOURCE_MEM;
 
 	num_res++;
-
-	if (smmu->event_gsiv)
-		acpi_iort_register_irq(smmu->event_gsiv, "eventq",
-				       ACPI_EDGE_SENSITIVE,
-				       &res[num_res++]);
-
-	if (smmu->pri_gsiv)
-		acpi_iort_register_irq(smmu->pri_gsiv, "priq",
-				       ACPI_EDGE_SENSITIVE,
-				       &res[num_res++]);
-
-	if (smmu->gerr_gsiv)
-		acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
-				       ACPI_EDGE_SENSITIVE,
-				       &res[num_res++]);
-
-	if (smmu->sync_gsiv)
-		acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
+	if (arm_smmu_v3_is_combined_irq(smmu))
+		acpi_iort_register_irq(smmu->event_gsiv, "combined",
 				       ACPI_EDGE_SENSITIVE,
 				       &res[num_res++]);
+	else {
+
+		if (smmu->event_gsiv)
+			acpi_iort_register_irq(smmu->event_gsiv, "eventq",
+					       ACPI_EDGE_SENSITIVE,
+					       &res[num_res++]);
+
+		if (smmu->pri_gsiv)
+			acpi_iort_register_irq(smmu->pri_gsiv, "priq",
+					       ACPI_EDGE_SENSITIVE,
+					       &res[num_res++]);
+
+		if (smmu->gerr_gsiv)
+			acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
+					       ACPI_EDGE_SENSITIVE,
+					       &res[num_res++]);
+
+		if (smmu->sync_gsiv)
+			acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
+					       ACPI_EDGE_SENSITIVE,
+					       &res[num_res++]);
+	}
 }
 
 static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2dea4a9..0f83f7d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -605,6 +605,7 @@ struct arm_smmu_device {
 	struct arm_smmu_priq		priq;
 
 	int				gerr_irq;
+	int				combined_irq;
 
 	unsigned long			ias; /* IPA */
 	unsigned long			oas; /* PA */
@@ -1314,6 +1315,29 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
+{
+	struct arm_smmu_device *smmu = dev;
+
+	arm_smmu_evtq_thread(irq, dev);
+	if (smmu->features & ARM_SMMU_FEAT_PRI)
+		arm_smmu_priq_thread(irq, dev);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t arm_smmu_combined_irq_handler(int irq, void *dev)
+{
+	irqreturn_t ret;
+
+	ret = arm_smmu_gerror_handler(irq, dev);
+	if (ret == IRQ_NONE) {
+		arm_smmu_cmdq_sync_handler(irq, dev);
+		return IRQ_WAKE_THREAD;
+	}
+	return ret;
+}
+
 /* IO_PGTABLE API */
 static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
 {
@@ -2230,18 +2254,9 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
 	devm_add_action(dev, arm_smmu_free_msis, dev);
 }
 
-static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
+static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)
 {
-	int ret, irq;
-	u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
-
-	/* Disable IRQs first */
-	ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
-				      ARM_SMMU_IRQ_CTRLACK);
-	if (ret) {
-		dev_err(smmu->dev, "failed to disable irqs\n");
-		return ret;
-	}
+	int irq, ret;
 
 	arm_smmu_setup_msis(smmu);
 
@@ -2284,10 +2299,41 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 			if (ret < 0)
 				dev_warn(smmu->dev,
 					 "failed to enable priq irq\n");
-			else
-				irqen_flags |= IRQ_CTRL_PRIQ_IRQEN;
 		}
 	}
+}
+
+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;
+
+	/* Disable IRQs first */
+	ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
+				      ARM_SMMU_IRQ_CTRLACK);
+	if (ret) {
+		dev_err(smmu->dev, "failed to disable irqs\n");
+		return ret;
+	}
+
+	irq = smmu->combined_irq;
+	if (irq) {
+		/*
+		 * Cavium ThunderX2 implementation doesn't not support unique
+		 * irq lines. Use single irq line for all the SMMUv3 interrupts.
+		 */
+		ret = devm_request_threaded_irq(smmu->dev, irq,
+					arm_smmu_combined_irq_handler,
+					arm_smmu_combined_irq_thread,
+					IRQF_ONESHOT,
+					"arm-smmu-v3-combined-irq", smmu);
+		if (ret < 0)
+			dev_warn(smmu->dev, "failed to enable combined irq\n");
+	} else
+		arm_smmu_setup_unique_irqs(smmu);
+
+	if (smmu->features & ARM_SMMU_FEAT_PRI)
+		irqen_flags |= IRQ_CTRL_PRIQ_IRQEN;
 
 	/* Enable interrupt generation on the SMMU */
 	ret = arm_smmu_write_reg_sync(smmu, irqen_flags,
@@ -2724,22 +2770,27 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 		return PTR_ERR(smmu->base);
 
 	/* Interrupt lines */
-	irq = platform_get_irq_byname(pdev, "eventq");
-	if (irq > 0)
-		smmu->evtq.q.irq = irq;
 
-	irq = platform_get_irq_byname(pdev, "priq");
+	irq = platform_get_irq_byname(pdev, "combined");
 	if (irq > 0)
-		smmu->priq.q.irq = irq;
+		smmu->combined_irq = irq;
+	else {
+		irq = platform_get_irq_byname(pdev, "eventq");
+		if (irq > 0)
+			smmu->evtq.q.irq = irq;
 
-	irq = platform_get_irq_byname(pdev, "cmdq-sync");
-	if (irq > 0)
-		smmu->cmdq.q.irq = irq;
+		irq = platform_get_irq_byname(pdev, "priq");
+		if (irq > 0)
+			smmu->priq.q.irq = irq;
 
-	irq = platform_get_irq_byname(pdev, "gerror");
-	if (irq > 0)
-		smmu->gerr_irq = irq;
+		irq = platform_get_irq_byname(pdev, "cmdq-sync");
+		if (irq > 0)
+			smmu->cmdq.q.irq = irq;
 
+		irq = platform_get_irq_byname(pdev, "gerror");
+		if (irq > 0)
+			smmu->gerr_irq = irq;
+	}
 	/* Probe the h/w */
 	ret = arm_smmu_device_hw_probe(smmu);
 	if (ret)
-- 
1.7.1

Re: [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

[Will Deacon]

Hi Geetha,

On Thu, Jun 22, 2017 at 05:35:38PM +0530, Geetha sowjanya wrote:
> From: Geetha Sowjanya <geethasowjanya.akula@cavium.com>
> 
> Cavium ThunderX2 SMMU doesn't support MSI and also doesn't have unique irq
> lines for gerror, eventq and cmdq-sync.
> 
> New named irq "combined" is set as a errata workaround, which allows to
> share the irq line by register single irq handler for all the interrupts.
> 
> Signed-off-by: Geetha sowjanya <gakula@caviumnetworks.com>
> ---
>  Documentation/arm64/silicon-errata.txt             |    1 +
>  .../devicetree/bindings/iommu/arm,smmu-v3.txt      |    1 +
>  drivers/acpi/arm64/iort.c                          |   54 +++++++----
>  drivers/iommu/arm-smmu-v3.c                        |  105 +++++++++++++++-----
>  4 files changed, 116 insertions(+), 45 deletions(-)

Thanks, this looks much better. Two things to change below, and I'd like to
see Lorenzo ack the iort changes.

> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 4693a32..42422f6 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -63,6 +63,7 @@ stable kernels.
>  | 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         |
>  |                |                 |                 |                             |
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> index 6ecc48c..a5a1ca4 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> @@ -26,6 +26,7 @@ the PCIe specification.
>                        * "priq"      - PRI Queue not empty
>                        * "cmdq-sync" - CMD_SYNC complete
>                        * "gerror"    - Global Error activated
> +                      * "combined"  - Handles above all 4 interrupts.

Please make it clear that:

  * The combined interrupt is optional, and should only be provided if
    the hardware supports just a single, combined interrupt line.

  * If provided, then the combined interrupt will be used in preference
    to any others.

>  - #iommu-cells      : See the generic IOMMU binding described in
>                          devicetree/bindings/pci/pci-iommu.txt
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index c166f3e..43e1f13 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -828,6 +828,18 @@ static int __init arm_smmu_v3_count_resources(struct acpi_iort_node *node)
>  	return num_res;
>  }
>  
> +static bool arm_smmu_v3_is_combined_irq(struct acpi_iort_smmu_v3 *smmu)
> +{
> +	/*
> +	 * Cavium ThunderX2 implementation doesn't not support unique
> +	 * irq line. Use single irq line for all the SMMUv3 interrupts.
> +	 */
> +	if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> +		return true;
> +
> +	return false;
> +}
> +
>  static unsigned long arm_smmu_v3_resource_size(struct acpi_iort_smmu_v3 *smmu)
>  {
>  	/*
> @@ -855,26 +867,32 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
>  	res[num_res].flags = IORESOURCE_MEM;
>  
>  	num_res++;
> -
> -	if (smmu->event_gsiv)
> -		acpi_iort_register_irq(smmu->event_gsiv, "eventq",
> -				       ACPI_EDGE_SENSITIVE,
> -				       &res[num_res++]);
> -
> -	if (smmu->pri_gsiv)
> -		acpi_iort_register_irq(smmu->pri_gsiv, "priq",
> -				       ACPI_EDGE_SENSITIVE,
> -				       &res[num_res++]);
> -
> -	if (smmu->gerr_gsiv)
> -		acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
> -				       ACPI_EDGE_SENSITIVE,
> -				       &res[num_res++]);
> -
> -	if (smmu->sync_gsiv)
> -		acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
> +	if (arm_smmu_v3_is_combined_irq(smmu))
> +		acpi_iort_register_irq(smmu->event_gsiv, "combined",
>  				       ACPI_EDGE_SENSITIVE,
>  				       &res[num_res++]);
> +	else {
> +
> +		if (smmu->event_gsiv)
> +			acpi_iort_register_irq(smmu->event_gsiv, "eventq",
> +					       ACPI_EDGE_SENSITIVE,
> +					       &res[num_res++]);
> +
> +		if (smmu->pri_gsiv)
> +			acpi_iort_register_irq(smmu->pri_gsiv, "priq",
> +					       ACPI_EDGE_SENSITIVE,
> +					       &res[num_res++]);
> +
> +		if (smmu->gerr_gsiv)
> +			acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
> +					       ACPI_EDGE_SENSITIVE,
> +					       &res[num_res++]);
> +
> +		if (smmu->sync_gsiv)
> +			acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
> +					       ACPI_EDGE_SENSITIVE,
> +					       &res[num_res++]);
> +	}
>  }
>  
>  static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 2dea4a9..0f83f7d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -605,6 +605,7 @@ struct arm_smmu_device {
>  	struct arm_smmu_priq		priq;
>  
>  	int				gerr_irq;
> +	int				combined_irq;
>  
>  	unsigned long			ias; /* IPA */
>  	unsigned long			oas; /* PA */
> @@ -1314,6 +1315,29 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
> +{
> +	struct arm_smmu_device *smmu = dev;
> +
> +	arm_smmu_evtq_thread(irq, dev);
> +	if (smmu->features & ARM_SMMU_FEAT_PRI)
> +		arm_smmu_priq_thread(irq, dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t arm_smmu_combined_irq_handler(int irq, void *dev)
> +{
> +	irqreturn_t ret;
> +
> +	ret = arm_smmu_gerror_handler(irq, dev);
> +	if (ret == IRQ_NONE) {

I don't think you can play that trick if the irq is an edge-triggered
interrupt, since you could lose an interrupt that fired whilst we were
in the handler.

The easiest thing is to always run the gerror and cmdq_sync handlers, and
then always return IRQ_WAKE_THREAD.

Will

Re: [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Will Deacon]

On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> Cavium ThunderX2 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 and also MSI for gerror,
>    eventq and cmdq-sync
> 
> The following patchset does software workaround for these two erratas.

I've picked up the first two patches, and left comments on the final patch.

Will

Re: [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Will Deacon]

On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > Cavium ThunderX2 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 and also MSI for gerror,
> >    eventq and cmdq-sync
> > 
> > The following patchset does software workaround for these two erratas.
> 
> I've picked up the first two patches, and left comments on the final patch.

... except that it doesn't build:


drivers/acpi/arm64/iort.c: In function ‘arm_smmu_v3_resource_size’:
drivers/acpi/arm64/iort.c:837:21: error: ‘ACPI_IORT_SMMU_V3_CAVIUM_CN99XX’ undeclared (first use in this function)
  if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
                     ^
drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
make[4]: *** [drivers/acpi/arm64/iort.o] Error 1


I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.

What's the plan here?

Will

Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Robert Richter]

On 22.06.17 19:58:22, Will Deacon wrote:
> On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> > On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > > Cavium ThunderX2 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 and also MSI for gerror,
> > >    eventq and cmdq-sync
> > > 
> > > The following patchset does software workaround for these two erratas.
> > 
> > I've picked up the first two patches, and left comments on the final patch.
> 
> ... except that it doesn't build:
> 
> 
> drivers/acpi/arm64/iort.c: In function ‘arm_smmu_v3_resource_size’:
> drivers/acpi/arm64/iort.c:837:21: error: ‘ACPI_IORT_SMMU_V3_CAVIUM_CN99XX’ undeclared (first use in this function)
>   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
>                      ^
> drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
> make[4]: *** [drivers/acpi/arm64/iort.o] Error 1
> 
> 
> I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.
> 
> What's the plan here?

It is defined already in acpica and we actually waiting for the acpi
maintainers to include it:

 https://github.com/acpica/acpica/commit/d00a4eb86e64

We could add

/* Until ACPICA headers cover IORT rev. C */
#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
#endif

to both files:

 drivers/acpi/arm64/iort.c
 drivers/iommu/arm-smmu-v3.c

This is similar to what Robin did.

(I checked arm64 include files and the closest was
arch/arm64/include/asm/acpi.h, bug this seems not really suitable to
me.)

I have created a separate patch to be applied at first below. We can
revert it after acpica was updated.

-Robert




>From ad7f0112a2a71059c32bd315835c33cc7bc660b8 Mon Sep 17 00:00:00 2001
From: Robert Richter <rrichter@cavium.com>
Date: Thu, 22 Jun 2017 21:20:54 +0200
Subject: [PATCH] iommu/arm-smmu-v3: Add temporary Cavium SMMU-V3 model nuber
 definitions

The model number is already defined in acpica and we actually waiting
for the acpi maintainers to include it:

 https://github.com/acpica/acpica/commit/d00a4eb86e64

Adding those temporary definitions until the change makes it into
include/acpi/actbl2.h.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/acpi/arm64/iort.c   | 5 +++++
 drivers/iommu/arm-smmu-v3.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 797b28dc7b34..15491237a657 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -31,6 +31,11 @@
 #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
 				(1 << ACPI_IORT_NODE_SMMU_V3))
 
+/* Until ACPICA headers cover IORT rev. C */
+#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
+#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
+#endif
+
 struct iort_its_msi_chip {
 	struct list_head	list;
 	struct fwnode_handle	*fw_node;
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 380969aa60d5..c759dfa7442d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -412,6 +412,11 @@
 #define MSI_IOVA_BASE			0x8000000
 #define MSI_IOVA_LENGTH			0x100000
 
+/* Until ACPICA headers cover IORT rev. C */
+#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
+#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
+#endif
+
 static bool disable_bypass;
 module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
-- 
2.11.0

Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Lorenzo Pieralisi]

On Thu, Jun 22, 2017 at 09:35:35PM +0200, Robert Richter wrote:
> On 22.06.17 19:58:22, Will Deacon wrote:
> > On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> > > On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > > > Cavium ThunderX2 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 and also MSI for gerror,
> > > >    eventq and cmdq-sync
> > > > 
> > > > The following patchset does software workaround for these two erratas.
> > > 
> > > I've picked up the first two patches, and left comments on the final patch.
> > 
> > ... except that it doesn't build:
> > 
> > 
> > drivers/acpi/arm64/iort.c: In function ‘arm_smmu_v3_resource_size’:
> > drivers/acpi/arm64/iort.c:837:21: error: ‘ACPI_IORT_SMMU_V3_CAVIUM_CN99XX’ undeclared (first use in this function)
> >   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> >                      ^
> > drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
> > make[4]: *** [drivers/acpi/arm64/iort.o] Error 1
> > 
> > 
> > I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.
> > 
> > What's the plan here?
> 
> It is defined already in acpica and we actually waiting for the acpi
> maintainers to include it:
> 
>  https://github.com/acpica/acpica/commit/d00a4eb86e64
> 
> We could add
> 
> /* Until ACPICA headers cover IORT rev. C */
> #ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> #define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> #endif
> 
> to both files:
> 
>  drivers/acpi/arm64/iort.c
>  drivers/iommu/arm-smmu-v3.c
> 

I thought it was a solved problem (and that the IORT patch was based
on Robin's workaround) but I was clearly wrong and I apologise to
Will about this.

FWIW, you could add the define in include/linux/acpi_iort.h and I will
remove it whenever ACPICA changes make it into the kernel.

Thanks,
Lorenzo

> This is similar to what Robin did.
> 
> (I checked arm64 include files and the closest was
> arch/arm64/include/asm/acpi.h, bug this seems not really suitable to
> me.)
> 
> I have created a separate patch to be applied at first below. We can
> revert it after acpica was updated.
> 
> -Robert
> 
> 
> 
> 
> From ad7f0112a2a71059c32bd315835c33cc7bc660b8 Mon Sep 17 00:00:00 2001
> From: Robert Richter <rrichter@cavium.com>
> Date: Thu, 22 Jun 2017 21:20:54 +0200
> Subject: [PATCH] iommu/arm-smmu-v3: Add temporary Cavium SMMU-V3 model nuber
>  definitions
> 
> The model number is already defined in acpica and we actually waiting
> for the acpi maintainers to include it:
> 
>  https://github.com/acpica/acpica/commit/d00a4eb86e64
> 
> Adding those temporary definitions until the change makes it into
> include/acpi/actbl2.h.
> 
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  drivers/acpi/arm64/iort.c   | 5 +++++
>  drivers/iommu/arm-smmu-v3.c | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 797b28dc7b34..15491237a657 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -31,6 +31,11 @@
>  #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
>  				(1 << ACPI_IORT_NODE_SMMU_V3))
>  
> +/* Until ACPICA headers cover IORT rev. C */
> +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> +#endif
> +
>  struct iort_its_msi_chip {
>  	struct list_head	list;
>  	struct fwnode_handle	*fw_node;
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 380969aa60d5..c759dfa7442d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -412,6 +412,11 @@
>  #define MSI_IOVA_BASE			0x8000000
>  #define MSI_IOVA_LENGTH			0x100000
>  
> +/* Until ACPICA headers cover IORT rev. C */
> +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> +#endif
> +
>  static bool disable_bypass;
>  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
> -- 
> 2.11.0
> 

Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Robert Richter]

On 22.06.17 22:04:37, Lorenzo Pieralisi wrote:
> On Thu, Jun 22, 2017 at 09:35:35PM +0200, Robert Richter wrote:
> > On 22.06.17 19:58:22, Will Deacon wrote:
> > > On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> > > > On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > > > > Cavium ThunderX2 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 and also MSI for gerror,
> > > > >    eventq and cmdq-sync
> > > > > 
> > > > > The following patchset does software workaround for these two erratas.
> > > > 
> > > > I've picked up the first two patches, and left comments on the final patch.
> > > 
> > > ... except that it doesn't build:
> > > 
> > > 
> > > drivers/acpi/arm64/iort.c: In function ‘arm_smmu_v3_resource_size’:
> > > drivers/acpi/arm64/iort.c:837:21: error: ‘ACPI_IORT_SMMU_V3_CAVIUM_CN99XX’ undeclared (first use in this function)
> > >   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> > >                      ^
> > > drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
> > > make[4]: *** [drivers/acpi/arm64/iort.o] Error 1
> > > 
> > > 
> > > I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.
> > > 
> > > What's the plan here?
> > 
> > It is defined already in acpica and we actually waiting for the acpi
> > maintainers to include it:
> > 
> >  https://github.com/acpica/acpica/commit/d00a4eb86e64
> > 
> > We could add
> > 
> > /* Until ACPICA headers cover IORT rev. C */
> > #ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > #define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > #endif
> > 
> > to both files:
> > 
> >  drivers/acpi/arm64/iort.c
> >  drivers/iommu/arm-smmu-v3.c
> > 
> 
> I thought it was a solved problem (and that the IORT patch was based
> on Robin's workaround) but I was clearly wrong and I apologise to
> Will about this.
> 
> FWIW, you could add the define in include/linux/acpi_iort.h and I will
> remove it whenever ACPICA changes make it into the kernel.

Adding it there will still let depend us on acpi maintainers, while I
think the over 2 files might go through arm64 tree smoothly. A change
in acpi_iort.h also adds the definition to other archs and I don't
think that adding arch #ifdefs to avoid that are welcome in that
header file too.

I am going to resend my patch below with an improved wording.

Thanks,

-Robert

> 
> Thanks,
> Lorenzo
> 
> > This is similar to what Robin did.
> > 
> > (I checked arm64 include files and the closest was
> > arch/arm64/include/asm/acpi.h, bug this seems not really suitable to
> > me.)
> > 
> > I have created a separate patch to be applied at first below. We can
> > revert it after acpica was updated.
> > 
> > -Robert
> > 
> > 
> > 
> > 
> > From ad7f0112a2a71059c32bd315835c33cc7bc660b8 Mon Sep 17 00:00:00 2001
> > From: Robert Richter <rrichter@cavium.com>
> > Date: Thu, 22 Jun 2017 21:20:54 +0200
> > Subject: [PATCH] iommu/arm-smmu-v3: Add temporary Cavium SMMU-V3 model nuber
> >  definitions
> > 
> > The model number is already defined in acpica and we actually waiting
> > for the acpi maintainers to include it:
> > 
> >  https://github.com/acpica/acpica/commit/d00a4eb86e64
> > 
> > Adding those temporary definitions until the change makes it into
> > include/acpi/actbl2.h.
> > 
> > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > ---
> >  drivers/acpi/arm64/iort.c   | 5 +++++
> >  drivers/iommu/arm-smmu-v3.c | 5 +++++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 797b28dc7b34..15491237a657 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -31,6 +31,11 @@
> >  #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
> >  				(1 << ACPI_IORT_NODE_SMMU_V3))
> >  
> > +/* Until ACPICA headers cover IORT rev. C */
> > +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > +#endif
> > +
> >  struct iort_its_msi_chip {
> >  	struct list_head	list;
> >  	struct fwnode_handle	*fw_node;
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> > index 380969aa60d5..c759dfa7442d 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -412,6 +412,11 @@
> >  #define MSI_IOVA_BASE			0x8000000
> >  #define MSI_IOVA_LENGTH			0x100000
> >  
> > +/* Until ACPICA headers cover IORT rev. C */
> > +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > +#endif
> > +
> >  static bool disable_bypass;
> >  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
> >  MODULE_PARM_DESC(disable_bypass,
> > -- 
> > 2.11.0
> > 

[PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions

[Robert Richter]

On 23.06.17 06:55:41, Robert Richter wrote:
> On 22.06.17 22:04:37, Lorenzo Pieralisi wrote:
> > On Thu, Jun 22, 2017 at 09:35:35PM +0200, Robert Richter wrote:
> > > On 22.06.17 19:58:22, Will Deacon wrote:
> > > > On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> > > > > On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > > > > > Cavium ThunderX2 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 and also MSI for gerror,
> > > > > >    eventq and cmdq-sync
> > > > > > 
> > > > > > The following patchset does software workaround for these two erratas.
> > > > > 
> > > > > I've picked up the first two patches, and left comments on the final patch.
> > > > 
> > > > ... except that it doesn't build:
> > > > 
> > > > 
> > > > drivers/acpi/arm64/iort.c: In function ‘arm_smmu_v3_resource_size’:
> > > > drivers/acpi/arm64/iort.c:837:21: error: ‘ACPI_IORT_SMMU_V3_CAVIUM_CN99XX’ undeclared (first use in this function)
> > > >   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> > > >                      ^
> > > > drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
> > > > make[4]: *** [drivers/acpi/arm64/iort.o] Error 1
> > > > 
> > > > 
> > > > I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.
> > > > 
> > > > What's the plan here?
> > > 
> > > It is defined already in acpica and we actually waiting for the acpi
> > > maintainers to include it:
> > > 
> > >  https://github.com/acpica/acpica/commit/d00a4eb86e64
> > > 
> > > We could add
> > > 
> > > /* Until ACPICA headers cover IORT rev. C */
> > > #ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > > #define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > > #endif
> > > 
> > > to both files:
> > > 
> > >  drivers/acpi/arm64/iort.c
> > >  drivers/iommu/arm-smmu-v3.c
> > > 
> > 
> > I thought it was a solved problem (and that the IORT patch was based
> > on Robin's workaround) but I was clearly wrong and I apologise to
> > Will about this.
> > 
> > FWIW, you could add the define in include/linux/acpi_iort.h and I will
> > remove it whenever ACPICA changes make it into the kernel.
> 
> Adding it there will still let depend us on acpi maintainers, while I
> think the over 2 files might go through arm64 tree smoothly. A change
> in acpi_iort.h also adds the definition to other archs and I don't
> think that adding arch #ifdefs to avoid that are welcome in that
> header file too.
> 
> I am going to resend my patch below with an improved wording.

Here it comes:

>From d210b4c540bc4adcebd51d5a87437d2049649e94 Mon Sep 17 00:00:00 2001
From: Robert Richter <rrichter@cavium.com>
Date: Thu, 22 Jun 2017 21:20:54 +0200
Subject: [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT
 model number definitions

The model number is already defined in acpica and we are actually
waiting for the acpi maintainers to include it:

 https://github.com/acpica/acpica/commit/d00a4eb86e64

Adding those temporary definitions until the change makes it into
include/acpi/actbl2.h. Once that is done this patch can be reverted.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/acpi/arm64/iort.c   | 5 +++++
 drivers/iommu/arm-smmu-v3.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 797b28dc7b34..15491237a657 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -31,6 +31,11 @@
 #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
 				(1 << ACPI_IORT_NODE_SMMU_V3))
 
+/* Until ACPICA headers cover IORT rev. C */
+#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
+#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
+#endif
+
 struct iort_its_msi_chip {
 	struct list_head	list;
 	struct fwnode_handle	*fw_node;
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 380969aa60d5..c759dfa7442d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -412,6 +412,11 @@
 #define MSI_IOVA_BASE			0x8000000
 #define MSI_IOVA_LENGTH			0x100000
 
+/* Until ACPICA headers cover IORT rev. C */
+#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
+#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
+#endif
+
 static bool disable_bypass;
 module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
-- 
2.11.0

Re: [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

[Geetha Akula]

On Thu, Jun 22, 2017 at 11:52 PM, Will Deacon <will.deacon@arm.com> wrote:
> Hi Geetha,
>
> On Thu, Jun 22, 2017 at 05:35:38PM +0530, Geetha sowjanya wrote:
>> From: Geetha Sowjanya <geethasowjanya.akula@cavium.com>
>>
>> Cavium ThunderX2 SMMU doesn't support MSI and also doesn't have unique irq
>> lines for gerror, eventq and cmdq-sync.
>>
>> New named irq "combined" is set as a errata workaround, which allows to
>> share the irq line by register single irq handler for all the interrupts.
>>
>> Signed-off-by: Geetha sowjanya <gakula@caviumnetworks.com>
>> ---
>>  Documentation/arm64/silicon-errata.txt             |    1 +
>>  .../devicetree/bindings/iommu/arm,smmu-v3.txt      |    1 +
>>  drivers/acpi/arm64/iort.c                          |   54 +++++++----
>>  drivers/iommu/arm-smmu-v3.c                        |  105 +++++++++++++++-----
>>  4 files changed, 116 insertions(+), 45 deletions(-)
>
> Thanks, this looks much better. Two things to change below, and I'd like to
> see Lorenzo ack the iort changes.

Thanks Will. I have resend the patch with suggested changes.


Geetha.
>
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 4693a32..42422f6 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -63,6 +63,7 @@ stable kernels.
>>  | 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         |
>>  |                |                 |                 |                             |
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
>> index 6ecc48c..a5a1ca4 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
>> @@ -26,6 +26,7 @@ the PCIe specification.
>>                        * "priq"      - PRI Queue not empty
>>                        * "cmdq-sync" - CMD_SYNC complete
>>                        * "gerror"    - Global Error activated
>> +                      * "combined"  - Handles above all 4 interrupts.
>
> Please make it clear that:
>
>   * The combined interrupt is optional, and should only be provided if
>     the hardware supports just a single, combined interrupt line.
>
>   * If provided, then the combined interrupt will be used in preference
>     to any others.
>
>>  - #iommu-cells      : See the generic IOMMU binding described in
>>                          devicetree/bindings/pci/pci-iommu.txt
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index c166f3e..43e1f13 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -828,6 +828,18 @@ static int __init arm_smmu_v3_count_resources(struct acpi_iort_node *node)
>>       return num_res;
>>  }
>>
>> +static bool arm_smmu_v3_is_combined_irq(struct acpi_iort_smmu_v3 *smmu)
>> +{
>> +     /*
>> +      * Cavium ThunderX2 implementation doesn't not support unique
>> +      * irq line. Use single irq line for all the SMMUv3 interrupts.
>> +      */
>> +     if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
>> +             return true;
>> +
>> +     return false;
>> +}
>> +
>>  static unsigned long arm_smmu_v3_resource_size(struct acpi_iort_smmu_v3 *smmu)
>>  {
>>       /*
>> @@ -855,26 +867,32 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
>>       res[num_res].flags = IORESOURCE_MEM;
>>
>>       num_res++;
>> -
>> -     if (smmu->event_gsiv)
>> -             acpi_iort_register_irq(smmu->event_gsiv, "eventq",
>> -                                    ACPI_EDGE_SENSITIVE,
>> -                                    &res[num_res++]);
>> -
>> -     if (smmu->pri_gsiv)
>> -             acpi_iort_register_irq(smmu->pri_gsiv, "priq",
>> -                                    ACPI_EDGE_SENSITIVE,
>> -                                    &res[num_res++]);
>> -
>> -     if (smmu->gerr_gsiv)
>> -             acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
>> -                                    ACPI_EDGE_SENSITIVE,
>> -                                    &res[num_res++]);
>> -
>> -     if (smmu->sync_gsiv)
>> -             acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
>> +     if (arm_smmu_v3_is_combined_irq(smmu))
>> +             acpi_iort_register_irq(smmu->event_gsiv, "combined",
>>                                      ACPI_EDGE_SENSITIVE,
>>                                      &res[num_res++]);
>> +     else {
>> +
>> +             if (smmu->event_gsiv)
>> +                     acpi_iort_register_irq(smmu->event_gsiv, "eventq",
>> +                                            ACPI_EDGE_SENSITIVE,
>> +                                            &res[num_res++]);
>> +
>> +             if (smmu->pri_gsiv)
>> +                     acpi_iort_register_irq(smmu->pri_gsiv, "priq",
>> +                                            ACPI_EDGE_SENSITIVE,
>> +                                            &res[num_res++]);
>> +
>> +             if (smmu->gerr_gsiv)
>> +                     acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
>> +                                            ACPI_EDGE_SENSITIVE,
>> +                                            &res[num_res++]);
>> +
>> +             if (smmu->sync_gsiv)
>> +                     acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
>> +                                            ACPI_EDGE_SENSITIVE,
>> +                                            &res[num_res++]);
>> +     }
>>  }
>>
>>  static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 2dea4a9..0f83f7d 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -605,6 +605,7 @@ struct arm_smmu_device {
>>       struct arm_smmu_priq            priq;
>>
>>       int                             gerr_irq;
>> +     int                             combined_irq;
>>
>>       unsigned long                   ias; /* IPA */
>>       unsigned long                   oas; /* PA */
>> @@ -1314,6 +1315,29 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
>>       return IRQ_HANDLED;
>>  }
>>
>> +static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
>> +{
>> +     struct arm_smmu_device *smmu = dev;
>> +
>> +     arm_smmu_evtq_thread(irq, dev);
>> +     if (smmu->features & ARM_SMMU_FEAT_PRI)
>> +             arm_smmu_priq_thread(irq, dev);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t arm_smmu_combined_irq_handler(int irq, void *dev)
>> +{
>> +     irqreturn_t ret;
>> +
>> +     ret = arm_smmu_gerror_handler(irq, dev);
>> +     if (ret == IRQ_NONE) {
>
> I don't think you can play that trick if the irq is an edge-triggered
> interrupt, since you could lose an interrupt that fired whilst we were
> in the handler.
>
> The easiest thing is to always run the gerror and cmdq_sync handlers, and
> then always return IRQ_WAKE_THREAD.
>
> Will

Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Lorenzo Pieralisi]

On Fri, Jun 23, 2017 at 06:55:41AM +0200, Robert Richter wrote:
> On 22.06.17 22:04:37, Lorenzo Pieralisi wrote:
> > On Thu, Jun 22, 2017 at 09:35:35PM +0200, Robert Richter wrote:
> > > On 22.06.17 19:58:22, Will Deacon wrote:
> > > > On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> > > > > On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > > > > > Cavium ThunderX2 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 and also MSI for gerror,
> > > > > >    eventq and cmdq-sync
> > > > > > 
> > > > > > The following patchset does software workaround for these two erratas.
> > > > > 
> > > > > I've picked up the first two patches, and left comments on the final patch.
> > > > 
> > > > ... except that it doesn't build:
> > > > 
> > > > 
> > > > drivers/acpi/arm64/iort.c: In function ‘arm_smmu_v3_resource_size’:
> > > > drivers/acpi/arm64/iort.c:837:21: error: ‘ACPI_IORT_SMMU_V3_CAVIUM_CN99XX’ undeclared (first use in this function)
> > > >   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> > > >                      ^
> > > > drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
> > > > make[4]: *** [drivers/acpi/arm64/iort.o] Error 1
> > > > 
> > > > 
> > > > I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.
> > > > 
> > > > What's the plan here?
> > > 
> > > It is defined already in acpica and we actually waiting for the acpi
> > > maintainers to include it:
> > > 
> > >  https://github.com/acpica/acpica/commit/d00a4eb86e64
> > > 
> > > We could add
> > > 
> > > /* Until ACPICA headers cover IORT rev. C */
> > > #ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > > #define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > > #endif
> > > 
> > > to both files:
> > > 
> > >  drivers/acpi/arm64/iort.c
> > >  drivers/iommu/arm-smmu-v3.c
> > > 
> > 
> > I thought it was a solved problem (and that the IORT patch was based
> > on Robin's workaround) but I was clearly wrong and I apologise to
> > Will about this.
> > 
> > FWIW, you could add the define in include/linux/acpi_iort.h and I will
> > remove it whenever ACPICA changes make it into the kernel.
> 
> Adding it there will still let depend us on acpi maintainers, while I
> think the over 2 files might go through arm64 tree smoothly. A change
> in acpi_iort.h also adds the definition to other archs and I don't
> think that adding arch #ifdefs to avoid that are welcome in that
> header file too.

I do not think it would be a problem but you have a point, it is fine to
add the define in the .c files, it is just a temporary plaster.

Thanks,
Lorenzo

> I am going to resend my patch below with an improved wording.
> 
> Thanks,
> 
> -Robert
> 
> > 
> > Thanks,
> > Lorenzo
> > 
> > > This is similar to what Robin did.
> > > 
> > > (I checked arm64 include files and the closest was
> > > arch/arm64/include/asm/acpi.h, bug this seems not really suitable to
> > > me.)
> > > 
> > > I have created a separate patch to be applied at first below. We can
> > > revert it after acpica was updated.
> > > 
> > > -Robert
> > > 
> > > 
> > > 
> > > 
> > > From ad7f0112a2a71059c32bd315835c33cc7bc660b8 Mon Sep 17 00:00:00 2001
> > > From: Robert Richter <rrichter@cavium.com>
> > > Date: Thu, 22 Jun 2017 21:20:54 +0200
> > > Subject: [PATCH] iommu/arm-smmu-v3: Add temporary Cavium SMMU-V3 model nuber
> > >  definitions
> > > 
> > > The model number is already defined in acpica and we actually waiting
> > > for the acpi maintainers to include it:
> > > 
> > >  https://github.com/acpica/acpica/commit/d00a4eb86e64
> > > 
> > > Adding those temporary definitions until the change makes it into
> > > include/acpi/actbl2.h.
> > > 
> > > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > > ---
> > >  drivers/acpi/arm64/iort.c   | 5 +++++
> > >  drivers/iommu/arm-smmu-v3.c | 5 +++++
> > >  2 files changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > > index 797b28dc7b34..15491237a657 100644
> > > --- a/drivers/acpi/arm64/iort.c
> > > +++ b/drivers/acpi/arm64/iort.c
> > > @@ -31,6 +31,11 @@
> > >  #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
> > >  				(1 << ACPI_IORT_NODE_SMMU_V3))
> > >  
> > > +/* Until ACPICA headers cover IORT rev. C */
> > > +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > > +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > > +#endif
> > > +
> > >  struct iort_its_msi_chip {
> > >  	struct list_head	list;
> > >  	struct fwnode_handle	*fw_node;
> > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> > > index 380969aa60d5..c759dfa7442d 100644
> > > --- a/drivers/iommu/arm-smmu-v3.c
> > > +++ b/drivers/iommu/arm-smmu-v3.c
> > > @@ -412,6 +412,11 @@
> > >  #define MSI_IOVA_BASE			0x8000000
> > >  #define MSI_IOVA_LENGTH			0x100000
> > >  
> > > +/* Until ACPICA headers cover IORT rev. C */
> > > +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > > +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > > +#endif
> > > +
> > >  static bool disable_bypass;
> > >  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
> > >  MODULE_PARM_DESC(disable_bypass,
> > > -- 
> > > 2.11.0
> > > 

Re: [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions

[Lorenzo Pieralisi]

On Fri, Jun 23, 2017 at 06:59:33AM +0200, Robert Richter wrote:
> On 23.06.17 06:55:41, Robert Richter wrote:
> > On 22.06.17 22:04:37, Lorenzo Pieralisi wrote:
> > > On Thu, Jun 22, 2017 at 09:35:35PM +0200, Robert Richter wrote:
> > > > On 22.06.17 19:58:22, Will Deacon wrote:
> > > > > On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> > > > > > On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > > > > > > Cavium ThunderX2 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 and also MSI for gerror,
> > > > > > >    eventq and cmdq-sync
> > > > > > > 
> > > > > > > The following patchset does software workaround for these two erratas.
> > > > > > 
> > > > > > I've picked up the first two patches, and left comments on the final patch.
> > > > > 
> > > > > ... except that it doesn't build:
> > > > > 
> > > > > 
> > > > > drivers/acpi/arm64/iort.c: In function ‘arm_smmu_v3_resource_size’:
> > > > > drivers/acpi/arm64/iort.c:837:21: error: ‘ACPI_IORT_SMMU_V3_CAVIUM_CN99XX’ undeclared (first use in this function)
> > > > >   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> > > > >                      ^
> > > > > drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
> > > > > make[4]: *** [drivers/acpi/arm64/iort.o] Error 1
> > > > > 
> > > > > 
> > > > > I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.
> > > > > 
> > > > > What's the plan here?
> > > > 
> > > > It is defined already in acpica and we actually waiting for the acpi
> > > > maintainers to include it:
> > > > 
> > > >  https://github.com/acpica/acpica/commit/d00a4eb86e64
> > > > 
> > > > We could add
> > > > 
> > > > /* Until ACPICA headers cover IORT rev. C */
> > > > #ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> > > > #define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> > > > #endif
> > > > 
> > > > to both files:
> > > > 
> > > >  drivers/acpi/arm64/iort.c
> > > >  drivers/iommu/arm-smmu-v3.c
> > > > 
> > > 
> > > I thought it was a solved problem (and that the IORT patch was based
> > > on Robin's workaround) but I was clearly wrong and I apologise to
> > > Will about this.
> > > 
> > > FWIW, you could add the define in include/linux/acpi_iort.h and I will
> > > remove it whenever ACPICA changes make it into the kernel.
> > 
> > Adding it there will still let depend us on acpi maintainers, while I
> > think the over 2 files might go through arm64 tree smoothly. A change
> > in acpi_iort.h also adds the definition to other archs and I don't
> > think that adding arch #ifdefs to avoid that are welcome in that
> > header file too.
> > 
> > I am going to resend my patch below with an improved wording.
> 
> Here it comes:
> 
> From d210b4c540bc4adcebd51d5a87437d2049649e94 Mon Sep 17 00:00:00 2001
> From: Robert Richter <rrichter@cavium.com>
> Date: Thu, 22 Jun 2017 21:20:54 +0200
> Subject: [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT
>  model number definitions
> 
> The model number is already defined in acpica and we are actually
> waiting for the acpi maintainers to include it:
> 
>  https://github.com/acpica/acpica/commit/d00a4eb86e64
> 
> Adding those temporary definitions until the change makes it into
> include/acpi/actbl2.h. Once that is done this patch can be reverted.
> 
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  drivers/acpi/arm64/iort.c   | 5 +++++

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

>  drivers/iommu/arm-smmu-v3.c | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 797b28dc7b34..15491237a657 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -31,6 +31,11 @@
>  #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
>  				(1 << ACPI_IORT_NODE_SMMU_V3))
>  
> +/* Until ACPICA headers cover IORT rev. C */
> +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> +#endif
> +
>  struct iort_its_msi_chip {
>  	struct list_head	list;
>  	struct fwnode_handle	*fw_node;
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 380969aa60d5..c759dfa7442d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -412,6 +412,11 @@
>  #define MSI_IOVA_BASE			0x8000000
>  #define MSI_IOVA_LENGTH			0x100000
>  
> +/* Until ACPICA headers cover IORT rev. C */
> +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> +#endif
> +
>  static bool disable_bypass;
>  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
> -- 
> 2.11.0
>