Re: [PATCH v2 08/14] vfio/pci: Move to the device set infrastructure

On Fri, Jul 23, 2021 at 09:47:49AM +0200, Christoph Hellwig wrote:

quoted

@@ -2020,12 +2004,17 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	INIT_LIST_HEAD(&vdev->vma_list);
 	init_rwsem(&vdev->memory_lock);
 
-	ret = vfio_pci_reflck_attach(vdev);
+	if (pci_is_root_bus(pdev->bus))
+		ret = vfio_assign_device_set(&vdev->vdev, vdev);
+	else if (!pci_probe_reset_slot(pdev->slot))
+		ret = vfio_assign_device_set(&vdev->vdev, pdev->slot);
+	else
+		ret = vfio_assign_device_set(&vdev->vdev, pdev->bus);

Maybe invert this and add a comment:

	if (pci_is_root_bus(pdev->bus))
		ret = vfio_assign_device_set(&vdev->vdev, vdev);
	/*
	 * If there is no slot reset support for this device, the whole
	 * bus needs to be grouped together to support bus-wide resets.
	 */

I like the comment

	else if (pci_probe_reset_slot(pdev->slot) < 0)
		ret = vfio_assign_device_set(&vdev->vdev, pdev->bus);
	else
		ret = vfio_assign_device_set(&vdev->vdev, pdev->slot);

The consensus idiom here is the !:

drivers/pci/pci.c:      return (!pci_probe_reset_slot(pdev->slot)) ?
drivers/vfio/pci/vfio_pci.c:            if (!pci_probe_reset_slot(vdev->pdev->slot))
drivers/vfio/pci/vfio_pci.c:            if (!pci_probe_reset_slot(vdev->pdev->slot))
drivers/vfio/pci/vfio_pci.c:    bool slot = !pci_probe_reset_slot(vdev->pdev->slot);
drivers/vfio/pci/vfio_pci.c:    if (!pci_probe_reset_slot(vdev->pdev->slot))

It reads quite poorly either way, IMHO, I'd rather switch it to a
readable bool, but not for this series

Thanks,
Jason