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

Jason Gunthorpe
2021-07-23T12:59:22Z

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