On Fri, Sep 17, 2021 at 01:59:16PM +0200, Cornelia Huck wrote:
> > ret = cio_cancel_halt_clear(sch, &iretry);
> > -
> > if (ret == -EIO) {
> > pr_err("vfio_ccw: could not quiesce subchannel 0.%x.%04x!\n",
> > sch->schid.ssid, sch->schid.sch_no);
> > - break;
> > + return ret;
>
> Looking at this, I wonder why we had special-cased -EIO -- for -ENODEV
> we should be done as well, as then the device is dead and we do not need
> to disable it.
cio_cancel_halt_clear() should probably succeed in that case.
> > @@ -413,13 +403,28 @@ static void fsm_close(struct vfio_ccw_private *private,
> > spin_unlock_irq(sch->lock);
> >
> > if (ret == -EBUSY)
> > - wait_for_completion_timeout(&completion, 3*HZ);
> > + wait_for_completion_timeout(&completion, 3 * HZ);
> >
> > private->completion = NULL;
> > flush_workqueue(vfio_ccw_work_q);
> > spin_lock_irq(sch->lock);
> > ret = cio_disable_subchannel(sch);
> > } while (ret == -EBUSY);
> > + return ret;
> > +}
> > +
> > +static void fsm_close(struct vfio_ccw_private *private,
> > + enum vfio_ccw_event event)
> > +{
> > + struct subchannel *sch = private->sch;
> > + int ret;
> > +
> > + spin_lock_irq(sch->lock);
> > + if (!sch->schib.pmcw.ena)
> > + goto err_unlock;
> > + ret = cio_disable_subchannel(sch);
>
> cio_disable_subchannel() should be happy to disable an already disabled
> subchannel, so I guess we can just walk through this and end up in
> CLOSED state... unless entering with !ena actually indicates that we
> messed up somewhere else in the state machine. I still need to find time
> to read the patches.
I don't know, I looked at that ena stuff for a bit and couldn't guess
what it is trying to do.
Arguably the channel should not be ripped away from vfio while the FSM
is in the open states, so I'm not sure what a lot of this is for.
Jason
