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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 21 Jun 2017 10:08:35 +0100
From:   Will Deacon <will.deacon@....com>
To:     Geetha Akula <geethasowjanya.akula@...il.com>
Cc:     Geetha sowjanya <gakula@...iumnetworks.com>,
        Robin Murphy <robin.murphy@....com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Hanjun Guo <hanjun.guo@...aro.org>,
        Sudeep Holla <sudeep.holla@....com>,
        Linux IOMMU <iommu@...ts.linux-foundation.org>,
        Robert Moore <robert.moore@...el.com>,
        Lv Zheng <lv.zheng@...el.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>, jcm@...hat.com,
        linux-kernel@...r.kernel.org,
        Robert Richter <robert.richter@...ium.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Sunil Goutham <sgoutham@...ium.com>,
        linux-arm-kernel@...ts.infradead.org, linux-acpi@...r.kernel.org,
        devel@...ica.org, Linu Cherian <linu.cherian@...ium.com>,
        Charles Garcia-Tobin <Charles.Garcia-Tobin@....com>,
        Rob Herring <robh@...nel.org>,
        Geetha Sowjanya <geethasowjanya.akula@...ium.com>,
        marc.zyngier@....com
Subject: Re: [PATCH v8 3/3] iommu/arm-smmu-v3: Add workaround for Cavium
 ThunderX2 erratum #126

Hi Geetha,

On Wed, Jun 21, 2017 at 12:09:45PM +0530, Geetha Akula wrote:
> On Tue, Jun 20, 2017 at 11:30 PM, Will Deacon <will.deacon@....com> wrote:
> > On Tue, Jun 20, 2017 at 07:47:39PM +0530, Geetha sowjanya wrote:
> >> From: Geetha Sowjanya <geethasowjanya.akula@...ium.com>
> >>
> >> Cavium ThunderX2 SMMU doesn't support MSI and also doesn't have unique irq
> >> lines for gerror, eventq and cmdq-sync.
> >>
> >> SHARED_IRQ option 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@...iumnetworks.com>
> >> ---
> >>  .../devicetree/bindings/iommu/arm,smmu-v3.txt      |    5 ++
> >>  drivers/iommu/arm-smmu-v3.c                        |   73 ++++++++++++++++----
> >>  2 files changed, 64 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> >> index 6ecc48c..44b40e0 100644
> >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> >> @@ -55,6 +55,11 @@ the PCIe specification.
> >>                     Set for Caviun ThunderX2 silicon that doesn't support
> >>                     SMMU page1 register space.
> >>
> >> +- cavium,cn9900-broken-unique-irqline
> >> +                    : Use single irq line for all the SMMUv3 interrupts.
> >> +                   Set for Caviun ThunderX2 silicon that doesn't support
> >> +                   MSI and also doesn't have unique irq lines for gerror,
> >> +                   eventq and cmdq-sync.
> >
> > I think we're better off just supporting a new (optional) named interrupt
> > as "combined", and then allowing that to be used instead of the others.
> 
> Are you suggesting to have new name irq "combined" like gerror ?
> If yes, then this won't be possible with apci. We need to update iort spec to
> add new name irq.

I'm mainly talking about the DT binding here, but I don't see why you
can't hack drivers/acpi/arm64/iort.c like you did for the other erratum and
have it register a single interrupt called "combined" based on the model
number.

> >> +                                     arm_smmu_shared_irq_thread,
> >> +                                     IRQF_ONESHOT | IRQF_SHARED,
> >
> > Why do you need IRQF_SHARED here?
> 
> 
> +devm_request_threaded_irq(smmu->dev, irq,
> +                                       arm_smmu_combined_irq_handler,
> +                                       arm_smmu_combined_irq_thread,
> +                                       IRQF_SHARED,
> +                                       "arm-smmu-v3-combined-irq", smmu);
> 
> On multi-node system, node1 SMMU's share irq lines with node0 SMMU's.

How does that work? Are these really MSIs under the hood? If so, why didn't
you just build them as... MSIs?

I sincerely hope that you never want to support paging of DMA memory on this
platform. It will run like a dog.

Will