On Fri, 2023-02-10 at 14:30 -0700, Alex Williamson wrote:
> On Fri, 10 Feb 2023 18:42:27 +0100
> Eric Farman <farman@xxxxxxxxxxxxx> wrote:
>
> > The logic in vfio_ccw_sch_shutdown() always assumed that the input
> > subchannel would point to a vfio_ccw_private struct, without
> > checking
> > that one exists. The blamed commit put in a check for this
> > scenario,
> > to prevent the possibility of a missing private.
> >
> > The trouble is that check was put alongside a WARN_ON(), presuming
> > that such a scenario would be a cause for concern. But this can be
> > triggered by binding a subchannel to vfio-ccw, and rebooting the
> > system before starting the mdev (via "mdevctl start" or similar)
> > or after stopping it. In those cases, shutdown doesn't need to
> > worry because either the private was never allocated, or it was
> > cleaned up by vfio_ccw_mdev_remove().
> >
> > Remove the WARN_ON() piece of this check, since there are plausible
> > scenarios where private would be NULL in this path.
> >
> > Fixes: 9e6f07cd1eaa ("vfio/ccw: create a parent struct")
> > Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> > ---
> > drivers/s390/cio/vfio_ccw_drv.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> > b/drivers/s390/cio/vfio_ccw_drv.c
> > index 54aba7cceb33..ff538a086fc7 100644
> > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > @@ -225,7 +225,7 @@ static void vfio_ccw_sch_shutdown(struct
> > subchannel *sch)
> > struct vfio_ccw_parent *parent = dev_get_drvdata(&sch-
> > >dev);
> > struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> > >dev);
> >
> > - if (WARN_ON(!private))
> > + if (!private)
> > return;
> >
> > vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
>
> I see I'm on the To: line here, is this intended to go through the
> vfio
> tree rather than s390?
Either way. I put you as "to" as the blamed commit went via vfio, but I
could ask Heiko or Vasili to take it if that's easier.
Eric
