Re: [PATCH v2 05/14] vfio/fsl-mc: Re-order vfio_fsl_mc_probe()

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Mar 17, 2021 at 06:36:09PM +0200, Diana Craciun OSS wrote:
> Hi,
> 
> Thanks for finding this!
> 
> I tested the series and currently the binding to vfio fails. The reason is
> that it is assumed that the objects scan is done after vfio_add_group_dev.
> But at this point the vdev structure is completly initialized.
>
> I'll add some more context.
> 
> There are two types of FSL MC devices:
> - a DPRC device
> - regular devices
> 
> A DPRC is some kind of container of the other devices. The DPRC VFIO device
> is scanning for all the existing devices in the container and triggers the
> probe function for those devices.

Oh. It ends up recursively calling probe() under the same stack frame?
I don't feel good about that

> However, there are some pieces of code
> that needs to be protected by a lock, lock that is created by
> vfio_fsl_mc_reflck_attach() function. This function is searching for the
> DPRC vdev (having the physical device) in the vfio group, so the "parent"
> device should have been added in the group before the child devices are
> probed.

Yes, I understood this part, but I didn't think it could be invoked
recursively from vfio_fsl_mc_init_device() :(

> I did some changes on top of these series and this is how they look like. I
> hope that I do not do something that violates the way the VFIO is designed.

Well, it is "ok" in that this is only about the reflck so it doesn't
appear to break the core's assumptions, but I don't like it at all.

I also have a later patch that revises the reflck search I now see I
will have to throw out.

I think it would be better to find the reflck entirely internally to
the driver than involving both the vfio and driver core in the
search. I will try to write that later

For now, this solution seems OK, I will fold it in, thanks

Jason