Re: [PATCH v2 07/13] vfio/ccw: Convert to use vfio_register_group_dev()

Cornelia Huck
2021-04-28T17:10:03Z

On Mon, 26 Apr 2021 17:00:09 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> This is more complicated because vfio_ccw is sharing the vfio_device
> between both the mdev_device and its vfio_device and the css_driver.
>
> The mdev is a singleton, and the reason for this sharing appears to be to
> allow the extra css_driver function callbacks to be delivered to the
> vfio_device.
>
> This keeps things as they were, with the css_driver allocating the
> singleton, not the mdev_driver, this is pretty confusing. I'm also
> uncertain how the lifetime model for the mdev works in the css_driver
> callbacks.
>
> At this point embed the vfio_device in the vfio_ccw_private and
> instantiate it as a vfio_device when the mdev probes. The drvdata of both
> the css_device and the mdev_device point at the private, and container_of
> is used to get it back from the vfio_device.

I've been staring at this for some time, and I'm not sure whether this
is a good approach.

We allow at most one mdev per subchannel (slicing it up does not make
sense), so we can be sure that there's a 1:1 relationship between mdev
and parent device, and we can track it via a single pointer.

The vfio_ccw_private driver data is allocated during probe (same as for
other css_drivers.) Embedding a vfio_device here means that we have a
structure tied into it that is operating with different lifetime rules.

What about creating a second structure instead that can embed the
vfio_device, is allocated during mdev probing, and is linked up with
the vfio_ccw_private structure? That would follow the pattern of other
drivers more closely.

>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/s390/cio/vfio_ccw_drv.c | 21 +++--
> drivers/s390/cio/vfio_ccw_ops.c | 135 +++++++++++++++-------------
> drivers/s390/cio/vfio_ccw_private.h | 5 ++
> 3 files changed, 94 insertions(+), 67 deletions(-)