| Message ID | 20130814200549.21626.79779.stgit@bling.home (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Wed, Aug 14, 2013 at 2:06 PM, Alex Williamson <alex.williamson@redhat.com> wrote: > Users of pci_reset_bus() and pci_reset_slot() need a way to probe > whether the bus or slot supports reset. Add trivial helper functions > and export them as vfio-pci will make use of these. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > > This is logically patch 10/9 for v5 pci: bus and slot reset interfaces. > If there's a need to re-spin anything I'll roll this in. > > drivers/pci/pci.c | 24 ++++++++++++++++++++++++ > include/linux/pci.h | 2 ++ > 2 files changed, 26 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index ba68451..888d847 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3610,6 +3610,18 @@ static int pci_slot_reset(struct pci_slot *slot, int probe) > } > > /** > + * pci_probe_reset_slot - probe whether a PCI slot can be reset > + * @slot: PCI slot to probe > + * > + * Return 0 if slot can be reset, negative if a slot reset is not supported. Remind me again why we need all the "probe" stuff. That really uglifies the code and the locking. I think I worked through it once, but regrettably, I didn't add any explanation and I've forgotten. I assume all the return values are immutable, i.e., they won't change because of hotplug events on the slot or the bus, right? Why do you need probe functions for slot and bus reset, but not for function reset? > + */ > +int pci_probe_reset_slot(struct pci_slot *slot) > +{ > + return pci_slot_reset(slot, 1); > +} > +EXPORT_SYMBOL_GPL(pci_probe_reset_slot); > + > +/** > * pci_reset_slot - reset a PCI slot > * @slot: PCI slot to reset > * > @@ -3662,6 +3674,18 @@ static int pci_bus_reset(struct pci_bus *bus, int probe) > } > > /** > + * pci_probe_reset_bus - probe whether a PCI bus can be reset > + * @bus: PCI bus to probe > + * > + * Return 0 if bus can be reset, negative if a bus reset is not supported. > + */ > +int pci_probe_reset_bus(struct pci_bus *bus) > +{ > + return pci_bus_reset(bus, 1); > +} > +EXPORT_SYMBOL_GPL(pci_probe_reset_bus); > + > +/** > * pci_reset_bus - reset a PCI bus > * @bus: top level PCI bus to reset > * > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1a8fd34..daf40cd 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -924,7 +924,9 @@ int pcie_set_mps(struct pci_dev *dev, int mps); > int __pci_reset_function(struct pci_dev *dev); > int __pci_reset_function_locked(struct pci_dev *dev); > int pci_reset_function(struct pci_dev *dev); > +int pci_probe_reset_slot(struct pci_slot *slot); > int pci_reset_slot(struct pci_slot *slot); > +int pci_probe_reset_bus(struct pci_bus *bus); > int pci_reset_bus(struct pci_bus *bus); > void pci_reset_bridge_secondary_bus(struct pci_dev *dev); > void pci_update_resource(struct pci_dev *dev, int resno); > -- 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 Wed, 2013-08-14 at 14:24 -0600, Bjorn Helgaas wrote: > On Wed, Aug 14, 2013 at 2:06 PM, Alex Williamson > <alex.williamson@redhat.com> wrote: > > Users of pci_reset_bus() and pci_reset_slot() need a way to probe > > whether the bus or slot supports reset. Add trivial helper functions > > and export them as vfio-pci will make use of these. > > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > > --- > > > > This is logically patch 10/9 for v5 pci: bus and slot reset interfaces. > > If there's a need to re-spin anything I'll roll this in. > > > > drivers/pci/pci.c | 24 ++++++++++++++++++++++++ > > include/linux/pci.h | 2 ++ > > 2 files changed, 26 insertions(+) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index ba68451..888d847 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -3610,6 +3610,18 @@ static int pci_slot_reset(struct pci_slot *slot, int probe) > > } > > > > /** > > + * pci_probe_reset_slot - probe whether a PCI slot can be reset > > + * @slot: PCI slot to probe > > + * > > + * Return 0 if slot can be reset, negative if a slot reset is not supported. > > Remind me again why we need all the "probe" stuff. That really > uglifies the code and the locking. I think I worked through it once, > but regrettably, I didn't add any explanation and I've forgotten. I'm not sure what insight I can provide. We use pci_probe_reset_function() to determine whether to create a reset entry in pci-sysfs. I obviously feel like I have a legitimate need for them here to add them and make use of them in my code. > I assume all the return values are immutable, i.e., they won't change > because of hotplug events on the slot or the bus, right? The only way I can think of that these would change is if you had a hotplug driver enabling slot reset that is then unloaded or detached from the controller requiring us to switch to a bus reset. Otherwise it's topology based. Note that I don't actually expose which is used to the user, I just give them a list of groups and devices and if it changes between when they ask and when they try to do a reset, then they have to ask again. BTW, note that pci_reset_function is not immutable and we seem to deal with it pretty well. If we have a device with no function-based reset capability we allow pci_reset_function to perform a secondary bus reset. If another device was hot added to the bus, we'd no longer allow it. AFAIK, nobody has ever encountered this and cared. > Why do you need probe functions for slot and bus reset, but not for > function reset? There's already a pci_probe_reset_function() ;) Interestingly, that's the one I don't need for vfio-pci because I want to reset a device when a user opens it anyway, so I just call pci_reset_function() and check the result. With bus and slot reset there are likely more devices affected (we can use pci_reset_function() if there's only a single device on the bus). I need to be able to tell the user the set of devices affected, so I need to know whether to look at the bus or the slot in case there's a difference. Thanks, Alex > > + */ > > +int pci_probe_reset_slot(struct pci_slot *slot) > > +{ > > + return pci_slot_reset(slot, 1); > > +} > > +EXPORT_SYMBOL_GPL(pci_probe_reset_slot); > > + > > +/** > > * pci_reset_slot - reset a PCI slot > > * @slot: PCI slot to reset > > * > > @@ -3662,6 +3674,18 @@ static int pci_bus_reset(struct pci_bus *bus, int probe) > > } > > > > /** > > + * pci_probe_reset_bus - probe whether a PCI bus can be reset > > + * @bus: PCI bus to probe > > + * > > + * Return 0 if bus can be reset, negative if a bus reset is not supported. > > + */ > > +int pci_probe_reset_bus(struct pci_bus *bus) > > +{ > > + return pci_bus_reset(bus, 1); > > +} > > +EXPORT_SYMBOL_GPL(pci_probe_reset_bus); > > + > > +/** > > * pci_reset_bus - reset a PCI bus > > * @bus: top level PCI bus to reset > > * > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 1a8fd34..daf40cd 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -924,7 +924,9 @@ int pcie_set_mps(struct pci_dev *dev, int mps); > > int __pci_reset_function(struct pci_dev *dev); > > int __pci_reset_function_locked(struct pci_dev *dev); > > int pci_reset_function(struct pci_dev *dev); > > +int pci_probe_reset_slot(struct pci_slot *slot); > > int pci_reset_slot(struct pci_slot *slot); > > +int pci_probe_reset_bus(struct pci_bus *bus); > > int pci_reset_bus(struct pci_bus *bus); > > void pci_reset_bridge_secondary_bus(struct pci_dev *dev); > > void pci_update_resource(struct pci_dev *dev, int resno); > > -- 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 Wed, Aug 14, 2013 at 2:06 PM, Alex Williamson <alex.williamson@redhat.com> wrote: > Users of pci_reset_bus() and pci_reset_slot() need a way to probe > whether the bus or slot supports reset. Add trivial helper functions > and export them as vfio-pci will make use of these. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Merged to the PCI "next" branch for v3.12, thanks. > --- > > This is logically patch 10/9 for v5 pci: bus and slot reset interfaces. > If there's a need to re-spin anything I'll roll this in. > > drivers/pci/pci.c | 24 ++++++++++++++++++++++++ > include/linux/pci.h | 2 ++ > 2 files changed, 26 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index ba68451..888d847 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3610,6 +3610,18 @@ static int pci_slot_reset(struct pci_slot *slot, int probe) > } > > /** > + * pci_probe_reset_slot - probe whether a PCI slot can be reset > + * @slot: PCI slot to probe > + * > + * Return 0 if slot can be reset, negative if a slot reset is not supported. > + */ > +int pci_probe_reset_slot(struct pci_slot *slot) > +{ > + return pci_slot_reset(slot, 1); > +} > +EXPORT_SYMBOL_GPL(pci_probe_reset_slot); > + > +/** > * pci_reset_slot - reset a PCI slot > * @slot: PCI slot to reset > * > @@ -3662,6 +3674,18 @@ static int pci_bus_reset(struct pci_bus *bus, int probe) > } > > /** > + * pci_probe_reset_bus - probe whether a PCI bus can be reset > + * @bus: PCI bus to probe > + * > + * Return 0 if bus can be reset, negative if a bus reset is not supported. > + */ > +int pci_probe_reset_bus(struct pci_bus *bus) > +{ > + return pci_bus_reset(bus, 1); > +} > +EXPORT_SYMBOL_GPL(pci_probe_reset_bus); > + > +/** > * pci_reset_bus - reset a PCI bus > * @bus: top level PCI bus to reset > * > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1a8fd34..daf40cd 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -924,7 +924,9 @@ int pcie_set_mps(struct pci_dev *dev, int mps); > int __pci_reset_function(struct pci_dev *dev); > int __pci_reset_function_locked(struct pci_dev *dev); > int pci_reset_function(struct pci_dev *dev); > +int pci_probe_reset_slot(struct pci_slot *slot); > int pci_reset_slot(struct pci_slot *slot); > +int pci_probe_reset_bus(struct pci_bus *bus); > int pci_reset_bus(struct pci_bus *bus); > void pci_reset_bridge_secondary_bus(struct pci_dev *dev); > void pci_update_resource(struct pci_dev *dev, int resno); > -- 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/pci/pci.c b/drivers/pci/pci.c index ba68451..888d847 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3610,6 +3610,18 @@ static int pci_slot_reset(struct pci_slot *slot, int probe) } /** + * pci_probe_reset_slot - probe whether a PCI slot can be reset + * @slot: PCI slot to probe + * + * Return 0 if slot can be reset, negative if a slot reset is not supported. + */ +int pci_probe_reset_slot(struct pci_slot *slot) +{ + return pci_slot_reset(slot, 1); +} +EXPORT_SYMBOL_GPL(pci_probe_reset_slot); + +/** * pci_reset_slot - reset a PCI slot * @slot: PCI slot to reset * @@ -3662,6 +3674,18 @@ static int pci_bus_reset(struct pci_bus *bus, int probe) } /** + * pci_probe_reset_bus - probe whether a PCI bus can be reset + * @bus: PCI bus to probe + * + * Return 0 if bus can be reset, negative if a bus reset is not supported. + */ +int pci_probe_reset_bus(struct pci_bus *bus) +{ + return pci_bus_reset(bus, 1); +} +EXPORT_SYMBOL_GPL(pci_probe_reset_bus); + +/** * pci_reset_bus - reset a PCI bus * @bus: top level PCI bus to reset * diff --git a/include/linux/pci.h b/include/linux/pci.h index 1a8fd34..daf40cd 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -924,7 +924,9 @@ int pcie_set_mps(struct pci_dev *dev, int mps); int __pci_reset_function(struct pci_dev *dev); int __pci_reset_function_locked(struct pci_dev *dev); int pci_reset_function(struct pci_dev *dev); +int pci_probe_reset_slot(struct pci_slot *slot); int pci_reset_slot(struct pci_slot *slot); +int pci_probe_reset_bus(struct pci_bus *bus); int pci_reset_bus(struct pci_bus *bus); void pci_reset_bridge_secondary_bus(struct pci_dev *dev); void pci_update_resource(struct pci_dev *dev, int resno);
Users of pci_reset_bus() and pci_reset_slot() need a way to probe whether the bus or slot supports reset. Add trivial helper functions and export them as vfio-pci will make use of these. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- This is logically patch 10/9 for v5 pci: bus and slot reset interfaces. If there's a need to re-spin anything I'll roll this in. drivers/pci/pci.c | 24 ++++++++++++++++++++++++ include/linux/pci.h | 2 ++ 2 files changed, 26 insertions(+) -- 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