Re: [PATCH 3/3] vfio-ccw: add handling for asnyc channel instructions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22/11/2018 17:54, Cornelia Huck wrote:
Add a region to the vfio-ccw device that can be used to submit
asynchronous I/O instructions. ssch continues to be handled by the
existing I/O region; the new region handles hsch and csch.

Interrupt status continues to be reported through the same channels
as for ssch.

Signed-off-by: Cornelia Huck <cohuck@xxxxxxxxxx>
---
  drivers/s390/cio/Makefile           |   3 +-
  drivers/s390/cio/vfio_ccw_async.c   |  88 ++++++++++++++++
  drivers/s390/cio/vfio_ccw_drv.c     |  48 ++++++---
  drivers/s390/cio/vfio_ccw_fsm.c     | 158 +++++++++++++++++++++++++++-
  drivers/s390/cio/vfio_ccw_ops.c     |  13 ++-
  drivers/s390/cio/vfio_ccw_private.h |   6 ++
  include/uapi/linux/vfio.h           |   4 +
  include/uapi/linux/vfio_ccw.h       |  12 +++
  8 files changed, 313 insertions(+), 19 deletions(-)
  create mode 100644 drivers/s390/cio/vfio_ccw_async.c

diff --git a/drivers/s390/cio/Makefile b/drivers/s390/cio/Makefile
index f230516abb96..f6a8db04177c 100644
--- a/drivers/s390/cio/Makefile
+++ b/drivers/s390/cio/Makefile
@@ -20,5 +20,6 @@ obj-$(CONFIG_CCWGROUP) += ccwgroup.o
  qdio-objs := qdio_main.o qdio_thinint.o qdio_debug.o qdio_setup.o
  obj-$(CONFIG_QDIO) += qdio.o
-vfio_ccw-objs += vfio_ccw_drv.o vfio_ccw_cp.o vfio_ccw_ops.o vfio_ccw_fsm.o
+vfio_ccw-objs += vfio_ccw_drv.o vfio_ccw_cp.o vfio_ccw_ops.o vfio_ccw_fsm.o \
+	vfio_ccw_async.o
  obj-$(CONFIG_VFIO_CCW) += vfio_ccw.o
diff --git a/drivers/s390/cio/vfio_ccw_async.c b/drivers/s390/cio/vfio_ccw_async.c
new file mode 100644
index 000000000000..8c7f51d17d70
--- /dev/null
+++ b/drivers/s390/cio/vfio_ccw_async.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0

...snip...


  static void __exit vfio_ccw_sch_exit(void)
diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index f94aa01f9c36..0caf77e8f377 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -3,8 +3,10 @@
   * Finite state machine for vfio-ccw device handling
   *
   * Copyright IBM Corp. 2017
+ * Copyright Red Hat, Inc. 2018
   *
   * Author(s): Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx>
+ *            Cornelia Huck <cohuck@xxxxxxxxxx>
   */
#include <linux/vfio.h>
@@ -68,6 +70,81 @@ static int fsm_io_helper(struct vfio_ccw_private *private)
  	return ret;
  }
