Re: [PATCH 09/12] PCI: Add a PCI_ID_F_VFIO_DRIVER_OVERRIDE flag to struct pci_device_id

On Thu, Aug 12, 2021 at 10:27:28AM -0300, Jason Gunthorpe wrote:

On Wed, Aug 11, 2021 at 02:07:37PM -0500, Bjorn Helgaas wrote:

quoted

On Thu, Aug 05, 2021 at 09:23:57PM -0300, Jason Gunthorpe wrote:

quoted

Do the other bus types have a flag analogous to
PCI_ID_F_VFIO_DRIVER_OVERRIDE?  If we're doing something similar to
other bus types, it'd be nice if the approach were similar.

They could, this series doesn't attempt it. I expect the approach to
be similar as driver_override was copied from PCI to other
busses. When this is completed I hope to take a look at it.

I think this would make more sense as two patches:

  - Add a "PCI_ID_DRIVER_OVERRIDE" flag.  This is not VFIO-specific,
    since nothing in PCI depends on the VFIO-ness of drivers that use
    the flag.  The only point here is that driver id_table entries
    with this flag only match when driver_override matches the driver.

  - Update file2alias.c to export the flags and the "vfio_pci:" alias.
    This seems to be the only place where VFIO comes into play, and
    putting it in a separate patch will make it much smaller and it
    will be clear how it could be extended for other buses.

quoted

I assume somewhere in here you need to unbind mlx5_core before binding
mlx5_vfio_pci?

Er, yes, I skipped some steps here where unbind/bind has to be done
 

quoted

quoted

   6) cat the matched module name to driver_override:
    echo mlx5_vfio_pci > /sys/bus/pci/devices/0000:01:00.0/driver_override

Don't you need something here to trigger the driver attach, i.e.,
should step 5 and step 6 be swapped?  What if the driver is already
loaded? 

The full sequence is more like:

     echo mlx5_vfio_pci > /sys/bus/pci/devices/0000:01:00.0/driver_override
     echo 0000:01:00.0 > /sys/bus/pci/devices/0000:01:00.0/driver/unbind
     echo 0000:01:00.0 > /sys/bus/pci/drivers_probe

Thanks a lot for this!  I didn't know about drivers_probe (see
drivers_probe_store()), and it doesn't seem to be documented anywhere
except sysfs-bus-usb, where it's only incidental to USB.

Bjorn