Commit a882c16a authored by Jason Gunthorpe's avatar Jason Gunthorpe Committed by Alex Williamson
Browse files

vfio/pci: Change vfio_pci_try_bus_reset() to use the dev_set 



vfio_pci_try_bus_reset() is triggering a reset of the entire_dev set if
any device within it has accumulated a needs_reset. This reset can only be
done once all of the drivers operating the PCI devices to be reset are in
a known safe state.

Make this clearer by directly operating on the dev_set instead of the
vfio_pci_device. Rename the function to vfio_pci_dev_set_try_reset().

Use the device list inside the dev_set to check that all drivers are in a
safe state instead of working backwards from the pci_device.

The dev_set->lock directly prevents devices from joining/leaving the set,
or changing their state, which further implies the pci_device cannot
change drivers or that the vfio_device be freed, eliminating the need for
get/put's.

If a pci_device to be reset is not in the dev_set then the reset cannot be
used as we can't know what the state of that driver is. Directly measure
this by checking that every pci_device is in the dev_set - which
effectively proves that VFIO drivers are attached to everything.

Remove the odd interaction around vfio_pci_set_power_state() - have the
only caller avoid its redundant vfio_pci_set_power_state() instead of
avoiding it inside vfio_pci_dev_set_try_reset().

This restructuring corrects a call to pci_dev_driver() without holding the
device_lock() and removes a hard wiring to &vfio_pci_driver.

Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarCornelia Huck <cohuck@redhat.com>
Link: https://lore.kernel.org/r/9-v4-9ea22c5e6afb+1adf-vfio_reflck_jgg@nvidia.com


Signed-off-by: default avatarAlex Williamson <alex.williamson@redhat.com>
parent 2cd8b14a

drivers/vfio/pci/vfio_pci.c

+82 −92
Original line number Diff line number Diff line
@@ -223,7 +223,7 @@ static void vfio_pci_probe_mmaps(struct vfio_pci_device *vdev) 
	}

}



static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev);

static bool vfio_pci_dev_set_try_reset(struct vfio_device_set *dev_set);

static void vfio_pci_disable(struct vfio_pci_device *vdev);

static int vfio_pci_try_zap_and_vma_lock_cb(struct pci_dev *pdev, void *data);
@@ -404,6 +404,9 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev) 
	struct vfio_pci_ioeventfd *ioeventfd, *ioeventfd_tmp;

	int i, bar;



	/* For needs_reset */

	lockdep_assert_held(&vdev->vdev.dev_set->lock);



	/* Stop the device from further DMA */

	pci_clear_master(pdev);
@@ -487,9 +490,7 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev) 
out:

	pci_disable_device(pdev);



	vfio_pci_try_bus_reset(vdev);



	if (!disable_idle_d3)

	if (!vfio_pci_dev_set_try_reset(vdev->vdev.dev_set) && !disable_idle_d3)

		vfio_pci_set_power_state(vdev, PCI_D3hot);

}
@@ -2145,36 +2146,6 @@ static struct pci_driver vfio_pci_driver = { 
	.err_handler		= &vfio_err_handlers,

};



static int vfio_pci_get_unused_devs(struct pci_dev *pdev, void *data)

{

	struct vfio_devices *devs = data;

	struct vfio_device *device;

	struct vfio_pci_device *vdev;



	if (devs->cur_index == devs->max_index)

		return -ENOSPC;



	device = vfio_device_get_from_dev(&pdev->dev);

	if (!device)

		return -EINVAL;



	if (pci_dev_driver(pdev) != &vfio_pci_driver) {

		vfio_device_put(device);

		return -EBUSY;

	}



	vdev = container_of(device, struct vfio_pci_device, vdev);



	/* Fault if the device is not unused */

	if (device->open_count) {

		vfio_device_put(device);

		return -EBUSY;

	}



	devs->devices[devs->cur_index++] = vdev;

	return 0;

}



static int vfio_pci_try_zap_and_vma_lock_cb(struct pci_dev *pdev, void *data)

{

	struct vfio_devices *devs = data;
@@ -2208,79 +2179,98 @@ static int vfio_pci_try_zap_and_vma_lock_cb(struct pci_dev *pdev, void *data) 
	return 0;

}



static int vfio_pci_is_device_in_set(struct pci_dev *pdev, void *data)

{

	struct vfio_device_set *dev_set = data;

	struct vfio_device *cur;



	list_for_each_entry(cur, &dev_set->device_list, dev_set_list)

		if (cur->dev == &pdev->dev)

			return 0;

	return -EBUSY;

}



