The subchannel should be left in a quiescent state unless the VFIO device FD is opened. When the FD is opened bring the chanel to active and allow the VFIO device to operate. When the device FD is closed then quiesce the channel.
To make this work the FSM needs to handle the transitions to/from open and closed so everything is sequenced. Rename state NOT_OPER to BROKEN and use it wheneven the driver has malfunctioned. STANDBY becomes CLOSED. The normal case FSM looks like: CLOSED -> IDLE -> PROCESS/PENDING* -> IDLE -> CLOSED
With a possible branch off to BROKEN from any state. Once the device is in BROKEN it cannot be recovered other than be reloading the driver.
Delete the triply redundant calls to vfio_ccw_sch_quiesce(). vfio_ccw_mdev_close_device() always leaves the subchannel quiescent. vfio_ccw_mdev_remove() cannot return until vfio_ccw_mdev_close_device() completes and vfio_ccw_sch_remove() cannot return until vfio_ccw_mdev_remove() completes. Have the FSM code take care of calling cp_free() when appropriate.
Device reset becomes a CLOSE/OPEN sequence which now properly handles the situation if the device becomes BROKEN.
Machine shutdown via vfio_ccw_sch_shutdown() now simply tries to close and leaves the device BROKEN (though arguably the bus should take care to quiet down the subchannel HW during shutdown, not the drivers)
Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- drivers/s390/cio/vfio_ccw_drv.c | 74 ++----------------- drivers/s390/cio/vfio_ccw_fsm.c | 110 +++++++++++++++++++++++++--- drivers/s390/cio/vfio_ccw_ops.c | 49 ++++--------- drivers/s390/cio/vfio_ccw_private.h | 12 +-- 4 files changed, 125 insertions(+), 120 deletions(-)
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 0407427770955d..769edbbd164313 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -36,51 +36,6 @@ debug_info_t *vfio_ccw_debug_trace_id; /* * Helpers */ -int vfio_ccw_sch_quiesce(struct subchannel *sch) -{ - struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev); - 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 { - - 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; - } - - /* - * Flush all I/O and wait for - * cancel/halt/clear completion. - */ - private->completion = &completion; - spin_unlock_irq(sch->lock); - - if (ret == -EBUSY) - 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); -out_unlock: - private->state = VFIO_CCW_STATE_NOT_OPER; - spin_unlock_irq(sch->lock); - return ret; -} - static void vfio_ccw_sch_io_todo(struct work_struct *work) { struct vfio_ccw_private *private; @@ -147,7 +102,7 @@ static struct vfio_ccw_private *vfio_ccw_alloc_private(struct subchannel *sch)
private->sch = sch; mutex_init(&private->io_mutex); - private->state = VFIO_CCW_STATE_NOT_OPER; + private->state = VFIO_CCW_STATE_CLOSED; INIT_LIST_HEAD(&private->crw); INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo); INIT_WORK(&private->crw_work, vfio_ccw_crw_todo); @@ -231,18 +186,9 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
dev_set_drvdata(&sch->dev, private);
- spin_lock_irq(sch->lock); - sch->isc = VFIO_CCW_ISC; - ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch); - spin_unlock_irq(sch->lock); - if (ret) - goto out_free; - - private->state = VFIO_CCW_STATE_STANDBY; - ret = vfio_ccw_mdev_reg(sch); if (ret) - goto out_disable; + goto out_free;
if (dev_get_uevent_suppress(&sch->dev)) { dev_set_uevent_suppress(&sch->dev, 0); @@ -254,8 +200,6 @@ static int vfio_ccw_sch_probe(struct subchannel *sch) sch->schid.sch_no); return 0;
-out_disable: - cio_disable_subchannel(sch); out_free: dev_set_drvdata(&sch->dev, NULL); vfio_ccw_free_private(private); @@ -266,7 +210,6 @@ 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_mdev_unreg(sch);
dev_set_drvdata(&sch->dev, NULL); @@ -280,7 +223,10 @@ 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); + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_BROKEN); }
/** @@ -307,16 +253,10 @@ static int vfio_ccw_sch_event(struct subchannel *sch, int process) goto out_unlock;
if (cio_update_schib(sch)) { - vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER); + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_BROKEN); rc = 0; goto out_unlock; } - - private = dev_get_drvdata(&sch->dev); - if (private->state == VFIO_CCW_STATE_NOT_OPER) { - private->state = private->mdev ? VFIO_CCW_STATE_IDLE : - VFIO_CCW_STATE_STANDBY; - } rc = 0;
out_unlock: diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c index e435a9cd92dacf..64ff1a5e3cb475 100644 --- a/drivers/s390/cio/vfio_ccw_fsm.c +++ b/drivers/s390/cio/vfio_ccw_fsm.c @@ -12,6 +12,8 @@ #include <linux/vfio.h> #include <linux/mdev.h>
+#include <asm/isc.h> + #include "ioasm.h" #include "vfio_ccw_private.h"
@@ -156,12 +158,12 @@ static int fsm_do_clear(struct vfio_ccw_private *private) return ret; }
-static void fsm_notoper(struct vfio_ccw_private *private, - enum vfio_ccw_event event) +static void fsm_broken(struct vfio_ccw_private *private, + enum vfio_ccw_event event) { struct subchannel *sch = private->sch;
- VFIO_CCW_TRACE_EVENT(2, "notoper"); + VFIO_CCW_TRACE_EVENT(2, "broken"); VFIO_CCW_TRACE_EVENT(2, dev_name(&sch->dev));
/* @@ -169,7 +171,8 @@ static void fsm_notoper(struct vfio_ccw_private *private, * Probably we should send the machine check to the guest. */ css_sched_sch_todo(sch, SCH_TODO_UNREG); - private->state = VFIO_CCW_STATE_NOT_OPER; + private->state = VFIO_CCW_STATE_BROKEN; + cp_free(&private->cp); }
/* @@ -367,38 +370,121 @@ static void fsm_irq(struct vfio_ccw_private *private, complete(private->completion); }
+static void fsm_open(struct vfio_ccw_private *private, + enum vfio_ccw_event event) +{ + struct subchannel *sch = private->sch; + int ret; + + spin_lock_irq(sch->lock); + sch->isc = VFIO_CCW_ISC; + ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch); + if (ret) + private->state = VFIO_CCW_STATE_BROKEN; + else + private->state = VFIO_CCW_STATE_IDLE; + spin_unlock_irq(sch->lock); +} + +static int flush_sch(struct vfio_ccw_private *private) +{ + struct subchannel *sch = private->sch; + DECLARE_COMPLETION_ONSTACK(completion); + int iretry, ret = 0; + + iretry = 255; + do { + 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); + return ret; + } + + /* + * Flush all I/O and wait for + * cancel/halt/clear completion. + */ + private->completion = &completion; + spin_unlock_irq(sch->lock); + + if (ret == -EBUSY) + 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); + if (ret == -EBUSY) + ret = flush_sch(private); + if (ret) + goto err_unlock; + private->state = VFIO_CCW_STATE_CLOSED; + spin_unlock_irq(sch->lock); + cp_free(&private->cp); + return; + +err_unlock: + spin_unlock_irq(sch->lock); + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_BROKEN); +} + /* * Device statemachine */ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = { - [VFIO_CCW_STATE_NOT_OPER] = { - [VFIO_CCW_EVENT_NOT_OPER] = fsm_nop, + [VFIO_CCW_STATE_BROKEN] = { + [VFIO_CCW_EVENT_BROKEN] = fsm_nop, [VFIO_CCW_EVENT_IO_REQ] = fsm_io_error, [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_error, [VFIO_CCW_EVENT_INTERRUPT] = fsm_disabled_irq, + [VFIO_CCW_EVENT_OPEN] = fsm_nop, + [VFIO_CCW_EVENT_CLOSE] = fsm_nop, }, - [VFIO_CCW_STATE_STANDBY] = { - [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, + [VFIO_CCW_STATE_CLOSED] = { + [VFIO_CCW_EVENT_BROKEN] = fsm_broken, [VFIO_CCW_EVENT_IO_REQ] = fsm_io_error, [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_error, - [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, + [VFIO_CCW_EVENT_INTERRUPT] = fsm_disabled_irq, + [VFIO_CCW_EVENT_OPEN] = fsm_open, + [VFIO_CCW_EVENT_CLOSE] = fsm_broken, }, [VFIO_CCW_STATE_IDLE] = { - [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, + [VFIO_CCW_EVENT_BROKEN] = fsm_broken, [VFIO_CCW_EVENT_IO_REQ] = fsm_io_request, [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_request, [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, + [VFIO_CCW_EVENT_OPEN] = fsm_broken, + [VFIO_CCW_EVENT_CLOSE] = fsm_close, }, [VFIO_CCW_STATE_CP_PROCESSING] = { - [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, + [VFIO_CCW_EVENT_BROKEN] = fsm_broken, [VFIO_CCW_EVENT_IO_REQ] = fsm_io_retry, [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_retry, [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, + [VFIO_CCW_EVENT_OPEN] = fsm_broken, + [VFIO_CCW_EVENT_CLOSE] = fsm_close, }, [VFIO_CCW_STATE_CP_PENDING] = { - [VFIO_CCW_EVENT_NOT_OPER] = fsm_notoper, + [VFIO_CCW_EVENT_BROKEN] = fsm_broken, [VFIO_CCW_EVENT_IO_REQ] = fsm_io_busy, [VFIO_CCW_EVENT_ASYNC_REQ] = fsm_async_request, [VFIO_CCW_EVENT_INTERRUPT] = fsm_irq, + [VFIO_CCW_EVENT_OPEN] = fsm_broken, + [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 d8589afac272f1..bd4d08afa3e4dc 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -21,10 +21,6 @@ static const struct vfio_device_ops vfio_ccw_dev_ops;
static int vfio_ccw_mdev_reset(struct vfio_ccw_private *private) { - struct subchannel *sch; - int ret; - - sch = private->sch; /* * TODO: * In the cureent stage, some things like "no I/O running" and "no @@ -33,15 +29,11 @@ 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; - - ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch); - if (!ret) - private->state = VFIO_CCW_STATE_IDLE; - - return ret; + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN); + if (private->state == VFIO_CCW_STATE_BROKEN) + return -EINVAL; + return 0; }
static int vfio_ccw_mdev_notifier(struct notifier_block *nb, @@ -118,9 +110,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev) struct vfio_ccw_private *private = dev_get_drvdata(mdev->dev.parent); int ret;
- if (private->state == VFIO_CCW_STATE_NOT_OPER) - return -ENODEV; - if (atomic_dec_if_positive(&private->avail) < 0) return -EPERM;
@@ -129,7 +118,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev) &vfio_ccw_dev_ops);
private->mdev = mdev; - private->state = VFIO_CCW_STATE_IDLE;
VFIO_CCW_MSG_EVENT(2, "mdev %pUl, sch %x.%x.%04x: create\n", mdev_uuid(mdev), private->sch->schid.cssid, @@ -146,7 +134,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev) vfio_uninit_group_dev(&private->vdev); atomic_inc(&private->avail); private->mdev = NULL; - private->state = VFIO_CCW_STATE_IDLE; return ret; }
@@ -160,16 +147,7 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev) private->sch->schid.sch_no);
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_uninit_group_dev(&private->vdev); - cp_free(&private->cp); private->mdev = NULL; atomic_inc(&private->avail); } @@ -181,6 +159,9 @@ static int vfio_ccw_mdev_open_device(struct vfio_device *vdev) unsigned long events = VFIO_IOMMU_NOTIFY_DMA_UNMAP; int ret;
+ if (private->state == VFIO_CCW_STATE_BROKEN) + return -EINVAL; + private->nb.notifier_call = vfio_ccw_mdev_notifier;
ret = vfio_register_notifier(vdev->dev, VFIO_IOMMU_NOTIFY, @@ -200,6 +181,11 @@ static int vfio_ccw_mdev_open_device(struct vfio_device *vdev) if (ret) goto out_unregister;
+ vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN); + if (private->state == VFIO_CCW_STATE_BROKEN) { + ret = -EINVAL; + goto out_unregister; + } return ret;
out_unregister: @@ -214,14 +200,7 @@ static void vfio_ccw_mdev_close_device(struct vfio_device *vdev) struct vfio_ccw_private *private = container_of(vdev, struct vfio_ccw_private, vdev);
- if ((private->state != VFIO_CCW_STATE_NOT_OPER) && - (private->state != VFIO_CCW_STATE_STANDBY)) { - if (!vfio_ccw_mdev_reset(private)) - private->state = VFIO_CCW_STATE_STANDBY; - /* The state will be NOT_OPER on error. */ - } - - cp_free(&private->cp); + vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); vfio_ccw_unregister_dev_regions(private); vfio_unregister_notifier(vdev->dev, VFIO_IOMMU_NOTIFY, &private->nb); } diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index 7272eb78861244..5e98eacdf31074 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -122,16 +122,14 @@ struct vfio_ccw_private { extern int vfio_ccw_mdev_reg(struct subchannel *sch); extern void vfio_ccw_mdev_unreg(struct subchannel *sch);
-extern int vfio_ccw_sch_quiesce(struct subchannel *sch); - extern struct mdev_driver vfio_ccw_mdev_driver;
/* * States of the device statemachine. */ enum vfio_ccw_state { - VFIO_CCW_STATE_NOT_OPER, - VFIO_CCW_STATE_STANDBY, + VFIO_CCW_STATE_BROKEN, + VFIO_CCW_STATE_CLOSED, VFIO_CCW_STATE_IDLE, VFIO_CCW_STATE_CP_PROCESSING, VFIO_CCW_STATE_CP_PENDING, @@ -143,10 +141,12 @@ enum vfio_ccw_state { * Asynchronous events of the device statemachine. */ enum vfio_ccw_event { - VFIO_CCW_EVENT_NOT_OPER, + VFIO_CCW_EVENT_BROKEN, VFIO_CCW_EVENT_IO_REQ, VFIO_CCW_EVENT_INTERRUPT, VFIO_CCW_EVENT_ASYNC_REQ, + VFIO_CCW_EVENT_OPEN, + VFIO_CCW_EVENT_CLOSE, /* last element! */ NR_VFIO_CCW_EVENTS }; @@ -158,7 +158,7 @@ typedef void (fsm_func_t)(struct vfio_ccw_private *, enum vfio_ccw_event); extern fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS];
static inline void vfio_ccw_fsm_event(struct vfio_ccw_private *private, - int event) + enum vfio_ccw_event event) { trace_vfio_ccw_fsm_event(private->sch->schid, private->state, event); vfio_ccw_jumptable[private->state][event](private, event);