Re: [PATCH v3 09/11] vfio/ccw: Create a CLOSE FSM event

Matthew Rosato
2022-07-05T19:29:29Z

On 6/30/22 4:36 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.
>
> 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>

Reviewed-by: Matthew Rosato <mjrosato@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 7d9189640da3..f98c9915e73d 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;
> }
>
> @@ -256,7 +247,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);
> @@ -270,7 +261,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 2811b2040490..89eb3feffa41 100644
> --- a/drivers/s390/cio/vfio_ccw_fsm.c
> +++ b/drivers/s390/cio/vfio_ccw_fsm.c
> @@ -384,6 +384,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
> */
> @@ -394,6 +415,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,
> @@ -401,6 +423,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_notoper,
> + [VFIO_CCW_EVENT_CLOSE] = fsm_close,
> },
> [VFIO_CCW_STATE_IDLE] = {
> [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper,
> @@ -408,6 +431,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,
> @@ -415,6 +439,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,
> @@ -422,5 +447,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
> };