* [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds
@ 2017-06-22 12:05 Geetha sowjanya
2017-06-22 12:05 ` [PATCH v9 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74 Geetha sowjanya
[not found] ` <1498133138-20244-1-git-send-email-gakula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
0 siblings, 2 replies; 14+ messages in thread
From: Geetha sowjanya @ 2017-06-22 12:05 UTC (permalink / raw)
To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8,
lorenzo.pieralisi-5wv7dgnIgG8, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A,
sudeep.holla-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, Charles.Garcia-Tobin-5wv7dgnIgG8,
Geetha sowjanya, geethasowjanya.akula-Re5JQEeQqe8AvxtiuMwx3w,
jcm-H+wXaHxf7aLQT0dZR+AlfA, linu.cherian-YGCgFSpz5w/QT0dZR+AlfA,
rjw-LthD3rsA81gm4RdzfppkhA, robert.moore-ral2JQCrhuEAvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
robert.richter-YGCgFSpz5w/QT0dZR+AlfA,
lv.zheng-ral2JQCrhuEAvxtiuMwx3w, catalin.marinas-5wv7dgnIgG8,
sgoutham-YGCgFSpz5w/QT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devel-E0kO6a4B6psdnm+yROfE0A
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(-)
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v9 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74 2017-06-22 12:05 [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Geetha sowjanya @ 2017-06-22 12:05 ` Geetha sowjanya [not found] ` <1498133138-20244-1-git-send-email-gakula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org> 1 sibling, 0 replies; 14+ messages in thread From: Geetha sowjanya @ 2017-06-22 12:05 UTC (permalink / raw) To: will.deacon, robin.murphy, lorenzo.pieralisi, hanjun.guo, sudeep.holla, iommu Cc: robert.moore, lv.zheng, rjw, jcm, linux-kernel, robert.richter, catalin.marinas, sgoutham, linux-arm-kernel, linux-acpi, geethasowjanya.akula, devel, linu.cherian, Charles.Garcia-Tobin, robh, 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <1498133138-20244-1-git-send-email-gakula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>]
* [PATCH v9 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model [not found] ` <1498133138-20244-1-git-send-email-gakula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org> @ 2017-06-22 12:05 ` Geetha sowjanya 2017-06-22 12:05 ` [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126 Geetha sowjanya 2017-06-22 18:22 ` [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Will Deacon 2 siblings, 0 replies; 14+ messages in thread From: Geetha sowjanya @ 2017-06-22 12:05 UTC (permalink / raw) To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A, sudeep.holla-5wv7dgnIgG8, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, Charles.Garcia-Tobin-5wv7dgnIgG8, Geetha Sowjanya, geethasowjanya.akula-Re5JQEeQqe8AvxtiuMwx3w, jcm-H+wXaHxf7aLQT0dZR+AlfA, linu.cherian-YGCgFSpz5w/QT0dZR+AlfA, rjw-LthD3rsA81gm4RdzfppkhA, robert.moore-ral2JQCrhuEAvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA, robert.richter-YGCgFSpz5w/QT0dZR+AlfA, lv.zheng-ral2JQCrhuEAvxtiuMwx3w, catalin.marinas-5wv7dgnIgG8, sgoutham-YGCgFSpz5w/QT0dZR+AlfA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devel-E0kO6a4B6psdnm+yROfE0A From: Linu Cherian <linu.cherian-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org> 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-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org> Signed-off-by: Geetha Sowjanya <geethasowjanya.akula-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org> --- 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126 [not found] ` <1498133138-20244-1-git-send-email-gakula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org> 2017-06-22 12:05 ` [PATCH v9 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model Geetha sowjanya @ 2017-06-22 12:05 ` Geetha sowjanya 2017-06-22 18:22 ` Will Deacon 2017-06-22 18:22 ` [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Will Deacon 2 siblings, 1 reply; 14+ messages in thread From: Geetha sowjanya @ 2017-06-22 12:05 UTC (permalink / raw) To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A, sudeep.holla-5wv7dgnIgG8, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, Charles.Garcia-Tobin-5wv7dgnIgG8, Geetha Sowjanya, geethasowjanya.akula-Re5JQEeQqe8AvxtiuMwx3w, jcm-H+wXaHxf7aLQT0dZR+AlfA, linu.cherian-YGCgFSpz5w/QT0dZR+AlfA, rjw-LthD3rsA81gm4RdzfppkhA, robert.moore-ral2JQCrhuEAvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA, robert.richter-YGCgFSpz5w/QT0dZR+AlfA, lv.zheng-ral2JQCrhuEAvxtiuMwx3w, catalin.marinas-5wv7dgnIgG8, Geetha sowjanya, sgoutham-YGCgFSpz5w/QT0dZR+AlfA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devel-E0kO6a4B6psdnm+yROfE0A From: Geetha Sowjanya <geethasowjanya.akula-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org> 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-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org> --- 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126 2017-06-22 12:05 ` [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126 Geetha sowjanya @ 2017-06-22 18:22 ` Will Deacon 2017-06-23 6:21 ` Geetha Akula 0 siblings, 1 reply; 14+ messages in thread From: Will Deacon @ 2017-06-22 18:22 UTC (permalink / raw) To: Geetha sowjanya Cc: robin.murphy, lorenzo.pieralisi, hanjun.guo, sudeep.holla, iommu, robert.moore, lv.zheng, rjw, jcm, linux-kernel, robert.richter, catalin.marinas, sgoutham, linux-arm-kernel, linux-acpi, geethasowjanya.akula, devel, linu.cherian, Charles.Garcia-Tobin, robh, Geetha Sowjanya 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126 2017-06-22 18:22 ` Will Deacon @ 2017-06-23 6:21 ` Geetha Akula 0 siblings, 0 replies; 14+ messages in thread From: Geetha Akula @ 2017-06-23 6:21 UTC (permalink / raw) To: Will Deacon Cc: Geetha sowjanya, Robin Murphy, Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Linux IOMMU, Robert Moore, Lv Zheng, Rafael J. Wysocki, jcm, linux-kernel, Robert Richter, Catalin Marinas, Sunil Goutham, linux-arm-kernel, linux-acpi, devel, Linu Cherian, Charles Garcia-Tobin, Rob Herring, Geetha Sowjanya <geet> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds [not found] ` <1498133138-20244-1-git-send-email-gakula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org> 2017-06-22 12:05 ` [PATCH v9 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model Geetha sowjanya 2017-06-22 12:05 ` [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126 Geetha sowjanya @ 2017-06-22 18:22 ` Will Deacon 2017-06-22 18:58 ` Will Deacon 2 siblings, 1 reply; 14+ messages in thread From: Will Deacon @ 2017-06-22 18:22 UTC (permalink / raw) To: Geetha sowjanya Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, robh-DgEjT+Ai2ygdnm+yROfE0A, devel-E0kO6a4B6psdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, Charles.Garcia-Tobin-5wv7dgnIgG8, geethasowjanya.akula-Re5JQEeQqe8AvxtiuMwx3w, jcm-H+wXaHxf7aLQT0dZR+AlfA, linu.cherian-YGCgFSpz5w/QT0dZR+AlfA, rjw-LthD3rsA81gm4RdzfppkhA, robert.moore-ral2JQCrhuEAvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8, sgoutham-YGCgFSpz5w/QT0dZR+AlfA, robert.richter-YGCgFSpz5w/QT0dZR+AlfA, lv.zheng-ral2JQCrhuEAvxtiuMwx3w 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds 2017-06-22 18:22 ` [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Will Deacon @ 2017-06-22 18:58 ` Will Deacon 2017-06-22 19:35 ` [Devel] " Robert Richter 0 siblings, 1 reply; 14+ messages in thread From: Will Deacon @ 2017-06-22 18:58 UTC (permalink / raw) To: Geetha sowjanya Cc: robin.murphy, lorenzo.pieralisi, hanjun.guo, sudeep.holla, iommu, robert.moore, lv.zheng, rjw, jcm, linux-kernel, robert.richter, catalin.marinas, sgoutham, linux-arm-kernel, linux-acpi, geethasowjanya.akula, devel, linu.cherian, Charles.Garcia-Tobin, robh 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds 2017-06-22 18:58 ` Will Deacon @ 2017-06-22 19:35 ` Robert Richter 2017-06-22 21:04 ` Lorenzo Pieralisi 0 siblings, 1 reply; 14+ messages in thread From: Robert Richter @ 2017-06-22 19:35 UTC (permalink / raw) To: Will Deacon Cc: Geetha sowjanya, linux-arm-kernel, robh, devel, lorenzo.pieralisi, catalin.marinas, Charles.Garcia-Tobin, geethasowjanya.akula, jcm, linu.cherian, rjw, linux-kernel, linux-acpi, iommu, sgoutham, robin.murphy, 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds 2017-06-22 19:35 ` [Devel] " Robert Richter @ 2017-06-22 21:04 ` Lorenzo Pieralisi 2017-06-23 4:55 ` Robert Richter 0 siblings, 1 reply; 14+ messages in thread From: Lorenzo Pieralisi @ 2017-06-22 21:04 UTC (permalink / raw) To: Robert Richter Cc: Will Deacon, Geetha sowjanya, linux-arm-kernel, robh, devel, catalin.marinas, Charles.Garcia-Tobin, geethasowjanya.akula, jcm, linu.cherian, rjw, linux-kernel, linux-acpi, iommu, sgoutham, robin.murphy, robert.richter 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds 2017-06-22 21:04 ` Lorenzo Pieralisi @ 2017-06-23 4:55 ` Robert Richter 2017-06-23 4:59 ` [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions Robert Richter 2017-06-23 8:43 ` [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Lorenzo Pieralisi 0 siblings, 2 replies; 14+ messages in thread From: Robert Richter @ 2017-06-23 4:55 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, sgoutham-YGCgFSpz5w/QT0dZR+AlfA, Robert Richter, linu.cherian-YGCgFSpz5w/QT0dZR+AlfA, rjw-LthD3rsA81gm4RdzfppkhA, catalin.marinas-5wv7dgnIgG8, Geetha sowjanya, Will Deacon, linux-kernel-u79uwXL29TY76Z2rM5mHXA, geethasowjanya.akula-Re5JQEeQqe8AvxtiuMwx3w, linux-acpi-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, jcm-H+wXaHxf7aLQT0dZR+AlfA, Charles.Garcia-Tobin-5wv7dgnIgG8, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devel-E0kO6a4B6psdnm+yROfE0A 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 > > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions 2017-06-23 4:55 ` Robert Richter @ 2017-06-23 4:59 ` Robert Richter 2017-06-23 10:11 ` Lorenzo Pieralisi 2017-06-23 8:43 ` [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Lorenzo Pieralisi 1 sibling, 1 reply; 14+ messages in thread From: Robert Richter @ 2017-06-23 4:59 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: Will Deacon, Geetha sowjanya, linux-arm-kernel, robh, devel, catalin.marinas, Charles.Garcia-Tobin, geethasowjanya.akula, jcm, linu.cherian, rjw, linux-kernel, linux-acpi, iommu, sgoutham, robin.murphy 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions 2017-06-23 4:59 ` [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions Robert Richter @ 2017-06-23 10:11 ` Lorenzo Pieralisi 0 siblings, 0 replies; 14+ messages in thread From: Lorenzo Pieralisi @ 2017-06-23 10:11 UTC (permalink / raw) To: Robert Richter Cc: Will Deacon, Geetha sowjanya, linux-arm-kernel, robh, devel, catalin.marinas, Charles.Garcia-Tobin, geethasowjanya.akula, jcm, linu.cherian, rjw, linux-kernel, linux-acpi, iommu, sgoutham, robin.murphy 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds 2017-06-23 4:55 ` Robert Richter 2017-06-23 4:59 ` [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions Robert Richter @ 2017-06-23 8:43 ` Lorenzo Pieralisi 1 sibling, 0 replies; 14+ messages in thread From: Lorenzo Pieralisi @ 2017-06-23 8:43 UTC (permalink / raw) To: Robert Richter Cc: Robert Richter, Will Deacon, Geetha sowjanya, linux-arm-kernel, robh, devel, catalin.marinas, Charles.Garcia-Tobin, geethasowjanya.akula, jcm, linu.cherian, rjw, linux-kernel, linux-acpi, iommu, sgoutham, robin.murphy 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 > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-06-23 10:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-22 12:05 [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Geetha sowjanya
2017-06-22 12:05 ` [PATCH v9 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74 Geetha sowjanya
[not found] ` <1498133138-20244-1-git-send-email-gakula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2017-06-22 12:05 ` [PATCH v9 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model Geetha sowjanya
2017-06-22 12:05 ` [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126 Geetha sowjanya
2017-06-22 18:22 ` Will Deacon
2017-06-23 6:21 ` Geetha Akula
2017-06-22 18:22 ` [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Will Deacon
2017-06-22 18:58 ` Will Deacon
2017-06-22 19:35 ` [Devel] " Robert Richter
2017-06-22 21:04 ` Lorenzo Pieralisi
2017-06-23 4:55 ` Robert Richter
2017-06-23 4:59 ` [PATCH] iommu/arm-smmu-v3, acpi: Add temporary Cavium SMMU-V3 IORT model number definitions Robert Richter
2017-06-23 10:11 ` Lorenzo Pieralisi
2017-06-23 8:43 ` [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds Lorenzo Pieralisi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).