Hi Mark,
On Fri, Nov 5, 2021 at 4:35 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> > > >> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > > >> +
> > > >> + gic_its: msi-controller@820000 {
> > > >> + 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@820000 {
compatible = "arm,gic-v3-its";
msi-controller;
Thanks
Brad
