MSI and driver APIs

MSI and driver APIs

[Benjamin Herrenschmidt]

I've been looking at MSI/MSI-X support on POWER platforms, both under
hypervisor or directly on machines like the new G5s and I found out that
the current code in drivers/pci isn't nearly as generic as it claims to
be and cannot really be re-used as is.

So I want to start looking into separate implementation for powerpc, and
based on what I come up with, find the commonalities and split the
generic code. However, there is at least one assumption that annoys me:

Currently, we assume that MSIs are disabled upon discovery of a device.
That is, a driver probe() routine is called with MSIs off.

This is annoying on platforms with "intelligent" firmwares like POWER
with hypervisor, as the firmware will have already configured MSIs for
the full system & assigned them to devices.

The current scenario basically means that I have to walk through all
devices, disable MSI if it was enabled by the firmware, and re-assign
MSIs later on upon driver request. In addition to that, the firmware
goes at lenght to guarantee that at least one MSI can be allocated per
device. If the OS releases those MSIs by disabling them, however, they
are returned to a global pool. When we later on request MSIs again from
that pool, there is no guarantee we don't run out and thus no guarantee
we succeed in allocating those MSIs.

Thus I would very much like to change the semantics so that a driver can
be entered with MSIs already assigned and enabled, though it has the
capability to request more MSIs and/or to disable them if the chipset is
buggy. That could be done either by adding a callback to check if MSIs
are enabled for a given device for example...

Another solution would be for me to implement a "trick" at the
architecture level. Since MSIs assigned to a device will have different
interrupt numbers than it's legacy PCI interrupt, I could leave the MSIs
enabled at probe() time, and only disable them once the driver does a
request_irq() on the legacy interrupt (since MSI and old style
interrupts are exclusive on a given device).

Of course, a subsequent pci_enable_msi/x() would re-enable/re-allocate
them, but I don't expect drivers to do that _and_ request the legacy
interrupt...

Or maybe somebody has better ideas ?

Regards,
Ben.

Re: MSI and driver APIs

[Roland Dreier]

    Benjamin> I've been looking at MSI/MSI-X support on POWER
    Benjamin> platforms, both under hypervisor or directly on machines
    Benjamin> like the new G5s and I found out that the current code
    Benjamin> in drivers/pci isn't nearly as generic as it claims to
    Benjamin> be and cannot really be re-used as is.

Excellent!  I've always wanted to find time to make the code generic
and add MSI support for some new platform like PPC 440SPe, but it's
never made it very far up my list.

    Benjamin> Thus I would very much like to change the semantics so
    Benjamin> that a driver can be entered with MSIs already assigned
    Benjamin> and enabled, though it has the capability to request
    Benjamin> more MSIs and/or to disable them if the chipset is
    Benjamin> buggy. That could be done either by adding a callback to
    Benjamin> check if MSIs are enabled for a given device for
    Benjamin> example...

It seems OK to me to say that a driver's probe routine could be called
with MSI enabled.  A naive driver would just use the irq number from
the PCI device struct and never care whether interrupts were INTx or
MSI.  This does fall down for hardware like tg3, where something
beyond the simple PCI header manipulation is required to turn on MSI use.

Full MSI-X would be much harder to handle transparently, since
handling multiple different interrupts typically requires a lot more
logic in the driver.

 - R.

Re: MSI and driver APIs

[Benjamin Herrenschmidt]

> It seems OK to me to say that a driver's probe routine could be called
> with MSI enabled.  A naive driver would just use the irq number from
> the PCI device struct and never care whether interrupts were INTx or
> MSI.  This does fall down for hardware like tg3, where something
> beyond the simple PCI header manipulation is required to turn on MSI use.
> 
> Full MSI-X would be much harder to handle transparently, since
> handling multiple different interrupts typically requires a lot more
> logic in the driver.

You can have multiple MSIs too (they just have to be contiguous in
numbers and aligned on the nearest power of 2).

I'm tempted to leave them enabled and only disable them when
request_irq() is done on the legacy INTx... Does anybody see a problem
with this approach ?

Ben.

Re: MSI and driver APIs

[Roland Dreier]

    Benjamin> You can have multiple MSIs too (they just have to be
    Benjamin> contiguous in numbers and aligned on the nearest power
    Benjamin> of 2).

Good point.  I just got too used to the x86-ism of the current MSI
code, which can't handle allocating contiguous vectors.

    Benjamin> I'm tempted to leave them enabled and only disable them
    Benjamin> when request_irq() is done on the legacy INTx... Does
    Benjamin> anybody see a problem with this approach ?

You might run into trouble on hardware (think tg3 or its ilk again)
where you might have to do something beyond disabling MSI in the PCI
header to switch the chip out of MSI mode.

 - R.

Re: MSI and driver APIs

[Benjamin Herrenschmidt]

