linux-kernel - Re: [PATCH] msi-x: let drivers retry when not enough vectors

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]

Date:	Fri, 08 May 2009 10:28:36 +1000
From:	Michael Ellerman <michael@...erman.id.au>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	virtualization@...ts.linux-foundation.org,
	Sheng Yang <sheng@...ux.intel.com>,
	Matthew Wilcox <matthew@....cx>, kvm@...r.kernel.org,
	"Michael S. Tsirkin" <mst@...hat.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, jbarnes@...tuousgeek.org,
	Matthew Wilcox <willy@...ux.intel.com>
Subject: Re: [PATCH] msi-x: let drivers retry when not enough vectors

On Fri, 2009-05-08 at 09:25 +0930, Rusty Russell wrote:
> On Thu, 7 May 2009 07:49:53 pm Sheng Yang wrote:
> > On Thursday 07 May 2009 17:53:02 Matthew Wilcox wrote:
> > > Here's a good example.  Let's suppose you have a driver which supports
> > > two different models of cards, one has 16 MSI-X interrupts, the other
> > > has 10.  You can call pci_enable_msix() asking for 16 vectors.  If your
> > > card is model A, you get 16 interrupts.  If your card is model B, it says
> > > "you can have 10".
> 
> Sheng is absolutely right, that's a horrid API.

It's not horrid, though it is tricky - but only for drivers that care,
you can still do:

if (pci_enable_msix(..))
	bail;

> If it actually enabled that number and returned it, it might make sense (cf. 
> write() returning less bytes than you give it).  

It could do that, but I think that would be worse. The driver, on
finding out it can only get a certain number of MSIs might need to make
a decision about how it allocates those - eg. in a network driver,
sharing them between TX/RX/management.

And in practice most of the drivers just bail if they can't get what
they asked for, so enabling less than they wanted would just mean they
have to go and disable them.

> But overloading the return 
> value to save an explicit call is just ugly; it's not worth saving a few lines 
> of code at cost of making all the drivers subtle and tricksy.

Looking at just this patch, I would agree, but unfortunately it's not
that simple. The first limitation on the number of MSIs the driver can
have is the number the device supports, that's what this patch does. But
there are others, and they come out of the arch code, or even the
firmware. So to implement pci_how_many_msis(), we'd need a parallel API
all the way down to the arch code, or a flag to all the existing
routines saying "don't really allocate, just find out". That would be
horrid IMHO.

cheers

Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)