+static int fsm_do_halt(struct vfio_ccw_private *private)
+{
+	struct subchannel *sch;
+	unsigned long flags;
+	int ccode;
+	int ret;
+
+	sch = private->sch;
+
+	spin_lock_irqsave(sch->lock, flags);
+	private->state = VFIO_CCW_STATE_BUSY;
+
+	/* Issue "Halt Subchannel" */
+	ccode = hsch(sch->schid);
+
+	switch (ccode) {
+	case 0:
+		/*
+		 * Initialize device status information
+		 */
+		sch->schib.scsw.cmd.actl |= SCSW_ACTL_HALT_PEND;
+		ret = 0;
+		break;
+	case 1:		/* Status pending */
+	case 2:		/* Busy */
+		ret = -EBUSY;
+		break;
+	case 3:		/* Device not operational */
+	{
+		ret = -ENODEV;
+		break;
+	}
+	default:
+		ret = ccode;
+	}

Shouldn't you set the state back here?

+	spin_unlock_irqrestore(sch->lock, flags);
+	return ret;
+}
+
+static int fsm_do_clear(struct vfio_ccw_private *private)
+{
+	struct subchannel *sch;
+	unsigned long flags;
+	int ccode;
+	int ret;
+
+	sch = private->sch;
+
+	spin_lock_irqsave(sch->lock, flags);
+	private->state = VFIO_CCW_STATE_BUSY;
+
+	/* Issue "Clear Subchannel" */
+	ccode = csch(sch->schid);
+
+	switch (ccode) {
+	case 0:
+		/*
+		 * Initialize device status information
+		 */
+		sch->schib.scsw.cmd.actl = SCSW_ACTL_CLEAR_PEND;
+		/* TODO: check what else we might need to clear */
+		ret = 0;
+		break;
+	case 3:		/* Device not operational */
+	{
+		ret = -ENODEV;
+		break;
+	}
+	default:
+		ret = ccode;
+	}
+	spin_unlock_irqrestore(sch->lock, flags);
+	return ret;
+}
+
  static void fsm_notoper(struct vfio_ccw_private *private,
  			enum vfio_ccw_event event)
  {
@@ -102,6 +179,20 @@ static void fsm_io_busy(struct vfio_ccw_private *private,
  	private->io_region->ret_code = -EBUSY;
  }
+static void fsm_async_error(struct vfio_ccw_private *private,
+			    enum vfio_ccw_event event)
+{
+	pr_err("vfio-ccw: FSM: halt/clear request from state:%d\n",
+	       private->state);
+	private->cmd_region->ret_code = -EIO;
+}
+
+static void fsm_async_busy(struct vfio_ccw_private *private,
+			   enum vfio_ccw_event event)
+{
+	private->cmd_region->ret_code = -EBUSY;
+}
+
  static void fsm_disabled_irq(struct vfio_ccw_private *private,
  			     enum vfio_ccw_event event)
  {
@@ -166,11 +257,11 @@ static void fsm_io_request(struct vfio_ccw_private *private,
  		}
  		return;
  	} else if (scsw->cmd.fctl & SCSW_FCTL_HALT_FUNC) {
-		/* XXX: Handle halt. */
+		/* halt is handled via the async cmd region */
  		io_region->ret_code = -EOPNOTSUPP;
  		goto err_out;
  	} else if (scsw->cmd.fctl & SCSW_FCTL_CLEAR_FUNC) {
-		/* XXX: Handle clear. */
+		/* clear is handled via the async cmd region */
  		io_region->ret_code = -EOPNOTSUPP;
  		goto err_out;

What about filtering inside the vfio_ccw_mdev_write_io_region() before the call to the FSM?


  	}
@@ -181,6 +272,59 @@ static void fsm_io_request(struct vfio_ccw_private *private,
  			       io_region->ret_code, errstr);
  }
+/*
+ * Deal with a halt request from userspace.
+ */
+static void fsm_halt_request(struct vfio_ccw_private *private,
+			     enum vfio_ccw_event event)
+{
+	struct ccw_cmd_region *cmd_region = private->cmd_region;
+	int state = private->state;
+
+	private->state = VFIO_CCW_STATE_BOXED;
+
+	if (cmd_region->command != VFIO_CCW_ASYNC_CMD_HSCH) {
+		/* should not happen? */

I think we should make sure it does not happen before we get here.
Like serializing HALT and CLEAR before the FSM.

+		cmd_region->ret_code = -EINVAL;
+		goto err_out;
+	}
+
+	cmd_region->ret_code = fsm_do_halt(private);

fsm_do_halt() set the state to BUSY.
Do we need a state change here and in fsm_do_halt ?

Why not only the BUSY state?

+	if (cmd_region->ret_code)
+		goto err_out;
+
+	return;
+
+err_out:
+	private->state = state;
+}
+

...snip...

Regards,
Pierre

--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux