| Message ID | 20220615203318.3830778-9-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/vfio-ccw rework | expand |
On 6/15/22 4:33 PM, Eric Farman wrote: > Refactor the vfio_ccw_sch_quiesce() routine to extract the bit that > disables the subchannel and affects the FSM state. Use this to form > the basis of a CLOSE event that will mirror the OPEN event, and move > the subchannel back to NOT_OPER state. Similar comments here related to previous patch. If a close event can trigger fsm_notoper then it should probably should cut a different trace entry when event == CLOSE > > A key difference with that mirroring is that while OPEN handles the > transition from NOT_OPER => STANDBY, the later probing of the mdev > handles the transition from STANDBY => IDLE. On the other hand, > the CLOSE event will move from one of the operating states {IDLE, > CP_PROCESSING, CP_PENDING} => NOT_OPER. That is, there is no stop > in a STANDBY state on the deconfigure path. > > Add a call to cp_free() in this event, such that it is captured for > the various permutations of this event. > > In the unlikely event that cio_disable_subchannel() returns -EBUSY, > the remaining logic of vfio_ccw_sch_quiesce() can still be used. > > Signed-off-by: Eric Farman <farman@linux.ibm.com> > --- > drivers/s390/cio/vfio_ccw_drv.c | 17 +++++------------ > drivers/s390/cio/vfio_ccw_fsm.c | 26 ++++++++++++++++++++++++++ > drivers/s390/cio/vfio_ccw_ops.c | 14 ++------------ > drivers/s390/cio/vfio_ccw_private.h | 1 + > 4 files changed, 34 insertions(+), 24 deletions(-) > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c > index 52249c40a565..62bd6f969b76 100644 > --- a/drivers/s390/cio/vfio_ccw_drv.c > +++ b/drivers/s390/cio/vfio_ccw_drv.c > @@ -41,13 +41,6 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch) > DECLARE_COMPLETION_ONSTACK(completion); > int iretry, ret = 0; > > - spin_lock_irq(sch->lock); > - if (!sch->schib.pmcw.ena) > - goto out_unlock; > - ret = cio_disable_subchannel(sch); > - if (ret != -EBUSY) > - goto out_unlock; > - > iretry = 255; > do { > > @@ -74,9 +67,7 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch) > spin_lock_irq(sch->lock); > ret = cio_disable_subchannel(sch); > } while (ret == -EBUSY); > -out_unlock: > - private->state = VFIO_CCW_STATE_NOT_OPER; > - spin_unlock_irq(sch->lock); > + > return ret; > } > > @@ -258,7 +249,7 @@ static void vfio_ccw_sch_remove(struct subchannel *sch) > { > struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev); > > - vfio_ccw_sch_quiesce(sch); > + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); > mdev_unregister_device(&sch->dev); > > dev_set_drvdata(&sch->dev, NULL); > @@ -272,7 +263,9 @@ static void vfio_ccw_sch_remove(struct subchannel *sch) > > static void vfio_ccw_sch_shutdown(struct subchannel *sch) > { > - vfio_ccw_sch_quiesce(sch); > + struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev); > + > + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); > } > > /** > diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c > index 7e7ed69e1461..fa546d33e595 100644 > --- a/drivers/s390/cio/vfio_ccw_fsm.c > +++ b/drivers/s390/cio/vfio_ccw_fsm.c > @@ -380,6 +380,27 @@ static void fsm_open(struct vfio_ccw_private *private, > spin_unlock_irq(sch->lock); > } > > +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 out_unlock; > + > + ret = cio_disable_subchannel(sch); > + if (ret == -EBUSY) > + vfio_ccw_sch_quiesce(sch); > + > +out_unlock: > + private->state = VFIO_CCW_STATE_NOT_OPER; > + spin_unlock_irq(sch->lock); > + cp_free(&private->cp); > +} > + > /* > * Device statemachine > */ > @@ -390,6 +411,7 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { > [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_error, > [VFIO_CCW_EVENT_INTERRUPT] = fsm_disabled_irq, > [VFIO_CCW_EVENT_OPEN] = fsm_open, > + [VFIO_CCW_EVENT_CLOSE] = fsm_nop, > }, > [VFIO_CCW_STATE_STANDBY] = { > [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, > @@ -397,6 +419,7 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { > [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_error, > [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, > [VFIO_CCW_EVENT_OPEN] = fsm_nop, > + [VFIO_CCW_EVENT_CLOSE] = fsm_notoper, But if we are in STANDBY doesn't that imply we already did the OPEN? Don't we need to close it now before going NOT_OPER?
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 52249c40a565..62bd6f969b76 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -41,13 +41,6 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch) DECLARE_COMPLETION_ONSTACK(completion); int iretry, ret = 0; - spin_lock_irq(sch->lock); - if (!sch->schib.pmcw.ena) - goto out_unlock; - ret = cio_disable_subchannel(sch); - if (ret != -EBUSY) - goto out_unlock; - iretry = 255; do { @@ -74,9 +67,7 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch) spin_lock_irq(sch->lock); ret = cio_disable_subchannel(sch); } while (ret == -EBUSY); -out_unlock: - private->state = VFIO_CCW_STATE_NOT_OPER; - spin_unlock_irq(sch->lock); + return ret; } @@ -258,7 +249,7 @@ static void vfio_ccw_sch_remove(struct subchannel *sch) { struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev); - vfio_ccw_sch_quiesce(sch); + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); mdev_unregister_device(&sch->dev); dev_set_drvdata(&sch->dev, NULL); @@ -272,7 +263,9 @@ static void vfio_ccw_sch_remove(struct subchannel *sch) static void vfio_ccw_sch_shutdown(struct subchannel *sch) { - vfio_ccw_sch_quiesce(sch); + struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev); + + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); } /** diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c index 7e7ed69e1461..fa546d33e595 100644 --- a/drivers/s390/cio/vfio_ccw_fsm.c +++ b/drivers/s390/cio/vfio_ccw_fsm.c @@ -380,6 +380,27 @@ static void fsm_open(struct vfio_ccw_private *private, spin_unlock_irq(sch->lock); } +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 out_unlock; + + ret = cio_disable_subchannel(sch); + if (ret == -EBUSY) + vfio_ccw_sch_quiesce(sch); + +out_unlock: + private->state = VFIO_CCW_STATE_NOT_OPER; + spin_unlock_irq(sch->lock); + cp_free(&private->cp); +} + /* * Device statemachine */ @@ -390,6 +411,7 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_error, [VFIO_CCW_EVENT_INTERRUPT] = fsm_disabled_irq, [VFIO_CCW_EVENT_OPEN] = fsm_open, + [VFIO_CCW_EVENT_CLOSE] = fsm_nop, }, [VFIO_CCW_STATE_STANDBY] = { [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, @@ -397,6 +419,7 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_error, [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, [VFIO_CCW_EVENT_OPEN] = fsm_nop, + [VFIO_CCW_EVENT_CLOSE] = fsm_notoper, }, [VFIO_CCW_STATE_IDLE] = { [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, @@ -404,6 +427,7 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_request, [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, [VFIO_CCW_EVENT_OPEN] = fsm_notoper, + [VFIO_CCW_EVENT_CLOSE] = fsm_close, }, [VFIO_CCW_STATE_CP_PROCESSING] = { [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, @@ -411,6 +435,7 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_retry, [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, [VFIO_CCW_EVENT_OPEN] = fsm_notoper, + [VFIO_CCW_EVENT_CLOSE] = fsm_close, }, [VFIO_CCW_STATE_CP_PENDING] = { [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, @@ -418,5 +443,6 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_request, [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, [VFIO_CCW_EVENT_OPEN] = fsm_notoper, + [VFIO_CCW_EVENT_CLOSE] = fsm_close, }, }; diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index a7ea9358e461..fc5b83187bd9 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -33,9 +33,7 @@ static int vfio_ccw_mdev_reset(struct vfio_ccw_private *private) * There are still a lot more instructions need to be handled. We * should come back here later. */ - ret = vfio_ccw_sch_quiesce(sch); - if (ret) - return ret; + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch); if (!ret) @@ -64,7 +62,6 @@ static int vfio_ccw_mdev_notifier(struct notifier_block *nb, if (vfio_ccw_mdev_reset(private)) return NOTIFY_BAD; - cp_free(&private->cp); return NOTIFY_OK; } @@ -159,15 +156,9 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev) vfio_unregister_group_dev(&private->vdev); - if ((private->state != VFIO_CCW_STATE_NOT_OPER) && - (private->state != VFIO_CCW_STATE_STANDBY)) { - if (!vfio_ccw_sch_quiesce(private->sch)) - private->state = VFIO_CCW_STATE_STANDBY; - /* The state will be NOT_OPER on error. */ - } + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); vfio_uninit_group_dev(&private->vdev); - cp_free(&private->cp); atomic_inc(&private->avail); } @@ -217,7 +208,6 @@ static void vfio_ccw_mdev_close_device(struct vfio_device *vdev) /* The state will be NOT_OPER on error. */ } - cp_free(&private->cp); vfio_ccw_unregister_dev_regions(private); vfio_unregister_notifier(vdev, VFIO_IOMMU_NOTIFY, &private->nb); } diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index 8dff1699a7d9..435d401b8fb9 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -143,6 +143,7 @@ enum vfio_ccw_event { VFIO_CCW_EVENT_INTERRUPT, VFIO_CCW_EVENT_ASYNC_REQ, VFIO_CCW_EVENT_OPEN, + VFIO_CCW_EVENT_CLOSE, /* last element! */ NR_VFIO_CCW_EVENTS };
Refactor the vfio_ccw_sch_quiesce() routine to extract the bit that disables the subchannel and affects the FSM state. Use this to form the basis of a CLOSE event that will mirror the OPEN event, and move the subchannel back to NOT_OPER state. A key difference with that mirroring is that while OPEN handles the transition from NOT_OPER => STANDBY, the later probing of the mdev handles the transition from STANDBY => IDLE. On the other hand, the CLOSE event will move from one of the operating states {IDLE, CP_PROCESSING, CP_PENDING} => NOT_OPER. That is, there is no stop in a STANDBY state on the deconfigure path. Add a call to cp_free() in this event, such that it is captured for the various permutations of this event. In the unlikely event that cio_disable_subchannel() returns -EBUSY, the remaining logic of vfio_ccw_sch_quiesce() can still be used. Signed-off-by: Eric Farman <farman@linux.ibm.com> --- drivers/s390/cio/vfio_ccw_drv.c | 17 +++++------------ drivers/s390/cio/vfio_ccw_fsm.c | 26 ++++++++++++++++++++++++++ drivers/s390/cio/vfio_ccw_ops.c | 14 ++------------ drivers/s390/cio/vfio_ccw_private.h | 1 + 4 files changed, 34 insertions(+), 24 deletions(-)