2017-06-22 12:25:36

by Geetha sowjanya

[permalink] [raw]
Subject: [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

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(-)


2017-06-22 12:25:53

by Geetha sowjanya

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

From: Linu Cherian <[email protected]>

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 <[email protected]>
Signed-off-by: Geetha Sowjanya <[email protected]>
---
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

2017-06-22 12:25:59

by Geetha sowjanya

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

From: Linu Cherian <[email protected]>

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 <[email protected]>
Signed-off-by: Geetha Sowjanya <[email protected]>
---
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

2017-06-22 12:26:24

by Geetha sowjanya

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

From: Geetha Sowjanya <[email protected]>

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 <[email protected]>
---
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

2017-06-22 18:22:16

by Will Deacon

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

Hi Geetha,

On Thu, Jun 22, 2017 at 05:35:38PM +0530, Geetha sowjanya wrote:
> From: Geetha Sowjanya <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

2017-06-22 18:22:47

by Will Deacon

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

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

2017-06-22 18:58:12

by Will Deacon

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

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

2017-06-22 19:35:42

by Robert Richter

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

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 <[email protected]>
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 <[email protected]>
---
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

2017-06-22 21:03:16

by Lorenzo Pieralisi

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

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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
>

2017-06-23 04:56:03

by Richter, Robert

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

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 <[email protected]>
> > 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 <[email protected]>
> > ---
> > 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
> >

2017-06-23 04:59:56

by Richter, Robert

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

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 <[email protected]>
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 <[email protected]>
---
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

2017-06-23 06:21:19

by Geetha Akula

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

On Thu, Jun 22, 2017 at 11:52 PM, Will Deacon <[email protected]> wrote:
> Hi Geetha,
>
> On Thu, Jun 22, 2017 at 05:35:38PM +0530, Geetha sowjanya wrote:
>> From: Geetha Sowjanya <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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

2017-06-23 08:42:04

by Lorenzo Pieralisi

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

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 <[email protected]>
> > > 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 <[email protected]>
> > > ---
> > > 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
> > >

2017-06-23 10:09:56

by Lorenzo Pieralisi

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

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 <[email protected]>
> 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 <[email protected]>
> ---
> drivers/acpi/arm64/iort.c | 5 +++++

Acked-by: Lorenzo Pieralisi <[email protected]>

> 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
>