On Mon, 26 Apr 2021 17:00:09 -0300 Jason Gunthorpe <jgg@xxxxxxxxxx> 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@xxxxxxxxxx> > --- > 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(-)