/*

 * If a bus or slot reset is available for the provided device and:

 *  - All of the devices affected by that bus or slot reset are unused

 *    (!refcnt)

 *  - At least one of the affected devices is marked dirty via

 *    needs_reset (such as by lack of FLR support)

 * Then attempt to perform that bus or slot reset.  Callers are required

 * to hold vdev->dev_set->lock, protecting the bus/slot reset group from

 * concurrent opens.  A vfio_device reference is acquired for each device

 * to prevent unbinds during the reset operation.

 * vfio-core considers a group to be viable and will create a vfio_device even

 * if some devices are bound to drivers like pci-stub or pcieport. Here we

 * require all PCI devices to be inside our dev_set since that ensures they stay

 * put and that every driver controlling the device can co-ordinate with the

 * device reset.

 *

 * NB: vfio-core considers a group to be viable even if some devices are

 * bound to drivers like pci-stub or pcieport.  Here we require all devices

 * to be bound to vfio_pci since that's the only way we can be sure they

 * stay put.

 * Returns the pci_dev to pass to pci_reset_bus() if every PCI device to be

 * reset is inside the dev_set, and pci_reset_bus() can succeed. NULL otherwise.

 */

static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev)

static struct pci_dev *

vfio_pci_dev_set_resettable(struct vfio_device_set *dev_set)

{

	struct vfio_devices devs = { .cur_index = 0 };

	int i = 0, ret = -EINVAL;

	bool slot = false;

	struct vfio_pci_device *tmp;

	struct pci_dev *pdev;



	if (!pci_probe_reset_slot(vdev->pdev->slot))

		slot = true;

	else if (pci_probe_reset_bus(vdev->pdev->bus))

		return;

	lockdep_assert_held(&dev_set->lock);



	if (vfio_pci_for_each_slot_or_bus(vdev->pdev, vfio_pci_count_devs,

					  &i, slot) || !i)

		return;

	/*

	 * By definition all PCI devices in the dev_set share the same PCI

	 * reset, so any pci_dev will have the same outcomes for

	 * pci_probe_reset_*() and pci_reset_bus().

	 */

	pdev = list_first_entry(&dev_set->device_list, struct vfio_pci_device,

				vdev.dev_set_list)->pdev;



	devs.max_index = i;

	devs.devices = kcalloc(i, sizeof(struct vfio_device *), GFP_KERNEL);

	if (!devs.devices)

		return;

	/* pci_reset_bus() is supported */

	if (pci_probe_reset_slot(pdev->slot) && pci_probe_reset_bus(pdev->bus))

		return NULL;



	if (vfio_pci_for_each_slot_or_bus(vdev->pdev,

					  vfio_pci_get_unused_devs,

					  &devs, slot))

		goto put_devs;

	if (vfio_pci_for_each_slot_or_bus(pdev, vfio_pci_is_device_in_set,

					  dev_set,

					  !pci_probe_reset_slot(pdev->slot)))

		return NULL;

	return pdev;

}



	/* Does at least one need a reset? */

	for (i = 0; i < devs.cur_index; i++) {

		tmp = devs.devices[i];

		if (tmp->needs_reset) {

			ret = pci_reset_bus(vdev->pdev);

			break;

static bool vfio_pci_dev_set_needs_reset(struct vfio_device_set *dev_set)

{

	struct vfio_pci_device *cur;

	bool needs_reset = false;



	list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list) {

		/* No VFIO device in the set can have an open device FD */

		if (cur->vdev.open_count)

			return false;

		needs_reset |= cur->needs_reset;

	}

	return needs_reset;

}



put_devs:

	for (i = 0; i < devs.cur_index; i++) {

		tmp = devs.devices[i];



/*

		 * If reset was successful, affected devices no longer need

		 * a reset and we should return all the collateral devices

		 * to low power.  If not successful, we either didn't reset

		 * the bus or timed out waiting for it, so let's not touch

		 * the power state.

 * If a bus or slot reset is available for the provided dev_set and:

 *  - All of the devices affected by that bus or slot reset are unused

 *  - At least one of the affected devices is marked dirty via

 *    needs_reset (such as by lack of FLR support)

 * Then attempt to perform that bus or slot reset.

 * Returns true if the dev_set was reset.

 */

		if (!ret) {

			tmp->needs_reset = false;

static bool vfio_pci_dev_set_try_reset(struct vfio_device_set *dev_set)

{

	struct vfio_pci_device *cur;

	struct pci_dev *pdev;

	int ret;



			if (tmp != vdev && !disable_idle_d3)

				vfio_pci_set_power_state(tmp, PCI_D3hot);

		}

	if (!vfio_pci_dev_set_needs_reset(dev_set))

		return false;



		vfio_device_put(&tmp->vdev);

	}

	pdev = vfio_pci_dev_set_resettable(dev_set);

	if (!pdev)

		return false;



	kfree(devs.devices);

	ret = pci_reset_bus(pdev);

	if (ret)

		return false;



	list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list) {

		cur->needs_reset = false;

		if (!disable_idle_d3)

			vfio_pci_set_power_state(cur, PCI_D3hot);

	}

	return true;

}



static void __exit vfio_pci_cleanup(void)