linux-kernel - Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2e4a6c4b-857f-1000-e7fd-327b0800fb97@gmail.com>
Date:   Mon, 15 Jun 2020 12:42:50 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Robin Murphy <robin.murphy@....com>, Lukas Wunner <lukas@...ner.de>
Cc:     Mark Brown <broonie@...nel.org>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Scott Branden <sbranden@...adcom.com>,
        Ray Jui <rjui@...adcom.com>, linux-kernel@...r.kernel.org,
        "open list:SPI SUBSYSTEM" <linux-spi@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..." 
        <bcm-kernel-feedback-list@...adcom.com>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-rpi-kernel@...ts.infradead.org>,
        Martin Sperl <kernel@...tin.sperl.org>,
        Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
Subject: Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support



On 6/15/2020 12:09 PM, Robin Murphy wrote:
> On 2020-06-08 12:41, Lukas Wunner wrote:
>> On Mon, Jun 08, 2020 at 12:11:11PM +0100, Robin Murphy wrote:
>>> And all in code that has at least one obvious inefficiency left on
>>> the table either way.
>>
>> Care to submit a patch to overcome that inefficiency?
> 
> I'll have a quick go, but without any way to measure performance impact
> (or even test for correctness) I don't fancy going too deep based purely
> on disassembly and ARM11 cycle timings.
> 
>>> This thread truly epitomises Knuth's "premature optimisation"
>>> quote... ;)
>>
>> The thread came about because it can be determined at compile time
>> whether the interrupt is going to be shared:
> 
> ...which is exactly my point - "because it can be" is anything but proof
> that avoiding a trivial check makes enough measurable difference to
> justify putting in the effort to do so.
> 
>> On the BCM2835 (Raspberry Pi 1), CONFIG_ARCH_MULTI_V6 is set and this
>> SoC doesn't have multiple bcm2835-spi instances, so no shared interrupt.
>>
>> The question is how to discern BCM2836/BCM2837 (Raspberry Pi 2/3), which
>> do not have multiple instances, and BCM2711 (Raspberry Pi 4) which does.
> 
> Hmm, how much relative importance does that have? On a 700MHz ARM11 it's
> obviously desirable to spend as little time in the IRQ handler as
> possible in order to have time left to do anything else, but on the
> other SoCs even if the IRQ remains permanently asserted it can still
> only consume 25% of the available CPU capacity, at which point the
> impact of 2-3 cycles either way at 1GHz+ seems pretty much immeasurable.
> 
>> The Raspberry Pi Foundation compiles BCM2711 kernels with
>> CONFIG_ARM_LPAE=y,
>> but Florian considered that kludgy as a discriminator and opted for
>> runtime-detection via the compatible string instead.  If you've got
>> a better idea please come forward.
>>
>> Is "optimize shared IRQ support away if IS_ENABLED(CONFIG_ARCH_MULTI_V6),
>> else leave it in" the best we can do?
> 
> In all honesty I'm starting to think it seriously might be :)

Or how about this: we slightly re-structure the interrupt handler such
that all possible interrupt conditions are explicitly handled and
terminate with a return IRQ_HANDLED, and those which are not, including
in the case of a "spurious" (because the interrupt was triggered for
another SPI controller instance), then we finish the function with
IRQ_NONE. This would not impact the performance for the BCM2835/36/37
which would still have a single controller/single interrupt line to handle.
-- 
Florian