| Message ID | 20180919163037.13497-7-okaya@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Add reset type parameter to PCI reset functions | expand |
On Wed, 19 Sep 2018 16:30:37 +0000 Sinan Kaya <okaya@kernel.org> wrote: > Looking to have more control between the users of the API vs. what the API > can do internally. The new reset_type tells the PCI core about the bounds > of the request. > > Signed-off-by: Sinan Kaya <okaya@kernel.org> > --- > drivers/pci/pci.c | 18 +++++++++++++++--- > drivers/vfio/pci/vfio_pci.c | 6 ++++-- > include/linux/pci.h | 2 +- > 3 files changed, 20 insertions(+), 6 deletions(-) Acked-by: Alex Williamson <alex.williamson@redhat.com> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index b4f14419bd51..f11d29f32119 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5232,13 +5232,25 @@ static int __pci_reset_bus(struct pci_bus *bus) > /** > * pci_reset_bus - Try to reset a PCI bus > * @pdev: top level PCI device to reset via slot/bus > + * @reset_type: resets to try > * > * Same as above except return -EAGAIN if the bus cannot be locked > */ > -int pci_reset_bus(struct pci_dev *pdev) > +int pci_reset_bus(struct pci_dev *pdev, u32 reset_type) > { > - return (!pci_probe_reset_slot(pdev->slot)) ? > - __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus); > + if (reset_type & PCI_RESET_LINK) { > + return (!pci_probe_reset_slot(pdev->slot)) ? > + __pci_reset_slot(pdev->slot) : > + __pci_reset_bus(pdev->bus); > + } > + > + if (reset_type & PCI_RESET_BUS) > + return __pci_reset_bus(pdev->bus); > + > + if (reset_type & PCI_RESET_SLOT) > + return __pci_reset_slot(pdev->slot); > + > + return -EINVAL; > } > EXPORT_SYMBOL_GPL(pci_reset_bus); > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > index fe7ada997c51..0e80c72b1eaa 100644 > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -1015,7 +1015,8 @@ static long vfio_pci_ioctl(void *device_data, > &info, slot); > if (!ret) > /* User has access, do the reset */ > - ret = pci_reset_bus(vdev->pdev); > + ret = pci_reset_bus(vdev->pdev, > + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); > > hot_reset_release: > for (i--; i >= 0; i--) > @@ -1390,7 +1391,8 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) > } > > if (needs_reset) > - ret = pci_reset_bus(vdev->pdev); > + ret = pci_reset_bus(vdev->pdev, > + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); > > put_devs: > for (i = 0; i < devs.cur_index; i++) { > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 4fdddcb85066..85f48e156753 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1128,7 +1128,7 @@ int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type); > int pci_try_reset_function(struct pci_dev *dev, u32 reset_type); > int pci_probe_reset_slot(struct pci_slot *slot); > int pci_probe_reset_bus(struct pci_bus *bus); > -int pci_reset_bus(struct pci_dev *dev); > +int pci_reset_bus(struct pci_dev *dev, u32 reset_type); > void pci_reset_secondary_bus(struct pci_dev *dev); > void pcibios_reset_secondary_bus(struct pci_dev *dev); > void pci_update_resource(struct pci_dev *dev, int resno);
On Wed, Sep 19, 2018 at 04:30:37PM +0000, Sinan Kaya wrote: > Looking to have more control between the users of the API vs. what the API > can do internally. The new reset_type tells the PCI core about the bounds > of the request. > > Signed-off-by: Sinan Kaya <okaya@kernel.org> > --- > drivers/pci/pci.c | 18 +++++++++++++++--- > drivers/vfio/pci/vfio_pci.c | 6 ++++-- > include/linux/pci.h | 2 +- > 3 files changed, 20 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index b4f14419bd51..f11d29f32119 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5232,13 +5232,25 @@ static int __pci_reset_bus(struct pci_bus *bus) > /** > * pci_reset_bus - Try to reset a PCI bus > * @pdev: top level PCI device to reset via slot/bus > + * @reset_type: resets to try > * > * Same as above except return -EAGAIN if the bus cannot be locked > */ > -int pci_reset_bus(struct pci_dev *pdev) > +int pci_reset_bus(struct pci_dev *pdev, u32 reset_type) > { > - return (!pci_probe_reset_slot(pdev->slot)) ? > - __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus); > + if (reset_type & PCI_RESET_LINK) { > + return (!pci_probe_reset_slot(pdev->slot)) ? > + __pci_reset_slot(pdev->slot) : > + __pci_reset_bus(pdev->bus); > + } > + > + if (reset_type & PCI_RESET_BUS) > + return __pci_reset_bus(pdev->bus); > + > + if (reset_type & PCI_RESET_SLOT) > + return __pci_reset_slot(pdev->slot); I don't understand this code. We have #define PCI_RESET_LINK (PCI_RESET_SLOT | PCI_RESET_BUS) so if either PCI_RESET_BUS or PCI_RESET_SLOT is set, we should take the first "if" above, which always returns, and the last two should never be reached. What am I missing? > + > + return -EINVAL; > } > EXPORT_SYMBOL_GPL(pci_reset_bus); > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > index fe7ada997c51..0e80c72b1eaa 100644 > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -1015,7 +1015,8 @@ static long vfio_pci_ioctl(void *device_data, > &info, slot); > if (!ret) > /* User has access, do the reset */ > - ret = pci_reset_bus(vdev->pdev); > + ret = pci_reset_bus(vdev->pdev, > + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); This is the only place in the whole series where a caller uses something other than PCI_RESET_ANY. Inquiring minds want to know why. I'm sure it's the right thing to do, it's just that we'll need to know how to choose in future cases. > > hot_reset_release: > for (i--; i >= 0; i--) > @@ -1390,7 +1391,8 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) > } > > if (needs_reset) > - ret = pci_reset_bus(vdev->pdev); > + ret = pci_reset_bus(vdev->pdev, > + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); > > put_devs: > for (i = 0; i < devs.cur_index; i++) { > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 4fdddcb85066..85f48e156753 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1128,7 +1128,7 @@ int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type); > int pci_try_reset_function(struct pci_dev *dev, u32 reset_type); > int pci_probe_reset_slot(struct pci_slot *slot); > int pci_probe_reset_bus(struct pci_bus *bus); > -int pci_reset_bus(struct pci_dev *dev); > +int pci_reset_bus(struct pci_dev *dev, u32 reset_type); > void pci_reset_secondary_bus(struct pci_dev *dev); > void pcibios_reset_secondary_bus(struct pci_dev *dev); > void pci_update_resource(struct pci_dev *dev, int resno); > -- > 2.18.0 >
On 9/25/2018 4:54 PM, Bjorn Helgaas wrote: > On Wed, Sep 19, 2018 at 04:30:37PM +0000, Sinan Kaya wrote: >> Looking to have more control between the users of the API vs. what the API >> can do internally. The new reset_type tells the PCI core about the bounds >> of the request. >> >> Signed-off-by: Sinan Kaya <okaya@kernel.org> >> --- >> drivers/pci/pci.c | 18 +++++++++++++++--- >> drivers/vfio/pci/vfio_pci.c | 6 ++++-- >> include/linux/pci.h | 2 +- >> 3 files changed, 20 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index b4f14419bd51..f11d29f32119 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -5232,13 +5232,25 @@ static int __pci_reset_bus(struct pci_bus *bus) >> /** >> * pci_reset_bus - Try to reset a PCI bus >> * @pdev: top level PCI device to reset via slot/bus >> + * @reset_type: resets to try >> * >> * Same as above except return -EAGAIN if the bus cannot be locked >> */ >> -int pci_reset_bus(struct pci_dev *pdev) >> +int pci_reset_bus(struct pci_dev *pdev, u32 reset_type) >> { >> - return (!pci_probe_reset_slot(pdev->slot)) ? >> - __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus); >> + if (reset_type & PCI_RESET_LINK) { >> + return (!pci_probe_reset_slot(pdev->slot)) ? >> + __pci_reset_slot(pdev->slot) : >> + __pci_reset_bus(pdev->bus); >> + } >> + >> + if (reset_type & PCI_RESET_BUS) >> + return __pci_reset_bus(pdev->bus); >> + >> + if (reset_type & PCI_RESET_SLOT) >> + return __pci_reset_slot(pdev->slot); > > I don't understand this code. We have > > #define PCI_RESET_LINK (PCI_RESET_SLOT | PCI_RESET_BUS) > > so if either PCI_RESET_BUS or PCI_RESET_SLOT is set, we should take > the first "if" above, which always returns, and the last two should > never be reached. What am I missing? Yup, this is broken. I need to follow up with another patchset. I was trying to cover the case where user said, "I need a link reset. I don't care whether it is a slot reset or bus reset as long as we achieve a reset." That's when PCI_RESET_LINK is used. We expect most drivers to PCI_RESET_LINK if they are interested in a link reset and doesn't need to know about the hotplug capability of a particular slot. > >> + >> + return -EINVAL; >> } >> EXPORT_SYMBOL_GPL(pci_reset_bus); >> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c >> index fe7ada997c51..0e80c72b1eaa 100644 >> --- a/drivers/vfio/pci/vfio_pci.c >> +++ b/drivers/vfio/pci/vfio_pci.c >> @@ -1015,7 +1015,8 @@ static long vfio_pci_ioctl(void *device_data, >> &info, slot); >> if (!ret) >> /* User has access, do the reset */ >> - ret = pci_reset_bus(vdev->pdev); >> + ret = pci_reset_bus(vdev->pdev, >> + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); > > This is the only place in the whole series where a caller uses > something other than PCI_RESET_ANY. Inquiring minds want to know why. > I'm sure it's the right thing to do, it's just that we'll need to know > how to choose in future cases. Use PCI_RESET_ANY for the existing behavior where you want one type of reset and don't care if it is PM/function/slot/bus/specific. Use PCI_RESET_LINK when you need link to retrain. Use PCI_RESET_SLOT when you know that this system is hotplug capable by calling probe functions. Use PCI_RESET_BUS when you know that this system is not hotplug capable and this endpoint will never be used in such a system. I can capture this into the commit message. > >> >> hot_reset_release: >> for (i--; i >= 0; i--) >> @@ -1390,7 +1391,8 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) >> } >> >> if (needs_reset) >> - ret = pci_reset_bus(vdev->pdev); >> + ret = pci_reset_bus(vdev->pdev, >> + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); >> >> put_devs: >> for (i = 0; i < devs.cur_index; i++) { >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index 4fdddcb85066..85f48e156753 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1128,7 +1128,7 @@ int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type); >> int pci_try_reset_function(struct pci_dev *dev, u32 reset_type); >> int pci_probe_reset_slot(struct pci_slot *slot); >> int pci_probe_reset_bus(struct pci_bus *bus); >> -int pci_reset_bus(struct pci_dev *dev); >> +int pci_reset_bus(struct pci_dev *dev, u32 reset_type); >> void pci_reset_secondary_bus(struct pci_dev *dev); >> void pcibios_reset_secondary_bus(struct pci_dev *dev); >> void pci_update_resource(struct pci_dev *dev, int resno); >> -- >> 2.18.0 >> >
On Tue, Sep 25, 2018 at 05:15:52PM -0400, Sinan Kaya wrote: > On 9/25/2018 4:54 PM, Bjorn Helgaas wrote: > > On Wed, Sep 19, 2018 at 04:30:37PM +0000, Sinan Kaya wrote: > > > Looking to have more control between the users of the API vs. what the API > > > can do internally. The new reset_type tells the PCI core about the bounds > > > of the request. > > > > > > Signed-off-by: Sinan Kaya <okaya@kernel.org> > > > --- > > > drivers/pci/pci.c | 18 +++++++++++++++--- > > > drivers/vfio/pci/vfio_pci.c | 6 ++++-- > > > include/linux/pci.h | 2 +- > > > 3 files changed, 20 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > index b4f14419bd51..f11d29f32119 100644 > > > --- a/drivers/pci/pci.c > > > +++ b/drivers/pci/pci.c > > > @@ -5232,13 +5232,25 @@ static int __pci_reset_bus(struct pci_bus *bus) > > > /** > > > * pci_reset_bus - Try to reset a PCI bus > > > * @pdev: top level PCI device to reset via slot/bus > > > + * @reset_type: resets to try > > > * > > > * Same as above except return -EAGAIN if the bus cannot be locked > > > */ > > > -int pci_reset_bus(struct pci_dev *pdev) > > > +int pci_reset_bus(struct pci_dev *pdev, u32 reset_type) > > > { > > > - return (!pci_probe_reset_slot(pdev->slot)) ? > > > - __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus); > > > + if (reset_type & PCI_RESET_LINK) { > > > + return (!pci_probe_reset_slot(pdev->slot)) ? > > > + __pci_reset_slot(pdev->slot) : > > > + __pci_reset_bus(pdev->bus); > > > + } > > > + > > > + if (reset_type & PCI_RESET_BUS) > > > + return __pci_reset_bus(pdev->bus); > > > + > > > + if (reset_type & PCI_RESET_SLOT) > > > + return __pci_reset_slot(pdev->slot); > > > > I don't understand this code. We have > > > > #define PCI_RESET_LINK (PCI_RESET_SLOT | PCI_RESET_BUS) > > > > so if either PCI_RESET_BUS or PCI_RESET_SLOT is set, we should take > > the first "if" above, which always returns, and the last two should > > never be reached. What am I missing? > > Yup, this is broken. I need to follow up with another patchset. > > I was trying to cover the case where user said, > > "I need a link reset. I don't care whether it is a slot reset or bus reset > as long as we achieve a reset." > > That's when PCI_RESET_LINK is used. We expect most drivers to PCI_RESET_LINK > if they are interested in a link reset and doesn't need to know about the > hotplug capability of a particular slot. > > > > > > + > > > + return -EINVAL; > > > } > > > EXPORT_SYMBOL_GPL(pci_reset_bus); > > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > > > index fe7ada997c51..0e80c72b1eaa 100644 > > > --- a/drivers/vfio/pci/vfio_pci.c > > > +++ b/drivers/vfio/pci/vfio_pci.c > > > @@ -1015,7 +1015,8 @@ static long vfio_pci_ioctl(void *device_data, > > > &info, slot); > > > if (!ret) > > > /* User has access, do the reset */ > > > - ret = pci_reset_bus(vdev->pdev); > > > + ret = pci_reset_bus(vdev->pdev, > > > + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); > > > > This is the only place in the whole series where a caller uses > > something other than PCI_RESET_ANY. Inquiring minds want to know why. > > I'm sure it's the right thing to do, it's just that we'll need to know > > how to choose in future cases. > > Use PCI_RESET_ANY for the existing behavior where you want one type of > reset and don't care if it is PM/function/slot/bus/specific. > > Use PCI_RESET_LINK when you need link to retrain. There are no callers using this. Is this intended specifically for the case of "the hardware wasn't smart enough to train at the correct speed, so we fiddled things and want to retrain"? Maybe it doesn't need to be exposed in include/linux/pci.h and could be defined internally in drivers/pci/pci.c if it's needed there? > Use PCI_RESET_SLOT when you know that this system is hotplug capable > by calling probe functions. I raise my eyebrows at this because (a) a driver generally can't tell whether hotplug is supported and (b) even if the driver does know, what is the benefit to the driver to specify this? What probe functions does this refer to? If I'm a driver writer, I really can't tell what I'm supposed to do with this guidance. If I'm supposed to call a probe function, what is it, when should I call it, and what should I do with the result? Am I supposed to know whether hotplug is supported? Why would I care? I'm sure you have good answers for all these questions; I just don't know what they are :) > Use PCI_RESET_BUS when you know that this system is not hotplug capable > and this endpoint will never be used in such a system. How can a driver know this? And what's the benefit of being specific? > I can capture this into the commit message. I think it needs to be more accessible, e.g., comments near the constants and/or the function declaration. We shouldn't expect users of the interface to dig through the changelogs for it. > > > hot_reset_release: > > > for (i--; i >= 0; i--) > > > @@ -1390,7 +1391,8 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) > > > } > > > if (needs_reset) > > > - ret = pci_reset_bus(vdev->pdev); > > > + ret = pci_reset_bus(vdev->pdev, > > > + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); > > > put_devs: > > > for (i = 0; i < devs.cur_index; i++) { > > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > > index 4fdddcb85066..85f48e156753 100644 > > > --- a/include/linux/pci.h > > > +++ b/include/linux/pci.h > > > @@ -1128,7 +1128,7 @@ int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type); > > > int pci_try_reset_function(struct pci_dev *dev, u32 reset_type); > > > int pci_probe_reset_slot(struct pci_slot *slot); > > > int pci_probe_reset_bus(struct pci_bus *bus); > > > -int pci_reset_bus(struct pci_dev *dev); > > > +int pci_reset_bus(struct pci_dev *dev, u32 reset_type); > > > void pci_reset_secondary_bus(struct pci_dev *dev); > > > void pcibios_reset_secondary_bus(struct pci_dev *dev); > > > void pci_update_resource(struct pci_dev *dev, int resno); > > > -- > > > 2.18.0 > > > > > >
On 9/25/2018 5:56 PM, Bjorn Helgaas wrote: >> Use PCI_RESET_LINK when you need link to retrain. > There are no callers using this. Is this intended specifically for > the case of "the hardware wasn't smart enough to train at the correct > speed, so we fiddled things and want to retrain"? > Yup, I'll go ahead and change the hfi1 driver to use PCI_RESET_LINK as originally planned so that there is an actual user. These constants are intended to be used by the user of pci_reset APIs. I think it makes sense to keep them there as well. I see your point that PCI_RESET_LINK was not used. We should also make the hfi1 follow the rules. > Maybe it doesn't need to be exposed in include/linux/pci.h and could > be defined internally in drivers/pci/pci.c if it's needed there? > see above. >> Use PCI_RESET_SLOT when you know that this system is hotplug capable >> by calling probe functions. > I raise my eyebrows at this because (a) a driver generally can't tell > whether hotplug is supported and (b) even if the driver does know, > what is the benefit to the driver to specify this? What probe > functions does this refer to? If I'm a driver writer, I really can't > tell what I'm supposed to do with this guidance. If I'm supposed to > call a probe function, what is it, when should I call it, and what > should I do with the result? Am I supposed to know whether hotplug is > supported? Why would I care? That was the whole reason why I hid the secondary bus reset API from the user and folded into pci_reset_bus() so that PCI core can deal with hotplug internally and can go to hotplug reset or bus reset internally. User just calls pci_reset_bus(). I fully agree with your assessment but there are also exceptions like VFIO where the code finds out which particular devices are part of a slot hierarchy and for each one it checks that VFIO ownership has been claimed. (Alex, please correct me if I'm not getting this right. I just read 200 lines of code in VFIO) /* Can we do a slot or bus reset or neither? */ if (!pci_probe_reset_slot(vdev->pdev->slot)) slot = true; else if (pci_probe_reset_bus(vdev->pdev->bus)) return -ENODEV; ret = vfio_pci_for_each_slot_or_bus(vdev->pdev, vfio_pci_fill_devs, &fill, slot); > > I'm sure you have good answers for all these questions; I just don't > know what they are:) > >> Use PCI_RESET_BUS when you know that this system is not hotplug capable >> and this endpoint will never be used in such a system. > How can a driver know this? And what's the benefit of being specific? There is really no benefit but there are drivers making assumptions about the type of platform they want to run like supporting single segment only etc. PCI_RESET_BUS gives you direct access to secondary bus reset. If you are calling this directly, you are responsible for saving and restoring state like the hfi1 driver and need to ensure that this card will never work on a hotplug capable system. If not, you are ...ed. That's why, I'm suggesting that most users will use PCI_RESET_LINK so that code works on all platforms. > >> I can capture this into the commit message. > I think it needs to be more accessible, e.g., comments near the constants > and/or the function declaration. We shouldn't expect users of the > interface to dig through the changelogs for it. > I can work on this once we agree if this is the way to go. I was responding to Alex's request to have a contact between the PCI core and the drivers because there are some driver that are more intelligent about the PCI tree than a simple endpoint driver.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b4f14419bd51..f11d29f32119 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5232,13 +5232,25 @@ static int __pci_reset_bus(struct pci_bus *bus) /** * pci_reset_bus - Try to reset a PCI bus * @pdev: top level PCI device to reset via slot/bus + * @reset_type: resets to try * * Same as above except return -EAGAIN if the bus cannot be locked */ -int pci_reset_bus(struct pci_dev *pdev) +int pci_reset_bus(struct pci_dev *pdev, u32 reset_type) { - return (!pci_probe_reset_slot(pdev->slot)) ? - __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus); + if (reset_type & PCI_RESET_LINK) { + return (!pci_probe_reset_slot(pdev->slot)) ? + __pci_reset_slot(pdev->slot) : + __pci_reset_bus(pdev->bus); + } + + if (reset_type & PCI_RESET_BUS) + return __pci_reset_bus(pdev->bus); + + if (reset_type & PCI_RESET_SLOT) + return __pci_reset_slot(pdev->slot); + + return -EINVAL; } EXPORT_SYMBOL_GPL(pci_reset_bus); diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index fe7ada997c51..0e80c72b1eaa 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -1015,7 +1015,8 @@ static long vfio_pci_ioctl(void *device_data, &info, slot); if (!ret) /* User has access, do the reset */ - ret = pci_reset_bus(vdev->pdev); + ret = pci_reset_bus(vdev->pdev, + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); hot_reset_release: for (i--; i >= 0; i--) @@ -1390,7 +1391,8 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) } if (needs_reset) - ret = pci_reset_bus(vdev->pdev); + ret = pci_reset_bus(vdev->pdev, + slot ? PCI_RESET_SLOT : PCI_RESET_BUS); put_devs: for (i = 0; i < devs.cur_index; i++) { diff --git a/include/linux/pci.h b/include/linux/pci.h index 4fdddcb85066..85f48e156753 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1128,7 +1128,7 @@ int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type); int pci_try_reset_function(struct pci_dev *dev, u32 reset_type); int pci_probe_reset_slot(struct pci_slot *slot); int pci_probe_reset_bus(struct pci_bus *bus); -int pci_reset_bus(struct pci_dev *dev); +int pci_reset_bus(struct pci_dev *dev, u32 reset_type); void pci_reset_secondary_bus(struct pci_dev *dev); void pcibios_reset_secondary_bus(struct pci_dev *dev); void pci_update_resource(struct pci_dev *dev, int resno);
Looking to have more control between the users of the API vs. what the API can do internally. The new reset_type tells the PCI core about the bounds of the request. Signed-off-by: Sinan Kaya <okaya@kernel.org> --- drivers/pci/pci.c | 18 +++++++++++++++--- drivers/vfio/pci/vfio_pci.c | 6 ++++-- include/linux/pci.h | 2 +- 3 files changed, 20 insertions(+), 6 deletions(-)