On Thu, Jan 05, 2012 at 10:24:11AM +0000, Ian Campbell wrote:
> On Thu, 2012-01-05 at 00:46 +0000, Konrad Rzeszutek Wilk wrote:
> > We use the __pci_reset_function_locked to perform the action.
> > Also on attaching ("bind") and detaching ("unbind") we save and
> > restore the configuration states. When the device is disconnected
> > from a guest we use the "pci_reset_function" to also reset the
> > device before being passed to another guest.
>
> Currently I thought the toolstack was supposed to do the reset (by
> writing to the reset node in sysfs) as it was (de)assigning devices
> to/from guests. That's not to say that there isn't an argument for
> preferring to do it kernels side but it would be interesting to make
> that argument explicitly.
I am kind of on the fence on this one. The one issue I found with
the toolstack is that it sometimes is run too late (now mind you -
I haven't actually found any bugs, so this might be all ballloony).
The xen-pciback can be used to inhibit drivers from grabbing the devices
during bootup. This can be done by 'xen-pciback.hide=(02:00.0)(02:00.1)(..)'
Pretty handy as you don't have to deal with:
echo "0000:02:00.1" > /sys/bus/pci/devices/0000:02:00.1/drivers/unbind
and then later 'rmmod e1000e' to save some memory space.
The unfortunate side effect is that xen-pciback does this:
pci_enable_device()
pci_disable_device(dev)
pci_write_config_word(dev, PCI_COMMAND, 0)
... and then when the guest finally uses the device it would (this is what
the toolstack does - I think?):
echo "1" > /sys/bus/pci/devices/0000:02:00.1/reset
which calls
pci_dev_reset() - do FLR or D3, or whatnot
pci_save_state()
pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);
pci_dev_reset(dev) [with a lock held so that "reset" on SysFS cannot be done]
pci_restore_state()
which means that the PCI configuration state before the "pci_save_state"
is already influenced by what the xen-pciback had done during startup:
the pci_enable/pci_disable/pci_write_config_work(dev, PCI_COMMAND, 0).
When the 'pci_restore_state()' is called it would restore it to
what xen-pciback.hide=(xx) did. Not what the device state is before xen-pciback
gets its hand on it.
This is part of this patchset - were we make sure to save it before we do our deed.
(with those pci_save_config/pci_restore_config).
And to be fair - I hadn't seen any issues with this, so it might not be
neccessary.
The other way of making this work is to remove the
pci_write_config_work(dev, PCI_COMMAND, 0) but I hadn't seen any bugs with
that ..
>
> I guess the toolstack doesn't currently save/restore the configuration
> state, could it though? I guess the state is all available in sysfs. On
> the other hand the kernel provides us with a very handy function which
> Just Does It...
Yeah, there is one more use case - 'xm'. The 'xm' does not do 'reset' on the
SysFS so having it done in the kernel is kind of nice. But then 'xm' is on the
deprecated list so xen-unstable does not care about it. But I do since
Fedora Core 16 is using it ..
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
