>>> On 06.11.17 at 18:48, <govinda.ta...@oracle.com> wrote: > --- a/Documentation/ABI/testing/sysfs-driver-pciback > +++ b/Documentation/ABI/testing/sysfs-driver-pciback > @@ -11,3 +11,15 @@ Description: > #echo 00:19.0-E0:2:FF > /sys/bus/pci/drivers/pciback/quirks > will allow the guest to read and write to the configuration > register 0x0E. > + > +What: /sys/bus/pci/drivers/pciback/do_flr > +Date: Nov 2017 > +KernelVersion: 4.15 > +Contact: xen-de...@lists.xenproject.org > +Description: > + An option to perform a slot or bus reset when a PCI device > + is owned by Xen PCI backend. Writing a string of DDDD:BB:DD.F > + will cause the pciback driver to perform a slot or bus reset > + if the device supports it. It also checks to make sure that > + all of the devices under the bridge are owned by Xen PCI > + backend.
Why do you name this "do_flr" when you don't even try FLR, but go to slot or then bus reset right away. > +static int pcistub_reset_dev(struct pci_dev *dev) > +{ > + struct xen_pcibk_dev_data *dev_data; > + bool slot = false, bus = false; > + > + if (!dev) > + return -EINVAL; > + > + dev_dbg(&dev->dev, "[%s]\n", __func__); > + > + if (!pci_probe_reset_slot(dev->slot)) { > + slot = true; > + } else if (!pci_probe_reset_bus(dev->bus)) { > + /* We won't attempt to reset a root bridge. */ > + if (!pci_is_root_bus(dev->bus)) > + bus = true; > + } > + > + if (!bus && !slot) > + return -EOPNOTSUPP; > + > + if (!slot) { > + struct pcistub_args arg = { .dev = NULL, .dcount = 0 }; Neither of the two initializers is really needed - just {} will do. > + /* > + * Make sure all devices on this bus are owned by the > + * PCI backend so that we can safely reset the whole bus. > + */ > + pci_walk_bus(dev->bus, pcistub_search_dev, &arg); > + > + /* All devices under the bus should be part of pcistub! */ > + if (arg.dev) { > + dev_err(&dev->dev, "%s device on the bus is not owned > by pcistub\n", > + pci_name(arg.dev)); I think "device" is superfluous here, while "the bus" could do with replacing by something actually identifying the bus. > + return -EBUSY; > + } > + > + dev_dbg(&dev->dev, "pcistub owns %d devices on the bus\n", > + arg.dcount); Same here for "the bus", provided this log message is useful in the first place. > + } Aren't you missing an "else" here? Aiui in the "slot" case it may still be multiple devices/functions which are affected. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel