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:

quoted

quoted

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