Re: [RFC PATCH] PCI pass-through fixups

[Alex Williamson <alex.williamson@hp.com>]

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.

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

Alex

Sheng - BTW my copies to you bounced with "X-Postfix; Server certificate
not verified"

[   26.462952] e1000e 0000:00:04.0: PCI INT B disabled
[   26.464083] BUG: unable to handle kernel NULL pointer dereference at 0000002d
[   26.465694] IP: [<c0208a95>] pci_find_capability+0x9/0x69
[   26.466837] *pde = 00000000 
[   26.467485] Oops: 0000 [#1] SMP 
[   26.468046] last sysfs file: /sys/block/sda/removable
[   26.468046] Modules linked in: ipv6 loop button serio_raw parport_pc snd_pcsp virtio_balloon psmouse parport snd_pcm snd_timer i2c_piix4 i2c_core snd soundcore snd_page_alloc evdev ext3 jbd mbcache sg sr_mod cdrom sd_mod piix ide_pci_generic ide_core ata_piix floppy ata_generic e1000e virtio_pci libata scsi_mod thermal processor fan thermal_sys
[   26.468046] 
[   26.468046] Pid: 2365, comm: halt Not tainted (2.6.29-net #1) 
[   26.468046] EIP: 0060:[<c0208a95>] EFLAGS: 00010286 CPU: 0
[   26.468046] EIP is at pci_find_capability+0x9/0x69
[   26.468046] EAX: 00000000 EBX: 00000000 ECX: f5c8be3c EDX: 00000010
[   26.468046] ESI: 00000010 EDI: f60a3c00 EBP: 00000000 ESP: f5c8be60
[   26.468046]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   26.468046] Process halt (pid: 2365, ti=f5c8a000 task=f5e2a2c0 task.ti=f5c8a000)
[   26.468046] Stack:
[   26.468046]  00428360 f60a3c00 f5c08360 00000000 f81a2197 00000000 00000002 c0270cbc
[   26.468046]  f60a3c00 4321fedc 28121969 f5c8a000 c020b635 f60a1058 c02614ea ffffffff
[   26.468046]  c013418c c01343bd c0122c31 0021bbc7 00000000 00000000 00000046 f707d900
[   26.468046] Call Trace:
[   26.468046]  [<f81a2197>] e1000_suspend+0x1ec/0x269 [e1000e]
[   26.468046]  [<c0270cbc>] cmos_suspend+0x77/0xae
[   26.468046]  [<c020b635>] pci_device_shutdown+0x16/0x25
[   26.468046]  [<c02614ea>] device_shutdown+0x31/0x63
[   26.468046]  [<c013418c>] kernel_power_off+0xa/0x2f
[   26.468046]  [<c01343bd>] sys_reboot+0xf6/0x16c
[   26.468046]  [<c0122c31>] try_to_wake_up+0x1ed/0x1f7
[   26.468046]  [<c01178ca>] pvclock_clocksource_read+0x48/0xa7
[   26.468046]  [<c0102026>] __switch_to+0xcd/0x14e
[   26.468046]  [<c01244de>] finish_task_switch+0x42/0xc7
[   26.468046]  [<c02eb510>] schedule+0x811/0x88b
[   26.468046]  [<c0196a6e>] dput+0x1a/0x108
[   26.468046]  [<c019b8cf>] mntput_no_expire+0x1a/0xfe
[   26.468046]  [<c0188281>] filp_close+0x4e/0x54
[   26.468046]  [<c01882eb>] sys_close+0x64/0x9f
[   26.468046]  [<c0103466>] syscall_call+0x7/0xb
[   26.468046] Code: 44 24 1c 75 06 0f b6 04 24 eb 10 fe 04 24 8b 17 8d 42 ff 85 d2 89 07 75 ab 31 c0 5b 5e 5b 5e 5f 5d c3 56 89 d6 53 89 c3 83 ec 08 <8a> 40 2d 8d 4c 24 04 88 44 24 03 8b 53 1c 8b 43 08 51 b9 06 00 
[   26.468046] EIP: [<c0208a95>] pci_find_capability+0x9/0x69 SS:ESP 0068:f5c8be60
[   26.529902] ---[ end trace 69f68df891ae55cc ]---