On Fri, Jul 23, 2021 at 02:29:03PM +0200, Christoph Hellwig wrote:

On Fri, Jul 23, 2021 at 09:22:27AM -0300, Jason Gunthorpe wrote:

...

...

But do we even need the else part? Assingning &mc_dev->dev is equivalent to the default per-device set anyway, isn't it?

Not quite, the default is this:

    if (!device->dev_set)
            vfio_assign_device_set(device, device);

Where 'device' is the vfio_device itself, the above is connecting to the struct fsl_mc_device.

Isn't there a 1:1 relation?

Yes, but one is a struct device * and the other is a struct vfio_device *. They don't have the same pointer value.

The above default code has the goal of creating a singleton dev_set because no other driver can reasonably obtain the 'struct vfio_device *'.

FSL is using a 'struct device *' as the key and the interesting case is when two drivers are loaded such that: mc_dev->dev.parent == &mc_dev->dev

Then they will have the same dev_set and the locking works out. The vfio_device * can never be == &mc_dev->dev because it is a different allocation.

Jason