linux-kernel - Re: Question about SPIs' interrupt trigger type restrictions

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: <87czg0mp5z.wl-maz@kernel.org>
Date:   Thu, 26 May 2022 13:52:40 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     richard clark <richard.xnu.clark@...il.com>
Cc:     Robin Murphy <robin.murphy@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        s32@....com, leoyang.li@....com, catalin-dan.udma@....com,
        bogdan.hamciuc@....com, bogdan.folea@....com,
        ciprianmarian.costea@....com, radu-nicolae.pirea@....com,
        ghennadi.procopciuc@....com
Subject: Re: Question about SPIs' interrupt trigger type restrictions

On Thu, 26 May 2022 13:09:32 +0100,
richard clark <richard.xnu.clark@...il.com> wrote:
> 
> CC'ing some nxp guys for the S32G274A SOC...
> 
> On Thu, May 26, 2022 at 2:54 PM Marc Zyngier <maz@...nel.org> wrote:
> >
> > On Thu, 26 May 2022 04:44:41 +0100,
> > richard clark <richard.xnu.clark@...il.com> wrote:
> > >
> > > On Thu, May 26, 2022 at 3:14 AM Robin Murphy <robin.murphy@....com> wrote:
> > > >
> > > > On 2022-05-25 11:01, richard clark wrote:
> > > > > Hi Marc,
> > > > >
> > > > > For below code snippet about SPI interrupt trigger type:
> > > > >
> > > > > static int gic_set_type(struct irq_data *d, unsigned int type)
> > > > > {
> > > > >          ...
> > > > >          /* SPIs have restrictions on the supported types */
> > > > >          if ((range == SPI_RANGE || range == ESPI_RANGE) &&
> > > > >              type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING)
> > > > >                  return -EINVAL;
> > > > >          ...
> > > > > }
> > > > >
> > > > > We have a device at hand whose interrupt type is SPI, Falling edge
> > > > > will trigger the interrupt. But the request_irq(50, handler,
> > > > > IRQ_TYPE_EDGE_FALLING, ...) will return -EINVAL.
> > > > >
> > > > > The question is, why must the SPI interrupt use IRQ_TYPE_EDGE_RISING
> > > > > instead of IRQ_TYPE_EDGE_FALLING?
> > > >
> > > > Because that's what the GIC architecture[1] says. From section 1.2.1
> > > > "Interrupt Types":
> > > >
> > > > "An interrupt that is edge-triggered has the following property:
> > > >         • It is asserted on detection of a rising edge of an interrupt signal
> > >
> > > This rising edge detection is not true, it's also asserted by
> > > falling edge, just like the GICD_ICFGR register says: Changing the
> > > interrupt configuration between level-sensitive and *edge-triggered
> > > (in either direction)* at a time when there is a pending interrupt
> > > ...,
> >
> > Let me finish the sentence for you:
> >
> > <quote>
> > ... will leave the interrupt in an UNKNOWN pending state.
> > </quote>
> 
> Context sensitive(register-update leaves UNKNOWN pending) and
> 
> >
> > and the direction here is about the configuration bit, not the edge
> > direction.
> 
> with this(configuration bit: either level-sensitive or
> edge-triggered), but it doesn't matter.
> 
> >
> > > which has been confirmed by GIC-500 on my platform.
> >
> > From the GIC500 r1p1 TRM, page 2-8:
> >
> > <quote>
> > SPIs are generated either by wire inputs or by writes to the AXI4
> > slave programming interface.  The GIC-500 can support up to 960 SPIs
> > corresponding to the external spi[991:32] signal. The number of SPIs
> > available depends on the implemented configuration. The permitted
> > values are 32-960, in steps of 32. The first SPI has an ID number of
> > 32. You can configure whether each SPI is triggered on a rising edge
> > or is active-HIGH level-sensitive.
> > </quote>
> >
> > So I have no idea what you are talking about, but you definitely have
> > the wrong end of the stick. Both the architecture and the
> > implementations are aligned with what the GIC drivers do.
> 
> What I am talking about is - The SPI is triggered on a rising edge
> only, while the falling edge is not as the document says. But I've
> observed the falling edge does trigger the SPI interrupt on my
> platform (the SOC is NXP S32G274A, an external wakeup signal with high
> to low transition to wake up the SOC - 'Wakeup/Interrupt Rising-Edge
> Event Enable Register (WIREER)' and 'Wakeup/Interrupt Falling-Edge
> Event Enable Register (WIFEER)', WIFEER set 1 and WIREER  set 0
> works).

This is thus driven by an external piece of HW, which, I expect, would
perform the signal conversion.

> 
> I don't know why the GIC has such a behavior and what the subtle
> rationale is behind this, so just mark this as a record...

If you can prove that the GIC itself (and not some piece of HW on the
signal path) latches on a falling edge, then that would be a huge
bug. I would encourage you (or NXP) to report it to ARM so that it
would be fixed.

Now, given that GIC500 has been with us for over 8 years, such a bug
would have been witnessed on tons of existing systems (all the
SPI-based MSIs would trigger twice, for example). Since there has been
(to my knowledge) no report of such an issue, I seriously doubt what
you are seeing is a GIC misbehaviour.

	M.

-- 
Without deviation from the norm, progress is not possible.