On Thu, 2005-12-15 at 13:18 -0800, Roland Dreier wrote:
>     Benjamin> You can have multiple MSIs too (they just have to be
>     Benjamin> contiguous in numbers and aligned on the nearest power
>     Benjamin> of 2).
> 
> Good point.  I just got too used to the x86-ism of the current MSI
> code, which can't handle allocating contiguous vectors.
> 
>     Benjamin> I'm tempted to leave them enabled and only disable them
>     Benjamin> when request_irq() is done on the legacy INTx... Does
>     Benjamin> anybody see a problem with this approach ?
> 
> You might run into trouble on hardware (think tg3 or its ilk again)
> where you might have to do something beyond disabling MSI in the PCI
> header to switch the chip out of MSI mode.

But won't the driver call pci_enable/disable_msi() in those cases ? If
not, it's easy enough to add (explicit disable rather than not-enabled).

I'm mostly concerned about "dumb" drivers that don't know about MSI at
all...

Ben.

Re: MSI and driver APIs

[Roland Dreier]

    Benjamin> But won't the driver call pci_enable/disable_msi() in
    Benjamin> those cases ? If not, it's easy enough to add (explicit
    Benjamin> disable rather than not-enabled).

    Benjamin> I'm mostly concerned about "dumb" drivers that don't
    Benjamin> know about MSI at all...

Well, a driver for a chip that does MSI may be "dumb" in that sense.
For example, tg3 only got MSI support in April '05 or so.

Although looking at tg3.c, it seems that nothing special is required
to disable MSI -- there is a special chip register bit to set to
enable MSI mode, but it doesn't seem necessary to clear the bit to
disable MSI.

The case I'm worried about is a chip where something special has to be
done to get out of MSI mode, but the driver is totally dumb and just
does request_irq() on its legacy interrupt.  The core PCI code doesn't
have the chip-specific knowledge to fully turn off MSI.

But I don't know of a real device that falls into that category, so
your scheme is probably OK.

 - R.

Re: MSI and driver APIs

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Thu, 15 Dec 2005 13:18:04 -0800

>     Benjamin> I'm tempted to leave them enabled and only disable them
>     Benjamin> when request_irq() is done on the legacy INTx... Does
>     Benjamin> anybody see a problem with this approach ?
> 
> You might run into trouble on hardware (think tg3 or its ilk again)
> where you might have to do something beyond disabling MSI in the PCI
> header to switch the chip out of MSI mode.

I think because of kinds of cases and other issues, going with
MSI by default is a non-starter.

Perhaps a better approach is to use a flag in the pci_driver_struct or
similar that says "you can have MSI enabled by default".  And
gradually convert drivers over which we know will handle it properly.

Doing some tom foolery with request_irq() sounds like a half-baked
idea at best.  The biggest argument against that is that this is
not a PCI interface, so expecting it to have PCI side effects is
really asking for trouble.

Re: MSI and driver APIs

[Benjamin Herrenschmidt]

> I think because of kinds of cases and other issues, going with
> MSI by default is a non-starter.
> 
> Perhaps a better approach is to use a flag in the pci_driver_struct or
> similar that says "you can have MSI enabled by default".  And
> gradually convert drivers over which we know will handle it properly.
> 
> Doing some tom foolery with request_irq() sounds like a half-baked
> idea at best.  The biggest argument against that is that this is
> not a PCI interface, so expecting it to have PCI side effects is
> really asking for trouble.

Hrm... true enough. I'll look into the driver flags option. I can
probably always fallback to just turning MSI off everywhere at boot time
and "reserve" an MSI number per device by simply holding on what was
allocated by the firmware.

I was thinking that I might be able to not return the firmware allocated
MSIs to the firmware, but just disable MSI in the device config space
myself and keep track that N MSIs are still associated with the device
even if not currently used. However, the way the IBM architecture is
worded, it's unclear if that will work, that is, it's unclear that if I
don't actually disable the MSIs with a firmware call (and thus return
them), the interrupt controller will accept the INTx, chances are that
it won't in fact.

Ben.

Re: MSI and driver APIs

[Greg KH]

On Thu, Dec 15, 2005 at 02:38:12PM +1100, Benjamin Herrenschmidt wrote:
> So I want to start looking into separate implementation for powerpc, and
> based on what I come up with, find the commonalities and split the
> generic code.

That would be great, as it really is i386/x86-64 specific right now.

> However, there is at least one assumption that annoys me:
> 
> Currently, we assume that MSIs are disabled upon discovery of a device.
> That is, a driver probe() routine is called with MSIs off.
> 
> This is annoying on platforms with "intelligent" firmwares like POWER
> with hypervisor, as the firmware will have already configured MSIs for
> the full system & assigned them to devices.

If you are curious as to why this is, look at the lkml archives about a
month or two ago, where I tried to enable MSI by default.  It was a real
mess and caused way more problems than it tried to solve.

So, as David said, I don't think this is going to work out, sorry.

thanks,

greg k-h