All of lore.kernel.org
 help / color / mirror / Atom feed
* Interrupts in pci-aardvark
@ 2021-03-28 14:09 Pali Rohár
  2021-03-30 13:21 ` Marc Zyngier
  2021-03-31 11:18 ` Pali Rohár
  0 siblings, 2 replies; 7+ messages in thread
From: Pali Rohár @ 2021-03-28 14:09 UTC (permalink / raw)
  To: Marc Zyngier, linux-pci; +Cc: Marek Behún

Hello!

I need some help with fixing interrupt handling in pci-aardvark.c
driver. If my understanding of HW is correct then whole interrupt
hierarchy for aardvark PCIe controller looks like this tree diagram:

                                     GIC
                                      |
                                      v
                                 Aardvark TOP
                                 |    |    |
                                 v    |    v
                                ...   |   ...
                                      |
                                      v
             +----+----+----+-- Aardvark CORE --+----+----+----+----+-- ...
             |    |    |    |    |    |    |    |    |    |    |
             v    v    v    v    v    v    v    v    |    v    v
            PME  ERR INTA INTB INTC INTD Link  Hot   |   ...  ...
                                         Down Reset  |
                                                     |
                                                     v
                 +----------------------------- Aardvark MSI ----------- ... ---------------+
                 |                                   |                                      |
                 v                                   v                                      v
   +------+- MSI bit0 --+- .. -+       +------+- MSI bit1 --+- .. -+          +------+- MSI bit31 -+- .. -+
   |      |      |      |      |       |      |      |      |      |          |      |      |      |      |
   v      v      v      v      v       v      v      v      v      v          v      v      v      v      v
  MSI    MSI    MSI    ...    MSI     MSI    MSI    MSI    ...    MSI        MSI    MSI    MSI    ...    MSI
0x0000 0x0020 0x0040        0xFFE0  0x0001 0x0021 0x0041        0xFFE1     0x001F 0x003F 0x005F        0xFFFF


Aardvark TOP interrupt is handled by advk_pcie_irq_handler() which then
calls advk_pcie_handle_int() for processing aardvark CORE interrupts and
then if summary MSI bit is set is called also advk_pcie_handle_msi().

When GIC triggers summary aardvark TOP interrupt then from aardvark HW I
can read which particular bits were set and therefore it is possible
that more interrupt happened. E.g. PME, ERR, INTB and MSI bit 4,5,8 can
be set at the same time. But for each MSI bit can be set only one final
16bit MSI interrupt number. So in interrupt handler I need to issue
callbacks for all those interrupts after mapping them to linux interrupt
numbers.

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.

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.

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

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Interrupts in pci-aardvark
  2021-03-28 14:09 Interrupts in pci-aardvark Pali Rohár
@ 2021-03-30 13:21 ` Marc Zyngier
  2021-03-31  9:56   ` Pali Rohár
  2021-03-31 11:18 ` Pali Rohár
  1 sibling, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2021-03-30 13:21 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-pci, Marek Behún

On Sun, 28 Mar 2021 15:09:12 +0100,
Pali Rohár <pali@kernel.org> wrote:

[...]

> 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

- 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).

> 
> 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.

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

> 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.

> 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.

Thanks,

	M.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Interrupts in pci-aardvark
  2021-03-30 13:21 ` Marc Zyngier
@ 2021-03-31  9:56   ` Pali Rohár
  2021-03-31 10:25     ` Marc Zyngier
  0 siblings, 1 reply; 7+ messages in thread
From: Pali Rohár @ 2021-03-31  9:56 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-pci, Marek Behún

On Tuesday 30 March 2021 14:21:47 Marc Zyngier wrote:
> On Sun, 28 Mar 2021 15:09:12 +0100,
> Pali Rohár <pali@kernel.org> wrote:
> 
> [...]
> 
> > 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.

> - 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.

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?

> > 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!

> 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.

> > 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.
> 
> > 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?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Interrupts in pci-aardvark
  2021-03-31  9:56   ` Pali Rohár
@ 2021-03-31 10:25     ` Marc Zyngier
  2021-03-31 11:03       ` Pali Rohár
  0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2021-03-31 10:25 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-pci, Marek Behún

On Wed, 31 Mar 2021 10:56:59 +0100,
Pali Rohár <pali@kernel.org> wrote:
> 
> On Tuesday 30 March 2021 14:21:47 Marc Zyngier wrote:
> > On Sun, 28 Mar 2021 15:09:12 +0100,
> > Pali Rohár <pali@kernel.org> wrote:
> > 
> > [...]
> > 
> > > 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.
> 
> > - 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.

> 
> 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.

> 
> > > 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!
> 
> > 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.
> 
> > > 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.
> > 
> > > 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.

	M.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Interrupts in pci-aardvark
  2021-03-31 10:25     ` Marc Zyngier
@ 2021-03-31 11:03       ` Pali Rohár
  2021-03-31 15:33         ` Marc Zyngier
  0 siblings, 1 reply; 7+ messages in thread
From: Pali Rohár @ 2021-03-31 11:03 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-pci, Marek Behún

On Wednesday 31 March 2021 11:25:43 Marc Zyngier wrote:
> On Wed, 31 Mar 2021 10:56:59 +0100,
> Pali Rohár <pali@kernel.org> wrote:
> > 
> > On Tuesday 30 March 2021 14:21:47 Marc Zyngier wrote:
> > > On Sun, 28 Mar 2021 15:09:12 +0100,
> > > Pali Rohár <pali@kernel.org> wrote:
> > > 
> > > [...]
> > > 
> > > > 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.
> > 
> > > - 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.

> > 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.

> > 
> > > > 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!
> > 
> > > 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.
> > 
> > > > 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.
> > > 
> > > > 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!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Interrupts in pci-aardvark
  2021-03-28 14:09 Interrupts in pci-aardvark Pali Rohár
  2021-03-30 13:21 ` Marc Zyngier
@ 2021-03-31 11:18 ` Pali Rohár
  1 sibling, 0 replies; 7+ messages in thread
From: Pali Rohár @ 2021-03-31 11:18 UTC (permalink / raw)
  To: Marc Zyngier, linux-pci; +Cc: Marek Behún

On Sunday 28 March 2021 16:09:12 Pali Rohár wrote:
> 
>                                      GIC
>                                       |
>                                       v
>                                  Aardvark TOP
>                                  |    |    |
>                                  v    |    v
>                                 ...   |   ...
>                                       |
>                                       v
>              +----+----+----+-- Aardvark CORE --+----+----+----+----+-- ...
>              |    |    |    |    |    |    |    |    |    |    |
>              v    v    v    v    v    v    v    v    |    v    v
>             PME  ERR INTA INTB INTC INTD Link  Hot   |   ...  ...
>                                          Down Reset  |
>                                                      |
>                                                      v
>                                                 Aardvark MSI

I have another question about PME, ERR, HP and Link interrupts.

Normally kernel sees a real PCIe Root Bridge device on PCIe bus and this
Root Bridge implements lot of functionality with INTx or MSI interrupts
which are triggered when some PME, ERR, HP or other action happens. When
INTx/MSI interrupts happens then pcie kernel drivers read from more PCIe
status registers which action happened (PME, ERR, etc...).

PCIe controller on Armada 37xx does not see any PCIe Root Bridge on the
bus, there is directly only endpoint or upstream part of packet switch.
Instead Armada 37xx provides registers which can be used to implement
virtual/emulated PCIe Root Bridge.

And now the issue is, when PME, ERR or any other action happens, GIC
triggers interrupts and from aardvark CORE bits can be read which action
happens.

But because kernel pcie drivers for AER, PME or HP expects INTx/MSI
interrupt, it is required to convert aardvark PME/ERR/... into INTA
interrupt (because emulated bridge supports only INTx). Which means that
specific action (e.g. ERR for AER) is now shared with all INTA
interrupts and kernel needs to in chain call all drivers which uses
shared INTA interrupt on IRQ domain for INTA. Which obviously degrades
performance.

Is there any way how to solve this issue without performance
degradation? E.g. it is possible to allocate special IRQ INTx domain
just for one particular PCIe device (that emulated PCIe Root Bridge)? So
when PME or ERR interrupt happens, aardvark interrupt handler would
convert it to INTA interrupt just for one device = Root Bridge.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Interrupts in pci-aardvark
  2021-03-31 11:03       ` Pali Rohár
@ 2021-03-31 15:33         ` Marc Zyngier
  0 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2021-03-31 15:33 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-pci, Marek Behún

On Wed, 31 Mar 2021 12:03:14 +0100,
Pali Rohár <pali@kernel.org> wrote:
> 
> On Wednesday 31 March 2021 11:25:43 Marc Zyngier wrote:
> > On Wed, 31 Mar 2021 10:56:59 +0100,
> > Pali Rohár <pali@kernel.org> wrote:
> > > 
> > > On Tuesday 30 March 2021 14:21:47 Marc Zyngier wrote:
> > > > On Sun, 28 Mar 2021 15:09:12 +0100,
> > > > Pali Rohár <pali@kernel.org> wrote:
> > > > 
> > > > [...]
> > > > 
> > > > > 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.
> > > 
> > > > - 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.

Which is fine. Using per-queue interrupts is standard procedure if you
want any kind of performance. But it also assume that the HW this is
plugged in is not braindead. Unfortunately, the Marvell stuff ticks
all the boxes for being a terrible implementation.

>
> > > 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.

You could do that. It will require you to refcount the
enabling/disabling of interrupts, but that's not too hard to get
right. My concern is more if *one* interrupt gets disabled, and that
the driver still expects other interrupts to fire...

Thanks,

	M.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-31 15:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-28 14:09 Interrupts in pci-aardvark Pali Rohár
2021-03-30 13:21 ` Marc Zyngier
2021-03-31  9:56   ` Pali Rohár
2021-03-31 10:25     ` Marc Zyngier
2021-03-31 11:03       ` Pali Rohár
2021-03-31 15:33         ` Marc Zyngier
2021-03-31 11:18 ` Pali Rohár

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.