Re: [RFC PATCH] PCI pass-through fixups

[Chris Wright <chrisw@sous-sol.org>]

* Alex Williamson (alex.williamson@hp.com) wrote:
> On Tue, 2009-04-07 at 07:54 -0700, Chris Wright wrote:
> > * Sheng Yang (sheng@linux.intel.com) wrote:
> > > On Tuesday 07 April 2009 08:02:10 Chris Wright wrote:
> > > > * Alex Williamson (alex.williamson@hp.com) wrote:
> > > > > I'm wondering if we need a spot for device specific fixups for PCI
> > > > > pass-through.  In the example below, I want to expose a single port of
> > > > > an Intel 82571EB quad port copper NIC to a guest.  It works great until
> > > > > I shutdown the guest, at which point the guest e1000e driver knows by
> > > > > the device ID that the NIC is a quad port, and blindly attempts to
> > > > > twiddle some bits on the bridge above it (that doesn't exist).
> > > >
> > > > And what happens?
> > > 
> > > And the output of DEVICE_ASSIGNMENT_DEBUG=1 may help. And I don't have trouble 
> > > here with another dual/quad port card by Intel, but not 82571EB. 
> > 
> > Right, it's driver dependent.  I too can assign a single port w/out
> > trouble.  I'm assuming the guest just complains at rmmod?
> 
> No, it's a little more severe than that, see below.  This may be rather
> unique, I certainly wasn't expecting different device IDs for a quad
> port versus a single port.  However, you can see in
> e1000e/82571.c:e1000_get_variants_82571() we set a flag for quad port
> device IDs, apparently for WOL only being supported on the first port.
> Then when we shutdown, we hit e1000e/netdev.c:e1000_suspend() where we
> take that quad port flag and blindly walk up to bus->self, which is
> NULL, and try to get the PCI capabilities on it.

That's a form of guest complaint ;-)  Yes, I noticed the global wol bit,
but didn't see the bus->self...ick

> It may be prudent for the driver to check the pointer, but there's
> probably also an argument that the device ID indicates something about
> the topology that makes that unnecessary, so we may hit the same problem
> in drivers we don't have source for.  I agree that my rough sketch of a
> patch is pretty fragile and static.  Maybe an option to override a
> device ID on the command line would be more flexible.  Something like:
> 
> -pcidevice host=09:00.0,device_id=0x105E
> 
> This puts the burden on the user, but at least allows us an out.

Still ugly, esp since there's no real way to know ahead of time