public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Farman <farman@linux.ibm.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: Jared Rossi <jrossi@linux.ibm.com>,
	Halil Pasic <pasic@linux.ibm.com>,
	linux-s390@vger.kernel.org, kvm@vger.kernel.org,
	Eric Farman <farman@linux.ibm.com>
Subject: [RFC PATCH v3 1/3] vfio-ccw: Indicate if a channel_program is started
Date: Tue, 16 Jun 2020 21:50:51 +0200	[thread overview]
Message-ID: <20200616195053.99253-2-farman@linux.ibm.com> (raw)
In-Reply-To: <20200616195053.99253-1-farman@linux.ibm.com>

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@linux.ibm.com>
---
 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


  reply	other threads:[~2020-06-16 19:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-16 19:50 [RFC PATCH v3 0/3] vfio-ccw: Fix interrupt handling for HALT/CLEAR Eric Farman
2020-06-16 19:50 ` Eric Farman [this message]
2020-06-17 23:11   ` [RFC PATCH v3 1/3] vfio-ccw: Indicate if a channel_program is started Halil Pasic
2020-06-18 11:47     ` Eric Farman
2020-06-16 19:50 ` [RFC PATCH v3 2/3] vfio-ccw: Remove the CP_PENDING FSM state Eric Farman
2020-06-16 19:50 ` [RFC PATCH v3 3/3] vfio-ccw: Check workqueue before doing START Eric Farman
2020-06-19 11:40   ` Cornelia Huck
2020-06-17 11:24 ` [RFC PATCH v3 0/3] vfio-ccw: Fix interrupt handling for HALT/CLEAR Eric Farman
2020-06-29 14:56   ` Cornelia Huck
2020-06-30 19:10     ` Eric Farman
2020-06-19 11:21 ` Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200616195053.99253-2-farman@linux.ibm.com \
    --to=farman@linux.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=jrossi@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pasic@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox