Re: [Devel] [RESEND PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
On 27.06.17 20:28:14, Geetha Akula wrote: > On Tue, Jun 27, 2017 at 7:36 PM, Will Deacon wrote: > > On Tue, Jun 27, 2017 at 03:56:10PM +0200, Robert Richter wrote: > >> On 23.06.17 19:04:36, Geetha sowjanya wrote: > >> > From: Geetha Sowjanya > >> > > >> > 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 > >> > --- > >> > Documentation/arm64/silicon-errata.txt |1 + > >> > .../devicetree/bindings/iommu/arm,smmu-v3.txt |6 + > >> > drivers/acpi/arm64/iort.c | 57 --- > >> > drivers/iommu/arm-smmu-v3.c| 100 > >> > ++- > >> > 4 files changed, 121 insertions(+), 43 deletions(-) > >> > >> > +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, > >> > >> Without the IRQF_SHARED flag set I see the following on a dual node > >> system now: > Node1 SMMU interrupts are programmed wrong in the firmware. > Node 0 and Node1 SMMU do not share interrupts. > I have verified the patch on dual node with correct interrupt numbers > programmed in firmware. Ah, ok, will update to latest fw. Thanks, -Robert
Re: [Devel] [RESEND PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
On Tue, Jun 27, 2017 at 7:36 PM, Will Deacon wrote: > On Tue, Jun 27, 2017 at 03:56:10PM +0200, Robert Richter wrote: >> On 23.06.17 19:04:36, Geetha sowjanya wrote: >> > From: Geetha Sowjanya >> > >> > 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 >> > --- >> > Documentation/arm64/silicon-errata.txt |1 + >> > .../devicetree/bindings/iommu/arm,smmu-v3.txt |6 + >> > drivers/acpi/arm64/iort.c | 57 --- >> > drivers/iommu/arm-smmu-v3.c| 100 >> > ++- >> > 4 files changed, 121 insertions(+), 43 deletions(-) >> >> > +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, >> >> Without the IRQF_SHARED flag set I see the following on a dual node >> system now: Node1 SMMU interrupts are programmed wrong in the firmware. Node 0 and Node1 SMMU do not share interrupts. I have verified the patch on dual node with correct interrupt numbers programmed in firmware. Thanks, Geetha. > > We asked about that before: > > https://marc.info/?l=linux-arm-kernel&m=149803613513068&w=2 > https://marc.info/?l=linux-acpi&m=149803744713475&w=2 > > and Geetha didn't reply, but the next version of the patch dropped the flag. > Is it just that firmware is misprogramming something here, or something > more fundamental? > > Will
Re: [Devel] [RESEND PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
On Tue, Jun 27, 2017 at 03:56:10PM +0200, Robert Richter wrote: > On 23.06.17 19:04:36, Geetha sowjanya wrote: > > From: Geetha Sowjanya > > > > 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 > > --- > > Documentation/arm64/silicon-errata.txt |1 + > > .../devicetree/bindings/iommu/arm,smmu-v3.txt |6 + > > drivers/acpi/arm64/iort.c | 57 --- > > drivers/iommu/arm-smmu-v3.c| 100 > > ++- > > 4 files changed, 121 insertions(+), 43 deletions(-) > > > +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, > > Without the IRQF_SHARED flag set I see the following on a dual node > system now: We asked about that before: https://marc.info/?l=linux-arm-kernel&m=149803613513068&w=2 https://marc.info/?l=linux-acpi&m=149803744713475&w=2 and Geetha didn't reply, but the next version of the patch dropped the flag. Is it just that firmware is misprogramming something here, or something more fundamental? Will
Re: [Devel] [RESEND PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
On 23.06.17 19:04:36, Geetha sowjanya wrote: > From: Geetha Sowjanya > > 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 > --- > Documentation/arm64/silicon-errata.txt |1 + > .../devicetree/bindings/iommu/arm,smmu-v3.txt |6 + > drivers/acpi/arm64/iort.c | 57 --- > drivers/iommu/arm-smmu-v3.c| 100 ++- > 4 files changed, 121 insertions(+), 43 deletions(-) > +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, Without the IRQF_SHARED flag set I see the following on a dual node system now: [ 17.403260] arm-smmu-v3 arm-smmu-v3.0.auto: option mask 0x2 [ 17.408853] arm-smmu-v3 arm-smmu-v3.0.auto: ias 44-bit, oas 44-bit (features 0x172d) [ 17.417259] arm-smmu-v3 arm-smmu-v3.1.auto: option mask 0x2 [ 17.422856] arm-smmu-v3 arm-smmu-v3.1.auto: ias 44-bit, oas 44-bit (features 0x172d) [ 17.431193] arm-smmu-v3 arm-smmu-v3.2.auto: option mask 0x2 [ 17.436787] arm-smmu-v3 arm-smmu-v3.2.auto: ias 44-bit, oas 44-bit (features 0x172d) [ 17.445123] arm-smmu-v3 arm-smmu-v3.3.auto: option mask 0x2 [ 17.450697] arm-smmu-v3 arm-smmu-v3.3.auto: ias 44-bit, oas 44-bit (features 0x172d) [ 17.458984] genirq: Flags mismatch irq 5. 2001 (arm-smmu-v3-combined-irq) vs. 2001 (arm-smmu-v3-combined-irq) [ 17.610636] arm-smmu-v3 arm-smmu-v3.3.auto: failed to enable combined irq [ 17.617491] arm-smmu-v3 arm-smmu-v3.4.auto: option mask 0x2 [ 17.623073] arm-smmu-v3 arm-smmu-v3.4.auto: ias 44-bit, oas 44-bit (features 0x172d) [ 17.631339] genirq: Flags mismatch irq 6. 2001 (arm-smmu-v3-combined-irq) vs. 2001 (arm-smmu-v3-combined-irq) [ 17.782944] arm-smmu-v3 arm-smmu-v3.4.auto: failed to enable combined irq [ 17.789780] arm-smmu-v3 arm-smmu-v3.5.auto: option mask 0x2 [ 17.795367] arm-smmu-v3 arm-smmu-v3.5.auto: ias 44-bit, oas 44-bit (features 0x172d) [ 17.803640] genirq: Flags mismatch irq 7. 2001 (arm-smmu-v3-combined-irq) vs. 2001 (arm-smmu-v3-combined-irq) [ 17.955248] arm-smmu-v3 arm-smmu-v3.5.auto: failed to enable combined irq In __setup_irq() in kernel/irq/manage.c there is a check if that flag is set which causes the error is shown. -Robert > + "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,
[RESEND PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
From: Geetha Sowjanya 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 --- Documentation/arm64/silicon-errata.txt |1 + .../devicetree/bindings/iommu/arm,smmu-v3.txt |6 + drivers/acpi/arm64/iort.c | 57 --- drivers/iommu/arm-smmu-v3.c| 100 ++- 4 files changed, 121 insertions(+), 43 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 e7855cf..c9abbf3 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt @@ -26,6 +26,12 @@ the PCIe specification. * "priq" - PRI Queue not empty * "cmdq-sync" - CMD_SYNC complete * "gerror"- Global Error activated + * "combined" - 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 bd97b7d..21c1ed3 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,39 @@ static void __init arm_smmu_v3_init_resources(struct resource *res, res[num_res].flags = IORESOURCE_MEM; num_res++; + if (arm_smmu_v3_is_combined_irq(smmu)) { + int irq = smmu->event_gsiv; - 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", + if (!(irq == smmu->pri_gsiv && irq == smmu->gerr_gsiv && + irq == smmu->sync_gsiv)) { + pr_warn(FW_BUG "Incosistent combined GSIV configuration\n"); + return; + } + acpi_iort_register_irq(smmu->event_gsiv, "combined", ACPI_EDGE_SENSITIVE, &res[num_res++]); + } else { - if (smmu->sync_gsiv) - acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync", - ACPI_EDGE_SENSITIVE, - &res[num_res++]); + if (smmu->event_gsiv) + acpi_iort_register_irq(smmu->event_gsiv, "eventq", + ACPI_EDGE_SENSITIVE, + &
Re: [PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
On Fri, Jun 23, 2017 at 11:28:05AM +0530, Geetha sowjanya wrote: [...] > --- 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++]); I think you should add a check to make sure ALL gsiv are the same in the IORT table, if they are not the combined IRQ must not be registered. Ideally we should add a patch that adds a return value to the iommu_init_resource callback (ie so that we can prevent allocating the platform device on error return value) but we are late -rc6 and I won't be able to read mailing lists next week so it is a bit tight, adding the check above should be quick to do. Lorenzo > + 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..fd5a48c 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_priqpriq; > > int gerr_irq; > + int combined_irq; > > unsigned long ias; /* IPA */ > unsigned long oas; /* PA */ > @@ -1314,6 +1315,24 @@ 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) > +{ > + arm_smmu_gerror_handler(irq, dev); > + arm_smmu_cmdq_sync_handler(irq, dev); > + return IRQ_WAKE_THREAD; > +} > + > /* IO_PGTABLE API */ > static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu) > { > @@ -2230,18 +2249,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; > -
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 wrote: > Hi Geetha, > > On Thu, Jun 22, 2017 at 05:35:38PM +0530, Geetha sowjanya wrote: >> From: Geetha Sowjanya >> >> 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 >> --- >> 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) >> +
[PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
From: Geetha Sowjanya 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 --- Documentation/arm64/silicon-errata.txt |1 + .../devicetree/bindings/iommu/arm,smmu-v3.txt |6 + drivers/acpi/arm64/iort.c | 54 +++ drivers/iommu/arm-smmu-v3.c| 100 ++- 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 e7855cf..c9abbf3 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt @@ -26,6 +26,12 @@ the PCIe specification. * "priq" - PRI Queue not empty * "cmdq-sync" - CMD_SYNC complete * "gerror"- Global Error activated + * "combined" - 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 bd97b7d..4e9523b 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) +
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 > > 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 > --- > 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) > +
[PATCH v9 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
From: Geetha Sowjanya 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 --- 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