Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAE3Pknn88NZNqiaHahM4HVvTU82hdMcbhGWWb4AyXXYMbcGuaQ@mail.gmail.com>
 <3ebbe879471898218f34918b1d009887@kernel.org>
In-Reply-To: <3ebbe879471898218f34918b1d009887@kernel.org>
From:   =?UTF-8?Q?Lu=C3=ADs_Matallui?= <matallui@gmail.com>
Date:   Fri, 3 Apr 2020 05:13:35 -0600
Message-ID: <CAE3PknnmUBPfCjJUxwLaTjAqU4tYXxakvH+qZTwYYyw4SS_DiA@mail.gmail.com>
Subject: Re: Help with IRQ-MSI-IRQ bridges
To:     Marc Zyngier <maz@kernel.org>
Cc:     linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Marc,

On Fri, 3 Apr 2020 at 04:18, Marc Zyngier <maz@kernel.org> wrote:
>
> Hi Luis,
>
> On 2020-04-03 02:35, Lu=C3=ADs Matallui wrote:
> > Hi,
> >
> > I've got this SoC which uses IRQ-MSI and MSI-IRQ bridges in order to
> > get interrupts from devices external to the ARM subsystem.
> > I already got some pointers from Maz and have been able to create the
> > drivers with the stacked domains and can now see the mappings working
> > fine across domains.
> >
> > Maz pointed me to the Marvell mvebu-gicp (for my MSI controller, a.k.a
> > MSI-IRQ bridge) and to mvebu-icu for the MSI client (IRQ-MSI bridge).
> >
> > I now have the interrupts working, but it seems like I'm missing a
> > bunch of them. And therefore my device doesn't work properly.
> > The main difference between my HW and Marvell's is that my IRQs are
> > not level-triggered and the MSIs don't support the two messages for
> > level-triggered interrupts.
>
> Which is probably a very good thing, as long as all your devices
> generate
> only edge-triggered interrupts.
>
> >
> > To illustrate my system:
> >
> > DEV --line--> IRQ-MSI Bridge (MSIC) --msi--> MSI-IRQ Bridge (GICP)
> > --line--> GICv2
> >
> > For MSIC, all I can do is configure the address and data for the MSI,
> > and I believe on every rising edge of the Device IRQ, an MSI is sent.
> > For GICP, all I have is a doorbell and a way to enable/disable it, and
> > whenever the doorbell is enabled and has a value !=3D 0, the IRQ line t=
o
> > GICv2 gets asserted.
> >
> > The first thing I noticed is that when I get an interrupt, the IRQ
> > flow goes like:
> >
> >   handle_irq();
> >   irq_eoi();
> >
> > So, I guess my first question here is, how can I guarantee that I
> > don't get another MSI whilst in handle_irq()?
>
> At the GIC level, once the interrupt is Ack'd, anything that is signed
> after this ack is a separate interrupt. It will be made pending and will
> fire once the GIC driver EOIs the first one.

The thing here is, there is no Ack, or at least my irqchips are not getting
the irq_ack() callback, which is where I was expecting to clear the doorbel=
l.

>
> > If I do, then I will clear the doorbell on irq_eoi() (because that's
> > my only choice) and will lose the queued IRQs.
>
> Why do you need to do anything at the doorbell level? This is just a
> write,
> so there should be nothing to clear. If you do need to clear anything,
> then your MSI-IRQ bridge isn't stateless as it should, and you'll need
> to
> give much more details about the HW. Do you have a pointer to the TRM
> for your HW?

The hardware is really simple. On the MSI controller (GICP) side, each
interrupt only has 3 registers: 1 status, 1 mask and 1 clear. When an
MSI lands a write on the status register, it asserts the interrupt line.
The interrupt stays asserted until we clear the status (using the clear
register). The mask register is just to enable the interrupt basically.
The MSI data is really irrelevant, as long as it's non-zero we always
obtain the same result.

On the MSI client side, we only configure the MSI address and data for
a certain device interrupt line, and for each rising edge, an MSI gets issu=
ed.

>
> > It also seems that I'm missing IRQs in the beginning after probing the
> > device, and before it was working for me when I was setting up all
> > these registers manually and simply using GICv2 as my only interrupt
> > controller.
>
> Well, setting all of this in firmware is always the preferred option
> if you don't expect things to change dynamically.

Well, the solution I have now works perfectly for the configuration, becaus=
e
the MSIC gets configured by msi_compose_msg -> msi_write_msg at IRQ
allocation time and never gets touched again.

Then when the IRQ gets activated, the GICP is unmasking the interrupt but
enabling the doorbell (setting the mask register).

The only thing I really need is to intercept every MSI before the handler s=
o
I can Ack it by clearing the doorbell status register.

>
> > I do see the unmask() ops being called for all my stacked irqchips, so
> > I don't understand how I'm missing so many interrupts.
>
> unmask is just a static configuration to enable the interrupt. There
> shouldn't
> be that many calls to that later on unless an endpoint driver
> disables/enables
> interrupts by hand.
>
> Please give us a bit more details to understand the context, as there is
> only
> so much I can do with so little HW information.
>
> Thanks,
>
>          M.
> --
> Jazz is not dead. It just smells funny...

Let me know if that is good enough information. There's really not much on
the HW side.

Thanks,
Luis