Re: Interrupts in pci-aardvark

On Wednesday 31 March 2021 11:25:43 Marc Zyngier wrote:

On Wed, 31 Mar 2021 10:56:59 +0100,
Pali Rohár [off-list ref] wrote:

quoted

On Tuesday 30 March 2021 14:21:47 Marc Zyngier wrote:

quoted

On Sun, 28 Mar 2021 15:09:12 +0100,
Pali Rohár [off-list ref] wrote:

[...]

quoted

Aardvark HW allows to mask summary TOP, summary CORE, individual CORE
(PME, ERR, INTA, INTB, ...), summary MSI and individual MSI bits
interrupts, but not final 16 bit MSI interrupt number. MSI bits are low
5 bits of 16 bit interrupt number. So it is not possible to mask or
unmask MSI interrupt number X. It is possible to only mask/unmask all
MSI interrupts which low 5 bits is specific value.

If you cannot mask individual MSIs, you have two choices:

- you only support MSI-X *or* MSI (not multi-MSI) and mask interrupts
  at the device level

There is no information in available documentation how to implement
MSI-X in Root Complex mode, so currently MSI-X is not possible.

quoted

- you restrict the number of MSIs to those you can actually control,
  and that's 2^5 = 32 (which is what the driver currently supports, I
  believe).

Well, 32 MSI interrupts is not enough for new modern wifi cards. E.g.
QCA6390 (ath11k) wifi card requires 32 interrupts and when this card is
connected to 2 port PCIe packet switch then packet switch requires 3
additional interrupts (1 for downstream and 1 for each upstream = 1+2).
So in this setup at least 64 MSI interrupts are required because PCI
functions of packet switch are initialized first (take interrupts 0,1,2)
and then is initialized wifi card which requires 32 aligned interrupts
(so 32,33,...,63). This setup is common on existing A3720 hardware:
Turris Mox with Mox G module.

The fact that it is common doesn't make it less broken. Also, 32
interrupts from the same device, all squashed behind a single global
IRQ seems pretty unhelpful.

Well, we cannot do anything with it. Qualcomm has probably decided to
design their new AX cards in this way and current ath11k kernel driver
requires 32 interrupts.

The only way what can be done is to provide 32 interrupts -- what driver
and hardware of these new AX cards requires.

quoted

Currently aardvark driver unmask all MSI interrupts and does not
implement masking/unmasking callbacks for individual interrupts.
Currently it has implemented support for 32 individual interrupts.

So it is an issue if masking / unmasking stay unimplemented?

A driver is allowed to call disable_irq() at any time. Not
implementing this breaks drivers. You may not care for a particular
application, but that is still broken in general.

Of course, you could rely on the device only supporting Multi-MSI
(which is what you seem to have), in which case you can disable
interrupts for the whole device. Goodbye performance.

In the worst case, interrupt can be masked in software. Yes, it
decreases performance but now when all MSI and INTx interrupts are
squashed behind one single interrupt (which is design / limitation of
Armada 3720) it could not be worse.

Anyway, hardware allows to mask group of interrupts which same low 5bit
bits. So these interrupts can be put into equivalence class based on low
5 bits.

So at least something can be done. E.g. when all connected devices
require maximally 32 interrupts. Or when some devices require Multi-MSI
without using whole Multi mask and some other devices just individual
interrupts, then these individual interrupts can be put into different
equivalence class. Which then allows masking some of them.

quoted

quoted

quoted

Also aardvark HW allows to globally enable / disable processing of MSI
interrupts. Masking summary MSI interrupt just cause that GIC does not
trigger it but from registers I can read it (e.g. when GIC calls
aardvark interrupt handler for other non-MSI interrupt).

And I would like to ask, what is in this hierarchy from kernel point of
view "bottom part of MSI" and what is the "upper part of MSI"? As in
above diagram there are 3 MSI layers.

The upper part is the bus-specific part, PCI in your case. You don't
need to implement it.

Ok!

quoted

The bottom part controls the HW, and deals with all the masking,
acknoledgement, allocation and demuxing.

So it is basically everything related to MSI interrupts in that diagram.

quoted

quoted

And which irq enable/disable/mask/unmask/ack callbacks I need to
implement for legacy irq, bottom MSI and upper MSI domains?

You need to provide what makes sense for your HW. I would guess that
you need at least mask/unmask and most probably ack at both levels,
and of course a compose_msg callback at the bottom level.

quoted

And where should I add code which globally enable/disable receiving of
aardvark MSI interrupts? Currently it is part of aardvark driver probe
function.

Seems like the logical place to put it. The kernel deals with
individual interrupts, and not with global switches.

Global enable is needed to call also when HW resume from suspend state.
Suspend is not currently implemented, but I would like to know what is
the preferred way, where to put "MSI initialization code" which needs
to be called for "global initialization & enable" and also after wakeup
from suspend. Has struct irq_chip callback for this action? Or such code
really should be called in driver probe function and then also in driver
resume function?

The irq_chip only deals with individual interrupts, and has no
callbacks for global settings. You will have to keep your global
settings for things that are global, such as probe and resume.

Ok!