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]

Message-ID: <2e334fe2-864e-d2b9-1699-20aee5d26007@arm.com>
Date:   Wed, 21 Jun 2017 10:30:39 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Will Deacon <will.deacon@....com>,
        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>
Subject: Re: [PATCH v8 3/3] iommu/arm-smmu-v3: Add workaround for Cavium
 ThunderX2 erratum #126

On 21/06/17 10:08, Will Deacon wrote:
> 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?

More specifically, I suspect that they are made out of message-signalled
SPIs, targeting the GIC distributor directly... That's the only way I
can imagine it has been built... If I'm right, we probably have the
firmware programming the same SPI number in both nodes.

But of course, that's pure speculation.

	M.
-- 
Jazz is not dead. It just smells funny...