On Tue, Jan 14, 2014 at 8:45 PM, Alex Williamson
<alex.williamson@xxxxxxxxxx> wrote:
> PCI resets will attempt to take the device_lock for any device to be
> reset. This is a problem if that lock is already held, for instance
> in the device remove path. It's not sufficient to simply kill the
> user process or skip the reset if called after .remove as a race could
> result in the same deadlock. Instead, we handle all resets as "best
> effort" using the PCI "try" reset interfaces. This prevents the user
> from being able to induce a deadlock by triggering a reset.
>
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> ---
>
> v2: Update error message to make it clear that device reset failed
> and change back to pr_warn to make sure it's visible. I'll
> address whether dev_warn would be more appropriate in future
> updates.
>
> Bjorn, if this looks good and you don't mind, please take this in
> through your tree so that it syncs with the introduction of the
> pci_try_reset interfaces. Thanks!
I applied this to pci/reset (along with the try_reset implementation)
and pushed it to my "next" branch, thanks!
> drivers/vfio/pci/vfio_pci.c | 29 +++++++++--------------------
> 1 file changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 6ab71b9..2319d20 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -139,25 +139,14 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
> pci_write_config_word(pdev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);
>
> /*
> - * Careful, device_lock may already be held. This is the case if
> - * a driver unbind is blocked. Try to get the locks ourselves to
> - * prevent a deadlock.
> + * Try to reset the device. The success of this is dependent on
> + * being able to lock the device, which is not always possible.
> */
> if (vdev->reset_works) {
> - bool reset_done = false;
> -
> - if (pci_cfg_access_trylock(pdev)) {
> - if (device_trylock(&pdev->dev)) {
> - __pci_reset_function_locked(pdev);
> - reset_done = true;
> - device_unlock(&pdev->dev);
> - }
> - pci_cfg_access_unlock(pdev);
> - }
> -
> - if (!reset_done)
> - pr_warn("%s: Unable to acquire locks for reset of %s\n",
> - __func__, dev_name(&pdev->dev));
> + int ret = pci_try_reset_function(pdev);
> + if (ret)
> + pr_warn("%s: Failed to reset device %s (%d)\n",
> + __func__, dev_name(&pdev->dev), ret);
> }
>
> pci_restore_state(pdev);
> @@ -514,7 +503,7 @@ static long vfio_pci_ioctl(void *device_data,
>
> } else if (cmd == VFIO_DEVICE_RESET) {
> return vdev->reset_works ?
> - pci_reset_function(vdev->pdev) : -EINVAL;
> + pci_try_reset_function(vdev->pdev) : -EINVAL;
>
> } else if (cmd == VFIO_DEVICE_GET_PCI_HOT_RESET_INFO) {
> struct vfio_pci_hot_reset_info hdr;
> @@ -684,8 +673,8 @@ reset_info_exit:
> &info, slot);
> if (!ret)
> /* User has access, do the reset */
> - ret = slot ? pci_reset_slot(vdev->pdev->slot) :
> - pci_reset_bus(vdev->pdev->bus);
> + ret = slot ? pci_try_reset_slot(vdev->pdev->slot) :
> + pci_try_reset_bus(vdev->pdev->bus);
>
> hot_reset_release:
> for (i--; i >= 0; i--)
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
