| Message ID | 1404997394-15715-3-git-send-email-david.vrabel@citrix.com |
|---|---|
| State | Rejected |
| Headers | show |
On Thu, 2014-07-10 at 14:03 +0100, David Vrabel wrote: > The standard implementation of pci_reset_function() does not try an > SBR if there are other sibling devices. This is a common > configuration for multi-function devices (e.g., GPUs with a HDMI audio > device function). > > If all the functions are co-assigned to the same domain at the same > time, then it is safe to perform an SBR to reset all functions at the > same time. > > Add a "reset" sysfs file with the same interface as the standard one > (i.e., write 1 to reset) that will try an SBR if all sibling devices > are unbound or bound to pciback. > > Note that this is weaker than the requirement for functions to be > simultaneously co-assigned, but the toolstack is expected to ensure > this. > > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > --- > Changes in v3: > - probe for function reset > - use pci_reset_bus(). > > Changes in v2: > - defer creating sysfs node to late init. > --- > drivers/xen/xen-pciback/pci_stub.c | 107 +++++++++++++++++++++++++++++++++++- > drivers/xen/xen-pciback/pciback.h | 1 + > 2 files changed, 105 insertions(+), 3 deletions(-) > > diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c > index d57a173..c7deceb 100644 > --- a/drivers/xen/xen-pciback/pci_stub.c > +++ b/drivers/xen/xen-pciback/pci_stub.c > @@ -63,6 +63,100 @@ static LIST_HEAD(pcistub_devices); > static int initialize_devices; > static LIST_HEAD(seized_devices); > > +/* > + * pci_reset_function() will only work if there is a mechanism to > + * reset that single function (e.g., FLR or a D-state transition). > + * For PCI hardware that has two or more functions but no per-function > + * reset, we can do a bus reset iff all the functions are co-assigned > + * to the same domain. > + * > + * If a function has no per-function reset mechanism the 'reset' sysfs > + * file that the toolstack uses to reset a function prior to assigning > + * the device will be missing. In this case, pciback adds its own > + * which will try a bus reset. > + * > + * Note: pciback does not check for co-assigment before doing a bus > + * reset, only that the devices are bound to pciback. The toolstack > + * is assumed to have done the right thing. > + */ > +static int __pcistub_reset_function(struct pci_dev *dev) > +{ > + struct pci_dev *pdev; > + int ret; > + > + ret = __pci_reset_function_locked(dev); > + if (ret == 0) > + return 0; > + > + if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self) > + return -ENOTTY; > + > + list_for_each_entry(pdev, &dev->bus->devices, bus_list) { What if there are buses below this one? > + if (pdev != dev && (!pdev->driver > + || strcmp(pdev->driver->name, "pciback"))) > + return -ENOTTY; > + } > + > + return pci_reset_bus(dev->bus); > +} > + > +static int pcistub_reset_function(struct pci_dev *dev) > +{ > + int ret; > + > + device_lock(&dev->dev); > + ret = __pcistub_reset_function(dev); > + device_unlock(&dev->dev); > + > + return ret; > +} > + > +static ssize_t pcistub_reset_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + unsigned long val; > + ssize_t result = strict_strtoul(buf, 0, &val); > + > + if (result < 0) > + return result; > + > + if (val != 1) > + return -EINVAL; > + > + result = pcistub_reset_function(pdev); > + if (result < 0) > + return result; > + return count; > +} > +static DEVICE_ATTR(reset, 0200, NULL, pcistub_reset_store); > + > +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; > +} So the idea here is that if pci-sysfs did not create a sysfs reset file, create one when it's bound to pciback that does a secondary bus reset instead of a reset isolated to the PCI function, right? It seems like a lot to ask of userspace to know that the extent of the reset depends on the driver that it's bound to. How does userspace figure this out when the device is bound to pciback and _does_ support a function level reset? Overloading "reset" seems like a bad idea. > + > +static void pcistub_remove_reset_file(struct pci_dev *pci) > +{ > + struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci); > + > + if (dev_data && dev_data->created_reset_file) > + device_remove_file(&pci->dev, &dev_attr_reset); > +} > + > static struct pcistub_device *pcistub_device_alloc(struct pci_dev *dev) > { > struct pcistub_device *psdev; > @@ -103,7 +197,8 @@ static void pcistub_device_release(struct kref *kref) > /* Call the reset function which does not take lock as this > * is called from "unbind" which takes a device_lock mutex. > */ > - __pci_reset_function_locked(dev); > + __pcistub_reset_function(psdev->dev); > + > if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state)) > dev_dbg(&dev->dev, "Could not reload PCI state\n"); > else > @@ -280,7 +375,7 @@ void pcistub_put_pci_dev(struct pci_dev *dev) > /* This is OK - we are running from workqueue context > * and want to inhibit the user from fiddling with 'reset' > */ > - pci_reset_function(dev); > + pcistub_reset_function(dev); > pci_restore_state(dev); > > /* This disables the device. */ > @@ -404,7 +499,7 @@ static int pcistub_init_device(struct pci_dev *dev) > dev_err(&dev->dev, "Could not store PCI conf saved state!\n"); > else { > dev_dbg(&dev->dev, "resetting (FLR, D3, etc) the device\n"); > - __pci_reset_function_locked(dev); > + __pcistub_reset_function(dev); > pci_restore_state(dev); > } > /* Now disable the device (this also ensures some private device > @@ -413,6 +508,10 @@ static int pcistub_init_device(struct pci_dev *dev) > dev_dbg(&dev->dev, "reset device\n"); > xen_pcibk_reset_device(dev); > > + err = pcistub_try_create_reset_file(dev); > + if (err < 0) > + goto config_release; > + > dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED; > return 0; > > @@ -540,6 +639,8 @@ static void pcistub_remove(struct pci_dev *dev) > > dev_dbg(&dev->dev, "removing\n"); > > + pcistub_remove_reset_file(dev); > + > spin_lock_irqsave(&pcistub_devices_lock, flags); > > xen_pcibk_config_quirk_release(dev); > diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h > index f72af87..708ade9 100644 > --- a/drivers/xen/xen-pciback/pciback.h > +++ b/drivers/xen/xen-pciback/pciback.h > @@ -42,6 +42,7 @@ struct xen_pcibk_device { > struct xen_pcibk_dev_data { > struct list_head config_fields; > struct pci_saved_state *pci_saved_state; > + bool created_reset_file; > unsigned int permissive:1; > unsigned int warned_on_write:1; > unsigned int enable_intx:1; -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/07/14 00:14, Alex Williamson wrote: > On Thu, 2014-07-10 at 14:03 +0100, David Vrabel wrote: >> The standard implementation of pci_reset_function() does not try an >> SBR if there are other sibling devices. This is a common >> configuration for multi-function devices (e.g., GPUs with a HDMI audio >> device function). >> >> If all the functions are co-assigned to the same domain at the same >> time, then it is safe to perform an SBR to reset all functions at the >> same time. >> >> Add a "reset" sysfs file with the same interface as the standard one >> (i.e., write 1 to reset) that will try an SBR if all sibling devices >> are unbound or bound to pciback. >> >> Note that this is weaker than the requirement for functions to be >> simultaneously co-assigned, but the toolstack is expected to ensure >> this. [...] >> +/* >> + * pci_reset_function() will only work if there is a mechanism to >> + * reset that single function (e.g., FLR or a D-state transition). >> + * For PCI hardware that has two or more functions but no per-function >> + * reset, we can do a bus reset iff all the functions are co-assigned >> + * to the same domain. >> + * >> + * If a function has no per-function reset mechanism the 'reset' sysfs >> + * file that the toolstack uses to reset a function prior to assigning >> + * the device will be missing. In this case, pciback adds its own >> + * which will try a bus reset. >> + * >> + * Note: pciback does not check for co-assigment before doing a bus >> + * reset, only that the devices are bound to pciback. The toolstack >> + * is assumed to have done the right thing. >> + */ >> +static int __pcistub_reset_function(struct pci_dev *dev) >> +{ >> + struct pci_dev *pdev; >> + int ret; >> + >> + ret = __pci_reset_function_locked(dev); >> + if (ret == 0) >> + return 0; >> + >> + if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self) >> + return -ENOTTY; >> + >> + list_for_each_entry(pdev, &dev->bus->devices, bus_list) { > > What if there are buses below this one? Good point. >> +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; >> +} > > So the idea here is that if pci-sysfs did not create a sysfs reset file, > create one when it's bound to pciback that does a secondary bus reset > instead of a reset isolated to the PCI function, right? It seems like a > lot to ask of userspace to know that the extent of the reset depends on > the driver that it's bound to. How does userspace figure this out when > the device is bound to pciback and _does_ support a function level > reset? Overloading "reset" seems like a bad idea. The idea is that this "reset" file will only do an SBR if the side-effect of resetting siblings is harmless. An alternate interface would be to provide "bus_reset" knobs and have the toolstack understand the bus topology and issue the appropriate bus reset if it determines it is safe and per-function reset isn't available. This seems like considerably more work both kernel and toolstack side. David -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 11, 2014 at 10:53:32AM +0100, David Vrabel wrote: > On 11/07/14 00:14, Alex Williamson wrote: > > On Thu, 2014-07-10 at 14:03 +0100, David Vrabel wrote: > >> The standard implementation of pci_reset_function() does not try an > >> SBR if there are other sibling devices. This is a common > >> configuration for multi-function devices (e.g., GPUs with a HDMI audio > >> device function). > >> > >> If all the functions are co-assigned to the same domain at the same > >> time, then it is safe to perform an SBR to reset all functions at the > >> same time. > >> > >> Add a "reset" sysfs file with the same interface as the standard one > >> (i.e., write 1 to reset) that will try an SBR if all sibling devices > >> are unbound or bound to pciback. > >> > >> Note that this is weaker than the requirement for functions to be > >> simultaneously co-assigned, but the toolstack is expected to ensure > >> this. > [...] > >> +/* > >> + * pci_reset_function() will only work if there is a mechanism to > >> + * reset that single function (e.g., FLR or a D-state transition). > >> + * For PCI hardware that has two or more functions but no per-function > >> + * reset, we can do a bus reset iff all the functions are co-assigned > >> + * to the same domain. > >> + * > >> + * If a function has no per-function reset mechanism the 'reset' sysfs > >> + * file that the toolstack uses to reset a function prior to assigning > >> + * the device will be missing. In this case, pciback adds its own > >> + * which will try a bus reset. > >> + * > >> + * Note: pciback does not check for co-assigment before doing a bus > >> + * reset, only that the devices are bound to pciback. The toolstack > >> + * is assumed to have done the right thing. > >> + */ > >> +static int __pcistub_reset_function(struct pci_dev *dev) > >> +{ > >> + struct pci_dev *pdev; > >> + int ret; > >> + > >> + ret = __pci_reset_function_locked(dev); > >> + if (ret == 0) > >> + return 0; > >> + > >> + if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self) > >> + return -ENOTTY; > >> + > >> + list_for_each_entry(pdev, &dev->bus->devices, bus_list) { > > > > What if there are buses below this one? > > Good point. You could use an pci_walk_bus(dev->bus, pcistub_pci_walk_wrapper, &arg); to check that all of them belong to pciback (see https://lkml.org/lkml/2014/7/8/782) > > >> +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; > >> +} > > > > So the idea here is that if pci-sysfs did not create a sysfs reset file, > > create one when it's bound to pciback that does a secondary bus reset > > instead of a reset isolated to the PCI function, right? It seems like a > > lot to ask of userspace to know that the extent of the reset depends on > > the driver that it's bound to. How does userspace figure this out when > > the device is bound to pciback and _does_ support a function level > > reset? Overloading "reset" seems like a bad idea. > > The idea is that this "reset" file will only do an SBR if the > side-effect of resetting siblings is harmless. > > An alternate interface would be to provide "bus_reset" knobs and have > the toolstack understand the bus topology and issue the appropriate bus > reset if it determines it is safe and per-function reset isn't > available. This seems like considerably more work both kernel and > toolstack side. In regards to alternative interface, I presume you are referring to the 'do_flr' one I posted? In that I thought it would be easier to implement this logic in the kernel since it has a pretty good idea of the bus->device topology is and better yet there is ton of good code in PCI API to use it. And it can figure these sort of things out pretty easily - like do not do the bus reset if say the devices are not all owned by pciback). See https://lkml.org/lkml/2014/7/8/782 Now the one part I hadn't thought about is 'safe'? The 'per-function reset isn't available' is certainy part of the patch. FYI, I had a different piece of code (not posted) which would add per device the 'do_flr' (so similar to 'reset') and would do the appropiate checks and do the bus reset if all the conditions were meet. As in instead of 'do_flr' in /sys/bus/pci/drivers/pciback/do_flr it would be in /sys/bus/pci/devices/0000:04:00.0/do_flr (if 04:00.0 was owned by pciback). is ce > > David -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2014-07-11 at 10:53 +0100, David Vrabel wrote: > On 11/07/14 00:14, Alex Williamson wrote: > > On Thu, 2014-07-10 at 14:03 +0100, David Vrabel wrote: > >> The standard implementation of pci_reset_function() does not try an > >> SBR if there are other sibling devices. This is a common > >> configuration for multi-function devices (e.g., GPUs with a HDMI audio > >> device function). > >> > >> If all the functions are co-assigned to the same domain at the same > >> time, then it is safe to perform an SBR to reset all functions at the > >> same time. > >> > >> Add a "reset" sysfs file with the same interface as the standard one > >> (i.e., write 1 to reset) that will try an SBR if all sibling devices > >> are unbound or bound to pciback. > >> > >> Note that this is weaker than the requirement for functions to be > >> simultaneously co-assigned, but the toolstack is expected to ensure > >> this. > [...] > >> +/* > >> + * pci_reset_function() will only work if there is a mechanism to > >> + * reset that single function (e.g., FLR or a D-state transition). > >> + * For PCI hardware that has two or more functions but no per-function > >> + * reset, we can do a bus reset iff all the functions are co-assigned > >> + * to the same domain. > >> + * > >> + * If a function has no per-function reset mechanism the 'reset' sysfs > >> + * file that the toolstack uses to reset a function prior to assigning > >> + * the device will be missing. In this case, pciback adds its own > >> + * which will try a bus reset. > >> + * > >> + * Note: pciback does not check for co-assigment before doing a bus > >> + * reset, only that the devices are bound to pciback. The toolstack > >> + * is assumed to have done the right thing. > >> + */ > >> +static int __pcistub_reset_function(struct pci_dev *dev) > >> +{ > >> + struct pci_dev *pdev; > >> + int ret; > >> + > >> + ret = __pci_reset_function_locked(dev); > >> + if (ret == 0) > >> + return 0; > >> + > >> + if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self) > >> + return -ENOTTY; > >> + > >> + list_for_each_entry(pdev, &dev->bus->devices, bus_list) { > > > > What if there are buses below this one? > > Good point. > > >> +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; > >> +} > > > > So the idea here is that if pci-sysfs did not create a sysfs reset file, > > create one when it's bound to pciback that does a secondary bus reset > > instead of a reset isolated to the PCI function, right? It seems like a > > lot to ask of userspace to know that the extent of the reset depends on > > the driver that it's bound to. How does userspace figure this out when > > the device is bound to pciback and _does_ support a function level > > reset? Overloading "reset" seems like a bad idea. > > The idea is that this "reset" file will only do an SBR if the > side-effect of resetting siblings is harmless. Let's say you have a graphics card with the GPU function assigned. The VM is running and now you want to hot-add the audio function. Is it harmless to do an SBR to get the audio function into a clean state prior to hotplug? Of course not. How does userspace know that the reset file under a device is going to do a SBR vs a function-local reset? It doesn't. This is exactly why VFIO has separate interfaces for these. An SBR is only useful on create or reset of the entire VM while a function-local reset can be used elsewhere. Thanks, Alex > An alternate interface would be to provide "bus_reset" knobs and have > the toolstack understand the bus topology and issue the appropriate bus > reset if it determines it is safe and per-function reset isn't > available. This seems like considerably more work both kernel and > toolstack side. > > David -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index d57a173..c7deceb 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -63,6 +63,100 @@ static LIST_HEAD(pcistub_devices); static int initialize_devices; static LIST_HEAD(seized_devices); +/* + * pci_reset_function() will only work if there is a mechanism to + * reset that single function (e.g., FLR or a D-state transition). + * For PCI hardware that has two or more functions but no per-function + * reset, we can do a bus reset iff all the functions are co-assigned + * to the same domain. + * + * If a function has no per-function reset mechanism the 'reset' sysfs + * file that the toolstack uses to reset a function prior to assigning + * the device will be missing. In this case, pciback adds its own + * which will try a bus reset. + * + * Note: pciback does not check for co-assigment before doing a bus + * reset, only that the devices are bound to pciback. The toolstack + * is assumed to have done the right thing. + */ +static int __pcistub_reset_function(struct pci_dev *dev) +{ + struct pci_dev *pdev; + int ret; + + ret = __pci_reset_function_locked(dev); + if (ret == 0) + return 0; + + if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self) + return -ENOTTY; + + list_for_each_entry(pdev, &dev->bus->devices, bus_list) { + if (pdev != dev && (!pdev->driver + || strcmp(pdev->driver->name, "pciback"))) + return -ENOTTY; + } + + return pci_reset_bus(dev->bus); +} + +static int pcistub_reset_function(struct pci_dev *dev) +{ + int ret; + + device_lock(&dev->dev); + ret = __pcistub_reset_function(dev); + device_unlock(&dev->dev); + + return ret; +} + +static ssize_t pcistub_reset_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct pci_dev *pdev = to_pci_dev(dev); + unsigned long val; + ssize_t result = strict_strtoul(buf, 0, &val); + + if (result < 0) + return result; + + if (val != 1) + return -EINVAL; + + result = pcistub_reset_function(pdev); + if (result < 0) + return result; + return count; +} +static DEVICE_ATTR(reset, 0200, NULL, pcistub_reset_store); + +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; +} + +static void pcistub_remove_reset_file(struct pci_dev *pci) +{ + struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci); + + if (dev_data && dev_data->created_reset_file) + device_remove_file(&pci->dev, &dev_attr_reset); +} + static struct pcistub_device *pcistub_device_alloc(struct pci_dev *dev) { struct pcistub_device *psdev; @@ -103,7 +197,8 @@ static void pcistub_device_release(struct kref *kref) /* Call the reset function which does not take lock as this * is called from "unbind" which takes a device_lock mutex. */ - __pci_reset_function_locked(dev); + __pcistub_reset_function(psdev->dev); + if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state)) dev_dbg(&dev->dev, "Could not reload PCI state\n"); else @@ -280,7 +375,7 @@ void pcistub_put_pci_dev(struct pci_dev *dev) /* This is OK - we are running from workqueue context * and want to inhibit the user from fiddling with 'reset' */ - pci_reset_function(dev); + pcistub_reset_function(dev); pci_restore_state(dev); /* This disables the device. */ @@ -404,7 +499,7 @@ static int pcistub_init_device(struct pci_dev *dev) dev_err(&dev->dev, "Could not store PCI conf saved state!\n"); else { dev_dbg(&dev->dev, "resetting (FLR, D3, etc) the device\n"); - __pci_reset_function_locked(dev); + __pcistub_reset_function(dev); pci_restore_state(dev); } /* Now disable the device (this also ensures some private device @@ -413,6 +508,10 @@ static int pcistub_init_device(struct pci_dev *dev) dev_dbg(&dev->dev, "reset device\n"); xen_pcibk_reset_device(dev); + err = pcistub_try_create_reset_file(dev); + if (err < 0) + goto config_release; + dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED; return 0; @@ -540,6 +639,8 @@ static void pcistub_remove(struct pci_dev *dev) dev_dbg(&dev->dev, "removing\n"); + pcistub_remove_reset_file(dev); + spin_lock_irqsave(&pcistub_devices_lock, flags); xen_pcibk_config_quirk_release(dev); diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h index f72af87..708ade9 100644 --- a/drivers/xen/xen-pciback/pciback.h +++ b/drivers/xen/xen-pciback/pciback.h @@ -42,6 +42,7 @@ struct xen_pcibk_device { struct xen_pcibk_dev_data { struct list_head config_fields; struct pci_saved_state *pci_saved_state; + bool created_reset_file; unsigned int permissive:1; unsigned int warned_on_write:1; unsigned int enable_intx:1;
The standard implementation of pci_reset_function() does not try an SBR if there are other sibling devices. This is a common configuration for multi-function devices (e.g., GPUs with a HDMI audio device function). If all the functions are co-assigned to the same domain at the same time, then it is safe to perform an SBR to reset all functions at the same time. Add a "reset" sysfs file with the same interface as the standard one (i.e., write 1 to reset) that will try an SBR if all sibling devices are unbound or bound to pciback. Note that this is weaker than the requirement for functions to be simultaneously co-assigned, but the toolstack is expected to ensure this. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- Changes in v3: - probe for function reset - use pci_reset_bus(). Changes in v2: - defer creating sysfs node to late init. --- drivers/xen/xen-pciback/pci_stub.c | 107 +++++++++++++++++++++++++++++++++++- drivers/xen/xen-pciback/pciback.h | 1 + 2 files changed, 105 insertions(+), 3 deletions(-)