lkml.org 
[lkml]   [2012]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Xen-devel] [PATCH 2/5] xen/pciback: Support pci_reset_function, aka FLR or D3 support.
    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 ..



    \
     
     \ /
      Last update: 2012-01-06 17:21    [W:0.025 / U:119.988 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site