linux-kernel - Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAK9rFnz6X3wR6bMPkVicGoqj+prm5OmY3y3B_8Fc+WGJxcVsXA@mail.gmail.com>
Date:   Mon, 8 Nov 2021 11:35:14 -0800
From:   Brad Larson <brad@...sando.io>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Mark Rutland <mark.rutland@....com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Mark Brown <broonie@...nel.org>,
        Serge Semin <fancer.lancer@...il.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Olof Johansson <olof@...om.net>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        linux-spi <linux-spi@...r.kernel.org>,
        linux-mmc <linux-mmc@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

Hi Mark,

On Fri, Nov 5, 2021 at 4:35 AM Marc Zyngier <maz@...nel.org> wrote:
>
> > > >> +                    interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > > >> +
> > > >> +                    gic_its: msi-controller@...000 {
> > > >> +                            compatible = "arm,gic-v3-its";
> > > >> +                            msi-controller;
> > > >> +                            #msi-cells = <1>;
> > > >> +                            reg = <0x0 0x820000 0x0 0x10000>;
> > > >> +                            socionext,synquacer-pre-its =
> > > >> +                                                    <0xc00000 0x1000000>;
> > > >> +                    };
> > > >> +            };
> > > >
> > > > Is there any shared lineage with Synquacer? The commit message didn't
> > > > describe this quirk.
> > >
> > > Funny, it looks like there is a sudden outburst of stupid copy/paste
> > > among HW designers. TI did the exact same thing recently.
> > >
> > > This totally negates all the advantages of having an ITS and makes
> > > sure that you have all the overhead. Facepalm...
> >
> > Some background may help explain.  To generate an LPI a peripheral must
> > write to the GITS_TRANSLATER (a specific address). For the ITS to know
> > which translations apply to the generated interrupts, it must know which
> > peripheral performed the write. The ID of the peripheral is known as its
> > DeviceID, which is often carried along with the write as an AXI sideband
> > signal.
>
> Yes, I happen to be vaguely familiar with the GIC architecture.
>
> > The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
> > between the peripheral and the ITS.  Instead of telling a peripheral to target
> > the GITS_TRANSLATER directly, we instead direct it to a specific offset
> > within a pre-ITS address range (our own IP block).  For writes that land in
> > that memory range, we derive the DeviceID from (offset >> 2).  The pre-ITS
> > block then sends (DeviceID, data) to the GITS_TRANSLATER.
> >
> > The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
> > When we looked at the upstream kernel later (we developed on 4.14)
> > we found that not only did it support something similar, it supported the
> > exact scheme we are using.
>
> And this scheme is totally wrong. It breaks interrupt isolation.
>
> Instead of having a single doorbell and getting the ITS to segregate
> between devices itself, you end-up with multiple ones, allowing a
> rogue device to impersonate another one by targeting another doorbell.
> You can't even use an SMMU to preserve some isolation, because all the
> doorbells are in the *same page*. Unmitigated disaster.
>
> At this stage, why did you bother having an ITS at all? You get none
> of the security features. Only the excess area, memory allocation,
> additional latency and complexity. All you get is a larger INTID
> space.
>
> This only shows that the hardware designer didn't understand the ITS
> at all. Which seems a common pattern, unfortunately.

The Elba SoC is an embedded chip and not intended as a SBSA-compliant
general platform.  In this implementation the ITS is used to provide
message-based interrupts for our (potentially large set) of hardware
based platform device instances.  Virtualization is not a consideration.
We don't have a SMMU.  Interrupt isolation isn't a practical consideration
for this product.  Propose adding a comment to the dts.

+                       /*
+                        * Elba SoC implemented a pre-ITS that happened to
+                        * be the same implementation as synquacer.
+                        */
                        its: interrupt-controller@...000 {
                                compatible = "arm,gic-v3-its";
                                msi-controller;

Thanks
Brad