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

[Jason Gunthorpe]

On Fri, Jul 23, 2021 at 09:47:49AM +0200, Christoph Hellwig wrote:
> > @@ -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