The interrupt path checks the FSM state when processing a final interrupt (an interrupt that is neither subchannel active, nor device active), to determine whether to call cp_free() and release the associated memory. But, this does not fully close the window where a START comes in after a HALT/CLEAR. If the START runs while the CLEAR interrupt is being processed, the channel program struct will be allocated while the interrupt would be considering whether or not to free it. If the FSM state is CP_PROCESSING, then everything is fine. But if the START is able to issue its SSCH and get a cc0, then the in-flight interrupt would have been for an unrelated operation (perhaps none, if the subchannel was previously idle). The channel_program struct has an "initialized" flag that is set early in the fsm_io_request() flow, to simplify the various cp_*() accessors. Let's extend this idea to include a "started" flag that announces that the channel program has successfully been issued to hardware. With this, the interrupt path can determine whether the final interrupt should also release the cp resources instead of relying on a transient FSM state. Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx> --- drivers/s390/cio/vfio_ccw_cp.c | 2 ++ drivers/s390/cio/vfio_ccw_cp.h | 1 + drivers/s390/cio/vfio_ccw_drv.c | 2 +- drivers/s390/cio/vfio_ccw_fsm.c | 11 +++++++++++ 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index b9febc581b1f..7748eeef434e 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -657,6 +657,7 @@ int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb) if (!ret) { cp->initialized = true; + cp->started = false; /* It is safe to force: if it was not set but idals used * ccwchain_calc_length would have returned an error. @@ -685,6 +686,7 @@ void cp_free(struct channel_program *cp) return; cp->initialized = false; + cp->started = false; list_for_each_entry_safe(chain, temp, &cp->ccwchain_list, next) { for (i = 0; i < chain->ch_len; i++) { pfn_array_unpin_free(chain->ch_pa + i, cp->mdev); diff --git a/drivers/s390/cio/vfio_ccw_cp.h b/drivers/s390/cio/vfio_ccw_cp.h index ba31240ce965..7ea14910aaaa 100644 --- a/drivers/s390/cio/vfio_ccw_cp.h +++ b/drivers/s390/cio/vfio_ccw_cp.h @@ -39,6 +39,7 @@ struct channel_program { union orb orb; struct device *mdev; bool initialized; + bool started; struct ccw1 *guest_cp; }; diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 8c625b530035..7e2a790dc9a1 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -94,7 +94,7 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work) (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)); if (scsw_is_solicited(&irb->scsw)) { cp_update_scsw(&private->cp, &irb->scsw); - if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) + if (is_final && private->cp.started) cp_free(&private->cp); } mutex_lock(&private->io_mutex); diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c index 23e61aa638e4..d806f88eba72 100644 --- a/drivers/s390/cio/vfio_ccw_fsm.c +++ b/drivers/s390/cio/vfio_ccw_fsm.c @@ -50,6 +50,7 @@ static int fsm_io_helper(struct vfio_ccw_private *private) sch->schib.scsw.cmd.actl |= SCSW_ACTL_START_PEND; ret = 0; private->state = VFIO_CCW_STATE_CP_PENDING; + private->cp.started = true; break; case 1: /* Status pending */ case 2: /* Busy */ @@ -246,6 +247,16 @@ static void fsm_io_request(struct vfio_ccw_private *private, char *errstr = "request"; struct subchannel_id schid = get_schid(private); + if (private->cp.started) { + io_region->ret_code = -EBUSY; + VFIO_CCW_MSG_EVENT(2, + "%pUl (%x.%x.%04x): busy\n", + mdev_uuid(mdev), schid.cssid, + schid.ssid, schid.sch_no); + errstr = "busy"; + goto err_out; + } + private->state = VFIO_CCW_STATE_CP_PROCESSING; memcpy(scsw, io_region->scsw_area, sizeof(*scsw)); -- 2.17.1