Messages in this thread | | | Date | Thu, 4 Dec 2014 14:10:54 +0100 | From | Sander Eikelenboom <> | Subject | Re: [Xen-devel] [PATCH v5 9/9] xen/pciback: Implement PCI reset slot or bus with 'do_flr' SysFS attribute |
| |
Thursday, December 4, 2014, 1:24:47 PM, you wrote:
> On 04/12/14 12:06, Konrad Rzeszutek Wilk wrote: >> >> On Dec 4, 2014 6:30 AM, David Vrabel <david.vrabel@citrix.com> wrote: >>> >>> On 03/12/14 21:40, Konrad Rzeszutek Wilk wrote: >>>> >>>> Instead of doing all this complex dance, we depend on the toolstack >>>> doing the right thing. As such implement the 'do_flr' SysFS attribute >>>> which 'xl' uses when a device is detached or attached from/to a guest. >>>> It bypasses the need to worry about the PCI lock. >>> >>> No. Get pciback to add its own "reset" sysfs file (as I have repeatedly >>> proposed). >>> >> >> Which does not work as the kobj will complain (as there is already an 'reset' associated with the PCI device).
> It is only needed if the core won't provide one.
> +static int pcistub_try_create_reset_file(struct pci_dev *pci) > +{ > + struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci); > + struct device *dev = &pci->dev; > + int ret; > + > + /* Already have a per-function reset? */ > + if (pci_probe_reset_function(pci) == 0) > + return 0; > + > + ret = device_create_file(dev, &dev_attr_reset); > + if (ret < 0) > + return ret; + dev_data->>created_reset_file = true; > + return 0; > +}
Wouldn't the "core-reset-sysfs-file" be still wired to the end up calling "pci.c:__pci_dev_reset" ?
The problem with that function is that from my testing it seems that the first option "pci_dev_specific_reset" always seems to return succes, so all the other options are skipped (flr, pm, slot, bus). However the device it self is not properly reset enough (perhaps the pci_dev_specific_reset is good enough for none virtualization purposes and it's probably the least intrusive. For virtualization however it would be nice to be sure it resets properly, or have a way to force a specific reset routine.)
So it's the ordering and skipping of the other resets that seems to make this workaround necessary in the first place.
> David
| |