[PATCH v3 00/11] s390/vfio-ccw rework

[PATCH v3 00/11] s390/vfio-ccw rework

[Eric Farman]

Here's an updated pass through the first chunk of vfio-ccw rework.

As with v2, this is all internal to vfio-ccw, with the exception of
the removal of mdev_uuid from include/linux/mdev.h in patch 1.

There is one conflict with the vfio-next branch [2], on patch 6.

The remainder of the work that Jason Gunthorpe originally started [1]
in this space remains for a future day.

v2-v3:
 - [KW, MR, JG] Added r-b's (Thank You!)
 - Patch 7 (new):
   - [MR] Add better debug to fsm_notoper
 - Patch 8:
   - [MR] Call fsm_notoper for the OPEN event from STANDBY state
   - [EF] Drop FSM state=STANDBY in vfio_ccw_sch_probe()
     (it is handled in FSM, and was dropped by patch 10)
 - Patch 9:
   - [MR] Call fsm_close for the CLOSE event from STANDBY state
v2: https://lore.kernel.org/r/20220615203318.3830778-1-farman@linux.ibm.com/
v1: https://lore.kernel.org/r/20220602171948.2790690-1-farman@linux.ibm.com/

Footnotes:
[1] https://lore.kernel.org/r/0-v3-57c1502c62fd+2190-ccw_mdev_jgg@nvidia.com/

Cc: Kirti Wankhede <kwankhede@nvidia.com>

Eric Farman (10):
  vfio/ccw: Fix FSM state if mdev probe fails
  vfio/ccw: Do not change FSM state in subchannel event
  vfio/ccw: Remove private->mdev
  vfio/ccw: Pass enum to FSM event jumptable
  vfio/ccw: Flatten MDEV device (un)register
  vfio/ccw: Update trace data for not operational event
  vfio/ccw: Create an OPEN FSM Event
  vfio/ccw: Create a CLOSE FSM event
  vfio/ccw: Refactor vfio_ccw_mdev_reset
  vfio/ccw: Move FSM open/close to MDEV open/close

Michael Kawano (1):
  vfio/ccw: Remove UUID from s390 debug log

 drivers/s390/cio/vfio_ccw_async.c   |  1 -
 drivers/s390/cio/vfio_ccw_drv.c     | 59 +++++-------------
 drivers/s390/cio/vfio_ccw_fsm.c     | 97 ++++++++++++++++++++++++-----
 drivers/s390/cio/vfio_ccw_ops.c     | 77 +++++++----------------
 drivers/s390/cio/vfio_ccw_private.h |  9 +--
 include/linux/mdev.h                |  5 --
 6 files changed, 123 insertions(+), 125 deletions(-)

-- 
2.32.0

[PATCH v3 01/11] vfio/ccw: Remove UUID from s390 debug log

[Eric Farman]

From: Michael Kawano <mkawano@linux.ibm.com>

As vfio-ccw devices are created/destroyed, the uuid of the associated
mdevs that are recorded in $S390DBF/vfio_ccw_msg/sprintf get lost.
This is because a pointer to the UUID is stored instead of the UUID
itself, and that memory may have been repurposed if/when the logs are
examined. The result is usually garbage UUID data in the logs, though
there is an outside chance of an oops happening here.

Simply remove the UUID from the traces, as the subchannel number will
provide useful configuration information for problem determination,
and is stored directly into the log instead of a pointer.

As we were the only consumer of mdev_uuid(), remove that too.

Cc: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Michael Kawano <mkawano@linux.ibm.com>
Fixes: 60e05d1cf0875 ("vfio-ccw: add some logging")
Fixes: b7701dfbf9832 ("vfio-ccw: Register a chp_event callback for vfio-ccw")
[farman: reworded commit message, added Fixes: tags]
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
---
 drivers/s390/cio/vfio_ccw_drv.c |  5 ++---
 drivers/s390/cio/vfio_ccw_fsm.c | 26 ++++++++++++--------------
 drivers/s390/cio/vfio_ccw_ops.c |  8 ++++----
 include/linux/mdev.h            |  5 -----
 4 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index ee182cfb467d..35055eb94115 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -14,7 +14,6 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/slab.h>
-#include <linux/uuid.h>
 #include <linux/mdev.h>
 
 #include <asm/isc.h>
@@ -358,8 +357,8 @@ static int vfio_ccw_chp_event(struct subchannel *sch,
 		return 0;
 
 	trace_vfio_ccw_chp_event(private->sch->schid, mask, event);
-	VFIO_CCW_MSG_EVENT(2, "%pUl (%x.%x.%04x): mask=0x%x event=%d\n",
-			   mdev_uuid(private->mdev), sch->schid.cssid,
+	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: mask=0x%x event=%d\n",
+			   sch->schid.cssid,
 			   sch->schid.ssid, sch->schid.sch_no,
 			   mask, event);
 
diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index 8483a266051c..bbcc5b486749 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -10,7 +10,6 @@
  */
 
 #include <linux/vfio.h>
-#include <linux/mdev.h>
 
 #include "ioasm.h"
 #include "vfio_ccw_private.h"
@@ -242,7 +241,6 @@ static void fsm_io_request(struct vfio_ccw_private *private,
 	union orb *orb;
 	union scsw *scsw = &private->scsw;
 	struct ccw_io_region *io_region = private->io_region;
-	struct mdev_device *mdev = private->mdev;
 	char *errstr = "request";
 	struct subchannel_id schid = get_schid(private);
 
@@ -256,8 +254,8 @@ static void fsm_io_request(struct vfio_ccw_private *private,
 		if (orb->tm.b) {
 			io_region->ret_code = -EOPNOTSUPP;
 			VFIO_CCW_MSG_EVENT(2,
-					   "%pUl (%x.%x.%04x): transport mode\n",
-					   mdev_uuid(mdev), schid.cssid,
+					   "sch %x.%x.%04x: transport mode\n",
+					   schid.cssid,
 					   schid.ssid, schid.sch_no);
 			errstr = "transport mode";
 			goto err_out;
@@ -265,8 +263,8 @@ static void fsm_io_request(struct vfio_ccw_private *private,
 		io_region->ret_code = cp_init(&private->cp, orb);
 		if (io_region->ret_code) {
 			VFIO_CCW_MSG_EVENT(2,
-					   "%pUl (%x.%x.%04x): cp_init=%d\n",
-					   mdev_uuid(mdev), schid.cssid,
+					   "sch %x.%x.%04x: cp_init=%d\n",
+					   schid.cssid,
 					   schid.ssid, schid.sch_no,
 					   io_region->ret_code);
 			errstr = "cp init";
@@ -276,8 +274,8 @@ static void fsm_io_request(struct vfio_ccw_private *private,
 		io_region->ret_code = cp_prefetch(&private->cp);
 		if (io_region->ret_code) {
 			VFIO_CCW_MSG_EVENT(2,
-					   "%pUl (%x.%x.%04x): cp_prefetch=%d\n",
-					   mdev_uuid(mdev), schid.cssid,
+					   "sch %x.%x.%04x: cp_prefetch=%d\n",
+					   schid.cssid,
 					   schid.ssid, schid.sch_no,
 					   io_region->ret_code);
 			errstr = "cp prefetch";
@@ -289,8 +287,8 @@ static void fsm_io_request(struct vfio_ccw_private *private,
 		io_region->ret_code = fsm_io_helper(private);
 		if (io_region->ret_code) {
 			VFIO_CCW_MSG_EVENT(2,
-					   "%pUl (%x.%x.%04x): fsm_io_helper=%d\n",
-					   mdev_uuid(mdev), schid.cssid,
+					   "sch %x.%x.%04x: fsm_io_helper=%d\n",
+					   schid.cssid,
 					   schid.ssid, schid.sch_no,
 					   io_region->ret_code);
 			errstr = "cp fsm_io_helper";
@@ -300,16 +298,16 @@ static void fsm_io_request(struct vfio_ccw_private *private,
 		return;
 	} else if (scsw->cmd.fctl & SCSW_FCTL_HALT_FUNC) {
 		VFIO_CCW_MSG_EVENT(2,
-				   "%pUl (%x.%x.%04x): halt on io_region\n",
-				   mdev_uuid(mdev), schid.cssid,
+				   "sch %x.%x.%04x: halt on io_region\n",
+				   schid.cssid,
 				   schid.ssid, schid.sch_no);
 		/* 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) {
 		VFIO_CCW_MSG_EVENT(2,
-				   "%pUl (%x.%x.%04x): clear on io_region\n",
-				   mdev_uuid(mdev), schid.cssid,
+				   "sch %x.%x.%04x: clear on io_region\n",
+				   schid.cssid,
 				   schid.ssid, schid.sch_no);
 		/* clear is handled via the async cmd region */
 		io_region->ret_code = -EOPNOTSUPP;
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index b49e2e9db2dc..0e05bff78b8e 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -131,8 +131,8 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
 	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,
+	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: create\n",
+			   private->sch->schid.cssid,
 			   private->sch->schid.ssid,
 			   private->sch->schid.sch_no);
 
@@ -154,8 +154,8 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
 {
 	struct vfio_ccw_private *private = dev_get_drvdata(mdev->dev.parent);
 
-	VFIO_CCW_MSG_EVENT(2, "mdev %pUl, sch %x.%x.%04x: remove\n",
-			   mdev_uuid(mdev), private->sch->schid.cssid,
+	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: remove\n",
+			   private->sch->schid.cssid,
 			   private->sch->schid.ssid,
 			   private->sch->schid.sch_no);
 
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index bb539794f54a..47ad3b104d9e 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -65,11 +65,6 @@ struct mdev_driver {
 	struct device_driver driver;
 };
 
-static inline const guid_t *mdev_uuid(struct mdev_device *mdev)
-{
-	return &mdev->uuid;
-}
-
 extern struct bus_type mdev_bus_type;
 
 int mdev_register_device(struct device *dev, struct mdev_driver *mdev_driver);
-- 
2.32.0

[PATCH v3 02/11] vfio/ccw: Fix FSM state if mdev probe fails

[Eric Farman]

The FSM is in STANDBY state when arriving in vfio_ccw_mdev_probe(),
and this routine converts it to IDLE as part of its processing.
The error exit sets it to IDLE (again) but clears the private->mdev
pointer.

The FSM should of course be managing the state itself, but the
correct thing for vfio_ccw_mdev_probe() to do would be to put
the state back the way it found it.

The corresponding check of private->mdev in vfio_ccw_sch_io_todo()
can be removed, since the distinction is unnecessary at this point.

Fixes: 3bf1311f351ef ("vfio/ccw: Convert to use vfio_register_emulated_iommu_dev()")
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_drv.c | 5 +++--
 drivers/s390/cio/vfio_ccw_ops.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index 35055eb94115..179eb614fa5b 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -106,9 +106,10 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work)
 	/*
 	 * Reset to IDLE only if processing of a channel program
 	 * has finished. Do not overwrite a possible processing
-	 * state if the final interrupt was for HSCH or CSCH.
+	 * state if the interrupt was unsolicited, or if the final
+	 * interrupt was for HSCH or CSCH.
 	 */
-	if (private->mdev && cp_is_finished)
+	if (cp_is_finished)
 		private->state = VFIO_CCW_STATE_IDLE;
 
 	if (private->io_trigger)
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 0e05bff78b8e..9a05dadcbb75 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -146,7 +146,7 @@ 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;
+	private->state = VFIO_CCW_STATE_STANDBY;
 	return ret;
 }
 
-- 
2.32.0

[PATCH v3 03/11] vfio/ccw: Do not change FSM state in subchannel event

[Eric Farman]

The routine vfio_ccw_sch_event() is tasked with handling subchannel events,
specifically machine checks, on behalf of vfio-ccw. It correctly calls
cio_update_schib(), and if that fails (meaning the subchannel is gone)
it makes an FSM event call to mark the subchannel Not Operational.

If that worked, however, then it decides that if the FSM state was already
Not Operational (implying the subchannel just came back), then it should
simply change the FSM to partially- or fully-open.

Remove this trickery, since a subchannel returning will require more
probing than simply "oh all is well again" to ensure it works correctly.

Fixes: bbe37e4cb8970 ("vfio: ccw: introduce a finite state machine")
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_drv.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index 179eb614fa5b..279ad2161f17 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -301,19 +301,11 @@ static int vfio_ccw_sch_event(struct subchannel *sch, int process)
 	if (work_pending(&sch->todo_work))
 		goto out_unlock;
 
-	if (cio_update_schib(sch)) {
-		vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
-		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;
 
+	if (cio_update_schib(sch))
+		vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
+
 out_unlock:
 	spin_unlock_irqrestore(sch->lock, flags);
 
-- 
2.32.0

[PATCH v3 04/11] vfio/ccw: Remove private->mdev

[Eric Farman]

There are no remaining users of private->mdev. Remove it.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/s390/cio/vfio_ccw_async.c   | 1 -
 drivers/s390/cio/vfio_ccw_ops.c     | 3 ---
 drivers/s390/cio/vfio_ccw_private.h | 2 --
 3 files changed, 6 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_async.c b/drivers/s390/cio/vfio_ccw_async.c
index 7a838e3d7c0f..420d89ba7f83 100644
--- a/drivers/s390/cio/vfio_ccw_async.c
+++ b/drivers/s390/cio/vfio_ccw_async.c
@@ -8,7 +8,6 @@
  */
 
 #include <linux/vfio.h>
-#include <linux/mdev.h>
 
 #include "vfio_ccw_private.h"
 
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 9a05dadcbb75..81377270d4a7 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -128,7 +128,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
 	vfio_init_group_dev(&private->vdev, &mdev->dev,
 			    &vfio_ccw_dev_ops);
 
-	private->mdev = mdev;
 	private->state = VFIO_CCW_STATE_IDLE;
 
 	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: create\n",
@@ -145,7 +144,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
 err_atomic:
 	vfio_uninit_group_dev(&private->vdev);
 	atomic_inc(&private->avail);
-	private->mdev = NULL;
 	private->state = VFIO_CCW_STATE_STANDBY;
 	return ret;
 }
@@ -170,7 +168,6 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
 
 	vfio_uninit_group_dev(&private->vdev);
 	cp_free(&private->cp);
-	private->mdev = NULL;
 	atomic_inc(&private->avail);
 }
 
diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index 7272eb788612..12b5537d478f 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -73,7 +73,6 @@ struct vfio_ccw_crw {
  * @state: internal state of the device
  * @completion: synchronization helper of the I/O completion
  * @avail: available for creating a mediated device
- * @mdev: pointer to the mediated device
  * @nb: notifier for vfio events
  * @io_region: MMIO region to input/output I/O arguments/results
  * @io_mutex: protect against concurrent update of I/O regions
@@ -97,7 +96,6 @@ struct vfio_ccw_private {
 	int			state;
 	struct completion	*completion;
 	atomic_t		avail;
-	struct mdev_device	*mdev;
 	struct notifier_block	nb;
 	struct ccw_io_region	*io_region;
 	struct mutex		io_mutex;
-- 
2.32.0

[PATCH v3 05/11] vfio/ccw: Pass enum to FSM event jumptable

[Eric Farman]

The FSM has an enumerated list of events defined.
Use that as the argument passed to the jump table,
instead of a regular int.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index 12b5537d478f..5c128eec596b 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -156,7 +156,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);
-- 
2.32.0

[PATCH v3 06/11] vfio/ccw: Flatten MDEV device (un)register

[Eric Farman]

The vfio_ccw_mdev_(un)reg routines are merely vfio-ccw routines that
pass control to mdev_(un)register_device. Since there's only one
caller of each, let's just call the mdev routines directly.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_drv.c     |  4 ++--
 drivers/s390/cio/vfio_ccw_ops.c     | 10 ----------
 drivers/s390/cio/vfio_ccw_private.h |  3 ---
 3 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index 279ad2161f17..fe87a2652a22 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -240,7 +240,7 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
 
 	private->state = VFIO_CCW_STATE_STANDBY;
 
-	ret = vfio_ccw_mdev_reg(sch);
+	ret = mdev_register_device(&sch->dev, &vfio_ccw_mdev_driver);
 	if (ret)
 		goto out_disable;
 
@@ -262,7 +262,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_mdev_unreg(sch);
+	mdev_unregister_device(&sch->dev);
 
 	dev_set_drvdata(&sch->dev, NULL);
 
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 81377270d4a7..a7ea9358e461 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -654,13 +654,3 @@ struct mdev_driver vfio_ccw_mdev_driver = {
 	.remove = vfio_ccw_mdev_remove,
 	.supported_type_groups  = mdev_type_groups,
 };
-
-int vfio_ccw_mdev_reg(struct subchannel *sch)
-{
-	return mdev_register_device(&sch->dev, &vfio_ccw_mdev_driver);
-}
-
-void vfio_ccw_mdev_unreg(struct subchannel *sch)
-{
-	mdev_unregister_device(&sch->dev);
-}
diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index 5c128eec596b..4cfdd5fc0961 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -117,9 +117,6 @@ struct vfio_ccw_private {
 	struct work_struct	crw_work;
 } __aligned(8);
 
-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;
-- 
2.32.0

[PATCH v3 07/11] vfio/ccw: Update trace data for not operational event

[Eric Farman]

We currently cut a very basic trace whenever the FSM directs
control to the not operational routine.

Convert this to a message, so it's alongside the other configuration
related traces (create, remove, etc.), and record both the event
that brought us here and the current state of the device.
This will provide some better footprints if things go bad.

Suggested-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_fsm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index bbcc5b486749..88e529a2e184 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -160,8 +160,12 @@ static void fsm_notoper(struct vfio_ccw_private *private,
 {
 	struct subchannel *sch = private->sch;
 
-	VFIO_CCW_TRACE_EVENT(2, "notoper");
-	VFIO_CCW_TRACE_EVENT(2, dev_name(&sch->dev));
+	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: notoper event %x state %x\n",
+			   sch->schid.cssid,
+			   sch->schid.ssid,
+			   sch->schid.sch_no,
+			   event,
+			   private->state);
 
 	/*
 	 * TODO:
-- 
2.32.0

Re: [PATCH v3 07/11] vfio/ccw: Update trace data for not operational event

[Matthew Rosato]

On 6/30/22 4:36 PM, Eric Farman wrote:
> We currently cut a very basic trace whenever the FSM directs
> control to the not operational routine.
> 
> Convert this to a message, so it's alongside the other configuration
> related traces (create, remove, etc.), and record both the event
> that brought us here and the current state of the device.
> This will provide some better footprints if things go bad.
> 
> Suggested-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>   drivers/s390/cio/vfio_ccw_fsm.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
> index bbcc5b486749..88e529a2e184 100644
> --- a/drivers/s390/cio/vfio_ccw_fsm.c
> +++ b/drivers/s390/cio/vfio_ccw_fsm.c
> @@ -160,8 +160,12 @@ static void fsm_notoper(struct vfio_ccw_private *private,
>   {
>   	struct subchannel *sch = private->sch;
>   
> -	VFIO_CCW_TRACE_EVENT(2, "notoper");
> -	VFIO_CCW_TRACE_EVENT(2, dev_name(&sch->dev));
> +	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: notoper event %x state %x\n",
> +			   sch->schid.cssid,
> +			   sch->schid.ssid,
> +			   sch->schid.sch_no,
> +			   event,
> +			   private->state);
>   
>   	/*
>   	 * TODO:

[PATCH v3 08/11] vfio/ccw: Create an OPEN FSM Event

[Eric Farman]

Move the process of enabling a subchannel for use by vfio-ccw
into the FSM, such that it can manage the sequence of lifecycle
events for the device.

That is, if the FSM state is NOT_OPER(erational), then do the work
that would enable the subchannel and move the FSM to STANDBY state.
An attempt to perform this event again from any of the other operating
states (IDLE, CP_PROCESSING, CP_PENDING) will convert the device back
to NOT_OPER so the configuration process can be started again.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/s390/cio/vfio_ccw_drv.c     |  9 ++-------
 drivers/s390/cio/vfio_ccw_fsm.c     | 21 +++++++++++++++++++++
 drivers/s390/cio/vfio_ccw_private.h |  1 +
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index fe87a2652a22..7d9189640da3 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -231,15 +231,10 @@ 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)
+	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
+	if (private->state == VFIO_CCW_STATE_NOT_OPER)
 		goto out_free;
 
-	private->state = VFIO_CCW_STATE_STANDBY;
-
 	ret = mdev_register_device(&sch->dev, &vfio_ccw_mdev_driver);
 	if (ret)
 		goto out_disable;
diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index 88e529a2e184..2811b2040490 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -11,6 +11,8 @@
 
 #include <linux/vfio.h>
 
+#include <asm/isc.h>
+
 #include "ioasm.h"
 #include "vfio_ccw_private.h"
 
@@ -368,6 +370,20 @@ 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_STANDBY;
+	spin_unlock_irq(sch->lock);
+}
+
 /*
  * Device statemachine
  */
@@ -377,29 +393,34 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = {
 		[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_open,
 	},
 	[VFIO_CCW_STATE_STANDBY] = {
 		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
 		[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_OPEN]		= fsm_notoper,
 	},
 	[VFIO_CCW_STATE_IDLE] = {
 		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
 		[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_notoper,
 	},
 	[VFIO_CCW_STATE_CP_PROCESSING] = {
 		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
 		[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_notoper,
 	},
 	[VFIO_CCW_STATE_CP_PENDING] = {
 		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
 		[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_notoper,
 	},
 };
diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index 4cfdd5fc0961..8dff1699a7d9 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -142,6 +142,7 @@ enum vfio_ccw_event {
 	VFIO_CCW_EVENT_IO_REQ,
 	VFIO_CCW_EVENT_INTERRUPT,
 	VFIO_CCW_EVENT_ASYNC_REQ,
+	VFIO_CCW_EVENT_OPEN,
 	/* last element! */
 	NR_VFIO_CCW_EVENTS
 };
-- 
2.32.0

Re: [PATCH v3 08/11] vfio/ccw: Create an OPEN FSM Event

[Matthew Rosato]

On 6/30/22 4:36 PM, Eric Farman wrote:
> Move the process of enabling a subchannel for use by vfio-ccw
> into the FSM, such that it can manage the sequence of lifecycle
> events for the device.
> 
> That is, if the FSM state is NOT_OPER(erational), then do the work
> that would enable the subchannel and move the FSM to STANDBY state.
> An attempt to perform this event again from any of the other operating
> states (IDLE, CP_PROCESSING, CP_PENDING) will convert the device back
> to NOT_OPER so the configuration process can be started again.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>   drivers/s390/cio/vfio_ccw_drv.c     |  9 ++-------
>   drivers/s390/cio/vfio_ccw_fsm.c     | 21 +++++++++++++++++++++
>   drivers/s390/cio/vfio_ccw_private.h |  1 +
>   3 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index fe87a2652a22..7d9189640da3 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -231,15 +231,10 @@ 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)
> +	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
> +	if (private->state == VFIO_CCW_STATE_NOT_OPER)
>   		goto out_free;
>   
> -	private->state = VFIO_CCW_STATE_STANDBY;
> -
>   	ret = mdev_register_device(&sch->dev, &vfio_ccw_mdev_driver);
>   	if (ret)
>   		goto out_disable;
> diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
> index 88e529a2e184..2811b2040490 100644
> --- a/drivers/s390/cio/vfio_ccw_fsm.c
> +++ b/drivers/s390/cio/vfio_ccw_fsm.c
> @@ -11,6 +11,8 @@
>   
>   #include <linux/vfio.h>
>   
> +#include <asm/isc.h>
> +
>   #include "ioasm.h"
>   #include "vfio_ccw_private.h"
>   
> @@ -368,6 +370,20 @@ 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_STANDBY;
> +	spin_unlock_irq(sch->lock);
> +}
> +
>   /*
>    * Device statemachine
>    */
> @@ -377,29 +393,34 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = {
>   		[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_open,
>   	},
>   	[VFIO_CCW_STATE_STANDBY] = {
>   		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
>   		[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_OPEN]		= fsm_notoper,
>   	},
>   	[VFIO_CCW_STATE_IDLE] = {
>   		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
>   		[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_notoper,
>   	},
>   	[VFIO_CCW_STATE_CP_PROCESSING] = {
>   		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
>   		[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_notoper,
>   	},
>   	[VFIO_CCW_STATE_CP_PENDING] = {
>   		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
>   		[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_notoper,
>   	},
>   };
> diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
> index 4cfdd5fc0961..8dff1699a7d9 100644
> --- a/drivers/s390/cio/vfio_ccw_private.h
> +++ b/drivers/s390/cio/vfio_ccw_private.h
> @@ -142,6 +142,7 @@ enum vfio_ccw_event {
>   	VFIO_CCW_EVENT_IO_REQ,
>   	VFIO_CCW_EVENT_INTERRUPT,
>   	VFIO_CCW_EVENT_ASYNC_REQ,
> +	VFIO_CCW_EVENT_OPEN,
>   	/* last element! */
>   	NR_VFIO_CCW_EVENTS
>   };

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

[Eric Farman]

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>
---
 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
 };
-- 
2.32.0

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

[Matthew Rosato]

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
>   };

[PATCH v3 10/11] vfio/ccw: Refactor vfio_ccw_mdev_reset

[Eric Farman]

Use both the FSM Close and Open events when resetting an mdev,
rather than making a separate call to cio_enable_subchannel().

Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/s390/cio/vfio_ccw_ops.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index fc5b83187bd9..4673b7ddfe20 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -21,25 +21,21 @@ 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
-	 * interrupt pending" are clear, but we are not sure what other state
-	 * we need to care about.
-	 * There are still a lot more instructions need to be handled. We
-	 * should come back here later.
+	 * If the FSM state is seen as Not Operational after closing
+	 * and re-opening the mdev, return an error.
+	 *
+	 * Otherwise, change the FSM from STANDBY to IDLE which is
+	 * normally done by vfio_ccw_mdev_probe() in current lifecycle.
 	 */
 	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
+	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
+	if (private->state == VFIO_CCW_STATE_NOT_OPER)
+		return -EINVAL;
 
-	ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch);
-	if (!ret)
-		private->state = VFIO_CCW_STATE_IDLE;
+	private->state = VFIO_CCW_STATE_IDLE;
 
-	return ret;
+	return 0;
 }
 
 static int vfio_ccw_mdev_notifier(struct notifier_block *nb,
-- 
2.32.0

Re: [PATCH v3 10/11] vfio/ccw: Refactor vfio_ccw_mdev_reset

[Matthew Rosato]

On 6/30/22 4:36 PM, Eric Farman wrote:
> Use both the FSM Close and Open events when resetting an mdev,
> rather than making a separate call to cio_enable_subchannel().
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>   drivers/s390/cio/vfio_ccw_ops.c | 24 ++++++++++--------------
>   1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index fc5b83187bd9..4673b7ddfe20 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -21,25 +21,21 @@ 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
> -	 * interrupt pending" are clear, but we are not sure what other state
> -	 * we need to care about.
> -	 * There are still a lot more instructions need to be handled. We
> -	 * should come back here later.
> +	 * If the FSM state is seen as Not Operational after closing
> +	 * and re-opening the mdev, return an error.
> +	 *
> +	 * Otherwise, change the FSM from STANDBY to IDLE which is
> +	 * normally done by vfio_ccw_mdev_probe() in current lifecycle.
>   	 */
>   	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
> +	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
> +	if (private->state == VFIO_CCW_STATE_NOT_OPER)
> +		return -EINVAL;
>   
> -	ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch);
> -	if (!ret)
> -		private->state = VFIO_CCW_STATE_IDLE;
> +	private->state = VFIO_CCW_STATE_IDLE;
>   
> -	return ret;
> +	return 0;
>   }
>   
>   static int vfio_ccw_mdev_notifier(struct notifier_block *nb,

[PATCH v3 11/11] vfio/ccw: Move FSM open/close to MDEV open/close

[Eric Farman]

Part of the confusion that has existed is the FSM lifecycle of
subchannels between the common CSS driver and the vfio-ccw driver.
During configuration, the FSM state goes from NOT_OPER to STANDBY
to IDLE, but then back to NOT_OPER. For example:

	vfio_ccw_sch_probe:		VFIO_CCW_STATE_NOT_OPER
	vfio_ccw_sch_probe:		VFIO_CCW_STATE_STANDBY
	vfio_ccw_mdev_probe:		VFIO_CCW_STATE_IDLE
	vfio_ccw_mdev_remove:		VFIO_CCW_STATE_NOT_OPER
	vfio_ccw_sch_remove:		VFIO_CCW_STATE_NOT_OPER
	vfio_ccw_sch_shutdown:		VFIO_CCW_STATE_NOT_OPER

Rearrange the open/close events to align with the mdev open/close,
to better manage the memory and state of the devices as time
progresses. Specifically, make mdev_open() perform the FSM open,
and mdev_close() perform the FSM close instead of reset (which is
both close and open).

This makes the NOT_OPER state a dead-end path, indicating the
device is probably not recoverable without fully probing and
re-configuring the device.

This has the nice side-effect of removing a number of special-cases
where the FSM state is managed outside of the FSM itself (such as
the aforementioned mdev_close() routine).

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/s390/cio/vfio_ccw_drv.c | 11 +++--------
 drivers/s390/cio/vfio_ccw_fsm.c | 32 +++++++++++++++++++++++---------
 drivers/s390/cio/vfio_ccw_ops.c | 26 +++++++++++---------------
 3 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index f98c9915e73d..4804101ccb0f 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -138,7 +138,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_STANDBY;
 	INIT_LIST_HEAD(&private->crw);
 	INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo);
 	INIT_WORK(&private->crw_work, vfio_ccw_crw_todo);
@@ -222,21 +222,15 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
 
 	dev_set_drvdata(&sch->dev, private);
 
-	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
-	if (private->state == VFIO_CCW_STATE_NOT_OPER)
-		goto out_free;
-
 	ret = mdev_register_device(&sch->dev, &vfio_ccw_mdev_driver);
 	if (ret)
-		goto out_disable;
+		goto out_free;
 
 	VFIO_CCW_MSG_EVENT(4, "bound to subchannel %x.%x.%04x\n",
 			   sch->schid.cssid, sch->schid.ssid,
 			   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);
@@ -264,6 +258,7 @@ static void vfio_ccw_sch_shutdown(struct subchannel *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_NOT_OPER);
 }
 
 /**
diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
index 89eb3feffa41..472e77f1bb6e 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -175,6 +175,7 @@ static void fsm_notoper(struct vfio_ccw_private *private,
 	 */
 	css_sched_sch_todo(sch, SCH_TODO_UNREG);
 	private->state = VFIO_CCW_STATE_NOT_OPER;
+	cp_free(&private->cp);
 }
 
 /*
@@ -379,9 +380,16 @@ static void fsm_open(struct vfio_ccw_private *private,
 	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_STANDBY;
+	if (ret)
+		goto err_unlock;
+
+	private->state = VFIO_CCW_STATE_IDLE;
 	spin_unlock_irq(sch->lock);
+	return;
+
+err_unlock:
+	spin_unlock_irq(sch->lock);
+	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
 }
 
 static void fsm_close(struct vfio_ccw_private *private,
@@ -393,16 +401,22 @@ static void fsm_close(struct vfio_ccw_private *private,
 	spin_lock_irq(sch->lock);
 
 	if (!sch->schib.pmcw.ena)
-		goto out_unlock;
+		goto err_unlock;
 
 	ret = cio_disable_subchannel(sch);
 	if (ret == -EBUSY)
 		vfio_ccw_sch_quiesce(sch);
+	if (ret)
+		goto err_unlock;
 
-out_unlock:
-	private->state = VFIO_CCW_STATE_NOT_OPER;
+	private->state = VFIO_CCW_STATE_STANDBY;
 	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_NOT_OPER);
 }
 
 /*
@@ -414,16 +428,16 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = {
 		[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_open,
+		[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_EVENT_IO_REQ]		= fsm_io_error,
 		[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_EVENT_INTERRUPT]	= fsm_disabled_irq,
+		[VFIO_CCW_EVENT_OPEN]		= fsm_open,
+		[VFIO_CCW_EVENT_CLOSE]		= fsm_notoper,
 	},
 	[VFIO_CCW_STATE_IDLE] = {
 		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 4673b7ddfe20..bc2176421dc5 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -24,17 +24,12 @@ static int vfio_ccw_mdev_reset(struct vfio_ccw_private *private)
 	/*
 	 * If the FSM state is seen as Not Operational after closing
 	 * and re-opening the mdev, return an error.
-	 *
-	 * Otherwise, change the FSM from STANDBY to IDLE which is
-	 * normally done by vfio_ccw_mdev_probe() in current lifecycle.
 	 */
 	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
 	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
 	if (private->state == VFIO_CCW_STATE_NOT_OPER)
 		return -EINVAL;
 
-	private->state = VFIO_CCW_STATE_IDLE;
-
 	return 0;
 }
 
@@ -121,8 +116,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
 	vfio_init_group_dev(&private->vdev, &mdev->dev,
 			    &vfio_ccw_dev_ops);
 
-	private->state = VFIO_CCW_STATE_IDLE;
-
 	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: create\n",
 			   private->sch->schid.cssid,
 			   private->sch->schid.ssid,
@@ -137,7 +130,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
 err_atomic:
 	vfio_uninit_group_dev(&private->vdev);
 	atomic_inc(&private->avail);
-	private->state = VFIO_CCW_STATE_STANDBY;
 	return ret;
 }
 
@@ -165,6 +157,10 @@ static int vfio_ccw_mdev_open_device(struct vfio_device *vdev)
 	unsigned long events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
 	int ret;
 
+	/* Device cannot simply be opened again from this state */
+	if (private->state == VFIO_CCW_STATE_NOT_OPER)
+		return -EINVAL;
+
 	private->nb.notifier_call = vfio_ccw_mdev_notifier;
 
 	ret = vfio_register_notifier(vdev, VFIO_IOMMU_NOTIFY,
@@ -184,6 +180,12 @@ 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_NOT_OPER) {
+		ret = -EINVAL;
+		goto out_unregister;
+	}
+
 	return ret;
 
 out_unregister:
@@ -197,13 +199,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. */
-	}
-
+	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
 	vfio_ccw_unregister_dev_regions(private);
 	vfio_unregister_notifier(vdev, VFIO_IOMMU_NOTIFY, &private->nb);
 }
-- 
2.32.0

Re: [PATCH v3 11/11] vfio/ccw: Move FSM open/close to MDEV open/close

[Matthew Rosato]

On 6/30/22 4:36 PM, Eric Farman wrote:
> Part of the confusion that has existed is the FSM lifecycle of
> subchannels between the common CSS driver and the vfio-ccw driver.
> During configuration, the FSM state goes from NOT_OPER to STANDBY
> to IDLE, but then back to NOT_OPER. For example:
> 
> 	vfio_ccw_sch_probe:		VFIO_CCW_STATE_NOT_OPER
> 	vfio_ccw_sch_probe:		VFIO_CCW_STATE_STANDBY
> 	vfio_ccw_mdev_probe:		VFIO_CCW_STATE_IDLE
> 	vfio_ccw_mdev_remove:		VFIO_CCW_STATE_NOT_OPER
> 	vfio_ccw_sch_remove:		VFIO_CCW_STATE_NOT_OPER
> 	vfio_ccw_sch_shutdown:		VFIO_CCW_STATE_NOT_OPER
> 
> Rearrange the open/close events to align with the mdev open/close,
> to better manage the memory and state of the devices as time
> progresses. Specifically, make mdev_open() perform the FSM open,
> and mdev_close() perform the FSM close instead of reset (which is
> both close and open).
> 
> This makes the NOT_OPER state a dead-end path, indicating the
> device is probably not recoverable without fully probing and
> re-configuring the device.
> 
> This has the nice side-effect of removing a number of special-cases
> where the FSM state is managed outside of the FSM itself (such as
> the aforementioned mdev_close() routine).
> 
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>   drivers/s390/cio/vfio_ccw_drv.c | 11 +++--------
>   drivers/s390/cio/vfio_ccw_fsm.c | 32 +++++++++++++++++++++++---------
>   drivers/s390/cio/vfio_ccw_ops.c | 26 +++++++++++---------------
>   3 files changed, 37 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index f98c9915e73d..4804101ccb0f 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -138,7 +138,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_STANDBY;
>   	INIT_LIST_HEAD(&private->crw);
>   	INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo);
>   	INIT_WORK(&private->crw_work, vfio_ccw_crw_todo);
> @@ -222,21 +222,15 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
>   
>   	dev_set_drvdata(&sch->dev, private);
>   
> -	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
> -	if (private->state == VFIO_CCW_STATE_NOT_OPER)
> -		goto out_free;
> -
>   	ret = mdev_register_device(&sch->dev, &vfio_ccw_mdev_driver);
>   	if (ret)
> -		goto out_disable;
> +		goto out_free;
>   
>   	VFIO_CCW_MSG_EVENT(4, "bound to subchannel %x.%x.%04x\n",
>   			   sch->schid.cssid, sch->schid.ssid,
>   			   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);
> @@ -264,6 +258,7 @@ static void vfio_ccw_sch_shutdown(struct subchannel *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_NOT_OPER);
>   }
>   
>   /**
> diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
> index 89eb3feffa41..472e77f1bb6e 100644
> --- a/drivers/s390/cio/vfio_ccw_fsm.c
> +++ b/drivers/s390/cio/vfio_ccw_fsm.c
> @@ -175,6 +175,7 @@ static void fsm_notoper(struct vfio_ccw_private *private,
>   	 */
>   	css_sched_sch_todo(sch, SCH_TODO_UNREG);
>   	private->state = VFIO_CCW_STATE_NOT_OPER;
> +	cp_free(&private->cp);
>   }
>   
>   /*
> @@ -379,9 +380,16 @@ static void fsm_open(struct vfio_ccw_private *private,
>   	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_STANDBY;
> +	if (ret)
> +		goto err_unlock;
> +
> +	private->state = VFIO_CCW_STATE_IDLE;
>   	spin_unlock_irq(sch->lock);
> +	return;
> +
> +err_unlock:
> +	spin_unlock_irq(sch->lock);
> +	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER);
>   }
>   
>   static void fsm_close(struct vfio_ccw_private *private,
> @@ -393,16 +401,22 @@ static void fsm_close(struct vfio_ccw_private *private,
>   	spin_lock_irq(sch->lock);
>   
>   	if (!sch->schib.pmcw.ena)
> -		goto out_unlock;
> +		goto err_unlock;
>   
>   	ret = cio_disable_subchannel(sch);
>   	if (ret == -EBUSY)
>   		vfio_ccw_sch_quiesce(sch);
> +	if (ret)
> +		goto err_unlock;
>   
> -out_unlock:
> -	private->state = VFIO_CCW_STATE_NOT_OPER;
> +	private->state = VFIO_CCW_STATE_STANDBY;
>   	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_NOT_OPER);
>   }
>   
>   /*
> @@ -414,16 +428,16 @@ fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS] = {
>   		[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_open,
> +		[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_EVENT_IO_REQ]		= fsm_io_error,
>   		[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_EVENT_INTERRUPT]	= fsm_disabled_irq,
> +		[VFIO_CCW_EVENT_OPEN]		= fsm_open,
> +		[VFIO_CCW_EVENT_CLOSE]		= fsm_notoper,
>   	},
>   	[VFIO_CCW_STATE_IDLE] = {
>   		[VFIO_CCW_EVENT_NOT_OPER]	= fsm_notoper,
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index 4673b7ddfe20..bc2176421dc5 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -24,17 +24,12 @@ static int vfio_ccw_mdev_reset(struct vfio_ccw_private *private)
>   	/*
>   	 * If the FSM state is seen as Not Operational after closing
>   	 * and re-opening the mdev, return an error.
> -	 *
> -	 * Otherwise, change the FSM from STANDBY to IDLE which is
> -	 * normally done by vfio_ccw_mdev_probe() in current lifecycle.
>   	 */
>   	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
>   	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_OPEN);
>   	if (private->state == VFIO_CCW_STATE_NOT_OPER)
>   		return -EINVAL;
>   
> -	private->state = VFIO_CCW_STATE_IDLE;
> -
>   	return 0;
>   }
>   
> @@ -121,8 +116,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
>   	vfio_init_group_dev(&private->vdev, &mdev->dev,
>   			    &vfio_ccw_dev_ops);
>   
> -	private->state = VFIO_CCW_STATE_IDLE;
> -
>   	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: create\n",
>   			   private->sch->schid.cssid,
>   			   private->sch->schid.ssid,
> @@ -137,7 +130,6 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev)
>   err_atomic:
>   	vfio_uninit_group_dev(&private->vdev);
>   	atomic_inc(&private->avail);
> -	private->state = VFIO_CCW_STATE_STANDBY;
>   	return ret;
>   }
>   
> @@ -165,6 +157,10 @@ static int vfio_ccw_mdev_open_device(struct vfio_device *vdev)
>   	unsigned long events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
>   	int ret;
>   
> +	/* Device cannot simply be opened again from this state */
> +	if (private->state == VFIO_CCW_STATE_NOT_OPER)
> +		return -EINVAL;
> +
>   	private->nb.notifier_call = vfio_ccw_mdev_notifier;
>   
>   	ret = vfio_register_notifier(vdev, VFIO_IOMMU_NOTIFY,
> @@ -184,6 +180,12 @@ 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_NOT_OPER) {
> +		ret = -EINVAL;
> +		goto out_unregister;
> +	}
> +
>   	return ret;
>   
>   out_unregister:
> @@ -197,13 +199,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. */
> -	}
> -
> +	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
>   	vfio_ccw_unregister_dev_regions(private);
>   	vfio_unregister_notifier(vdev, VFIO_IOMMU_NOTIFY, &private->nb);
>   }

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Jason Gunthorpe]

On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
> Here's an updated pass through the first chunk of vfio-ccw rework.
> 
> As with v2, this is all internal to vfio-ccw, with the exception of
> the removal of mdev_uuid from include/linux/mdev.h in patch 1.
> 
> There is one conflict with the vfio-next branch [2], on patch 6.

What tree do you plan to take it through?

> The remainder of the work that Jason Gunthorpe originally started [1]
> in this space remains for a future day.

Lets see.. These were already applied:

  vfio/ccw: Remove unneeded GFP_DMA
  vfio/ccw: Use functions for alloc/free of the vfio_ccw_private
  vfio/ccw: Pass vfio_ccw_private not mdev_device to various functions
  vfio/ccw: Convert to use vfio_register_emulated_iommu_dev()

This series replaces this one:
  vfio/ccw: Make the FSM complete and synchronize it to the mdev

Christoph recently re-posted this:
https://lore.kernel.org/kvm/20220628051435.695540-10-hch@lst.de/
  vfio/mdev: Consolidate all the device_api sysfs into the core code

So this is still left ?
  vfio/ccw: Remove private->mdev
  vfio: Export vfio_device_try_get()
  vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the
    mdev

IIRC Kevin's team needs those for their device FD patches?

Thanks,
Jason

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Eric Farman]

On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
> On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
> > Here's an updated pass through the first chunk of vfio-ccw rework.
> > 
> > As with v2, this is all internal to vfio-ccw, with the exception of
> > the removal of mdev_uuid from include/linux/mdev.h in patch 1.
> > 
> > There is one conflict with the vfio-next branch [2], on patch 6.
> 
> What tree do you plan to take it through?

Don't know. I know Matt's PCI series has a conflict with this same
patch also, but I haven't seen resolution to that. @Christian,
thoughts?

> 
> > The remainder of the work that Jason Gunthorpe originally started
> > [1]
> > in this space remains for a future day.
> 
> Lets see.. These were already applied:
> 
>   vfio/ccw: Remove unneeded GFP_DMA
>   vfio/ccw: Use functions for alloc/free of the vfio_ccw_private
>   vfio/ccw: Pass vfio_ccw_private not mdev_device to various
> functions
>   vfio/ccw: Convert to use vfio_register_emulated_iommu_dev()
> 
> This series replaces this one:
>   vfio/ccw: Make the FSM complete and synchronize it to the mdev
> 
> Christoph recently re-posted this:
> https://lore.kernel.org/kvm/20220628051435.695540-10-hch@lst.de/
>   vfio/mdev: Consolidate all the device_api sysfs into the core code

Correct. Same for "vfio/mdev: Add mdev available instance checking to
the core" which you originally had proposed.

> 
> So this is still left ?
>   vfio/ccw: Remove private->mdev

This is by this series (patch 1-4).

>   vfio: Export vfio_device_try_get()
>   vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the
>     mdev
> 
> IIRC Kevin's team needs those for their device FD patches?

That's my understanding too. 

> 
> Thanks,
> Jason

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Christian Borntraeger]

Am 01.07.22 um 14:40 schrieb Eric Farman:
> On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
>> On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
>>> Here's an updated pass through the first chunk of vfio-ccw rework.
>>>
>>> As with v2, this is all internal to vfio-ccw, with the exception of
>>> the removal of mdev_uuid from include/linux/mdev.h in patch 1.
>>>
>>> There is one conflict with the vfio-next branch [2], on patch 6.
>>
>> What tree do you plan to take it through?
> 
> Don't know. I know Matt's PCI series has a conflict with this same
> patch also, but I haven't seen resolution to that. @Christian,
> thoughts?


What about me making a topic branch that it being merged by Alex AND the KVM tree
so that each of the conflicts can be solved in that way?

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Jason Gunthorpe]

On Fri, Jul 01, 2022 at 02:48:25PM +0200, Christian Borntraeger wrote:

> Am 01.07.22 um 14:40 schrieb Eric Farman:
> > On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
> > > On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
> > > > Here's an updated pass through the first chunk of vfio-ccw rework.
> > > > 
> > > > As with v2, this is all internal to vfio-ccw, with the exception of
> > > > the removal of mdev_uuid from include/linux/mdev.h in patch 1.
> > > > 
> > > > There is one conflict with the vfio-next branch [2], on patch 6.
> > > 
> > > What tree do you plan to take it through?
> > 
> > Don't know. I know Matt's PCI series has a conflict with this same
> > patch also, but I haven't seen resolution to that. @Christian,
> > thoughts?
> 
> 
> What about me making a topic branch that it being merged by Alex AND the KVM tree
> so that each of the conflicts can be solved in that way?

It make sense, I would base it on Alex's VFIO tree just to avoid
some conflicts in the first place. Matt can rebase on this, so lets
get things going?

Jason

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Christian Borntraeger]

Am 04.07.22 um 13:25 schrieb Jason Gunthorpe:
> On Fri, Jul 01, 2022 at 02:48:25PM +0200, Christian Borntraeger wrote:
> 
>> Am 01.07.22 um 14:40 schrieb Eric Farman:
>>> On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
>>>> On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
>>>>> Here's an updated pass through the first chunk of vfio-ccw rework.
>>>>>
>>>>> As with v2, this is all internal to vfio-ccw, with the exception of
>>>>> the removal of mdev_uuid from include/linux/mdev.h in patch 1.
>>>>>
>>>>> There is one conflict with the vfio-next branch [2], on patch 6.
>>>>
>>>> What tree do you plan to take it through?
>>>
>>> Don't know. I know Matt's PCI series has a conflict with this same
>>> patch also, but I haven't seen resolution to that. @Christian,
>>> thoughts?
>>
>>
>> What about me making a topic branch that it being merged by Alex AND the KVM tree
>> so that each of the conflicts can be solved in that way?
> 
> It make sense, I would base it on Alex's VFIO tree just to avoid
> some conflicts in the first place. Matt can rebase on this, so lets
> get things going?

So yes. Lets rebase on VFIO-next. Ideally Alex would then directly pick Eric
patches.

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Matthew Rosato]

On 7/7/22 5:06 AM, Christian Borntraeger wrote:
> 
> 
> Am 04.07.22 um 13:25 schrieb Jason Gunthorpe:
>> On Fri, Jul 01, 2022 at 02:48:25PM +0200, Christian Borntraeger wrote:
>>
>>> Am 01.07.22 um 14:40 schrieb Eric Farman:
>>>> On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
>>>>> On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
>>>>>> Here's an updated pass through the first chunk of vfio-ccw rework.
>>>>>>
>>>>>> As with v2, this is all internal to vfio-ccw, with the exception of
>>>>>> the removal of mdev_uuid from include/linux/mdev.h in patch 1.
>>>>>>
>>>>>> There is one conflict with the vfio-next branch [2], on patch 6.
>>>>>
>>>>> What tree do you plan to take it through?
>>>>
>>>> Don't know. I know Matt's PCI series has a conflict with this same
>>>> patch also, but I haven't seen resolution to that. @Christian,
>>>> thoughts?
>>>
>>>
>>> What about me making a topic branch that it being merged by Alex AND 
>>> the KVM tree
>>> so that each of the conflicts can be solved in that way?
>>
>> It make sense, I would base it on Alex's VFIO tree just to avoid
>> some conflicts in the first place. Matt can rebase on this, so lets
>> get things going?
> 
> So yes. Lets rebase on VFIO-next. Ideally Alex would then directly pick 
> Eric
> patches.

@Christian to be clear, do you want me to also rebase the zPCI series on 
vfio-next then?

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Christian Borntraeger]

Am 07.07.22 um 14:34 schrieb Matthew Rosato:
> On 7/7/22 5:06 AM, Christian Borntraeger wrote:
>>
>>
>> Am 04.07.22 um 13:25 schrieb Jason Gunthorpe:
>>> On Fri, Jul 01, 2022 at 02:48:25PM +0200, Christian Borntraeger wrote:
>>>
>>>> Am 01.07.22 um 14:40 schrieb Eric Farman:
>>>>> On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
>>>>>> On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
>>>>>>> Here's an updated pass through the first chunk of vfio-ccw rework.
>>>>>>>
>>>>>>> As with v2, this is all internal to vfio-ccw, with the exception of
>>>>>>> the removal of mdev_uuid from include/linux/mdev.h in patch 1.
>>>>>>>
>>>>>>> There is one conflict with the vfio-next branch [2], on patch 6.
>>>>>>
>>>>>> What tree do you plan to take it through?
>>>>>
>>>>> Don't know. I know Matt's PCI series has a conflict with this same
>>>>> patch also, but I haven't seen resolution to that. @Christian,
>>>>> thoughts?
>>>>
>>>>
>>>> What about me making a topic branch that it being merged by Alex AND the KVM tree
>>>> so that each of the conflicts can be solved in that way?
>>>
>>> It make sense, I would base it on Alex's VFIO tree just to avoid
>>> some conflicts in the first place. Matt can rebase on this, so lets
>>> get things going?
>>
>> So yes. Lets rebase on VFIO-next. Ideally Alex would then directly pick Eric
>> patches.
> 
> @Christian to be clear, do you want me to also rebase the zPCI series on vfio-next then?

For that we are probably better of me having a topic branch that is then merged by Alex
and Paolo. Alex, Paolo, would be make sense?

As an alternative: will the vfio patches build without the KVM patches and vice versa,
I assume not?

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Matthew Rosato]

On 7/7/22 9:04 AM, Christian Borntraeger wrote:
> Am 07.07.22 um 14:34 schrieb Matthew Rosato:
>> On 7/7/22 5:06 AM, Christian Borntraeger wrote:
>>>
>>>
>>> Am 04.07.22 um 13:25 schrieb Jason Gunthorpe:
>>>> On Fri, Jul 01, 2022 at 02:48:25PM +0200, Christian Borntraeger wrote:
>>>>
>>>>> Am 01.07.22 um 14:40 schrieb Eric Farman:
>>>>>> On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
>>>>>>> On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
>>>>>>>> Here's an updated pass through the first chunk of vfio-ccw rework.
>>>>>>>>
>>>>>>>> As with v2, this is all internal to vfio-ccw, with the exception of
>>>>>>>> the removal of mdev_uuid from include/linux/mdev.h in patch 1.
>>>>>>>>
>>>>>>>> There is one conflict with the vfio-next branch [2], on patch 6.
>>>>>>>
>>>>>>> What tree do you plan to take it through?
>>>>>>
>>>>>> Don't know. I know Matt's PCI series has a conflict with this same
>>>>>> patch also, but I haven't seen resolution to that. @Christian,
>>>>>> thoughts?
>>>>>
>>>>>
>>>>> What about me making a topic branch that it being merged by Alex 
>>>>> AND the KVM tree
>>>>> so that each of the conflicts can be solved in that way?
>>>>
>>>> It make sense, I would base it on Alex's VFIO tree just to avoid
>>>> some conflicts in the first place. Matt can rebase on this, so lets
>>>> get things going?
>>>
>>> So yes. Lets rebase on VFIO-next. Ideally Alex would then directly 
>>> pick Eric
>>> patches.
>>
>> @Christian to be clear, do you want me to also rebase the zPCI series 
>> on vfio-next then?
> 
> For that we are probably better of me having a topic branch that is then 
> merged by Alex
> and Paolo. Alex, Paolo, would be make sense?

For reference if needed, the zPCI series in question:
https://lore.kernel.org/linux-s390/20220606203325.110625-1-mjrosato@linux.ibm.com/

> 
> As an alternative: will the vfio patches build without the KVM patches 
> and vice versa,
> I assume not?

No, there are dependencies in both directions.

At this point if the topic branch is how we will proceed then I suggest 
just taking v9 as-is; the few minor nit comments from Pierre can be 
addressed as follow-ons if desired.

Re: [PATCH v3 00/11] s390/vfio-ccw rework

[Yi Liu]

On 2022/7/1 20:40, Eric Farman wrote:
> On Thu, 2022-06-30 at 20:44 -0300, Jason Gunthorpe wrote:
>> On Thu, Jun 30, 2022 at 10:36:36PM +0200, Eric Farman wrote:
>>> Here's an updated pass through the first chunk of vfio-ccw rework.
>>>
>>> As with v2, this is all internal to vfio-ccw, with the exception of
>>> the removal of mdev_uuid from include/linux/mdev.h in patch 1.
>>>
>>> There is one conflict with the vfio-next branch [2], on patch 6.
>>
>> What tree do you plan to take it through?
> 
> Don't know. I know Matt's PCI series has a conflict with this same
> patch also, but I haven't seen resolution to that. @Christian,
> thoughts?
> 
>>
>>> The remainder of the work that Jason Gunthorpe originally started
>>> [1]
>>> in this space remains for a future day.
>>
>> Lets see.. These were already applied:
>>
>>    vfio/ccw: Remove unneeded GFP_DMA
>>    vfio/ccw: Use functions for alloc/free of the vfio_ccw_private
>>    vfio/ccw: Pass vfio_ccw_private not mdev_device to various
>> functions
>>    vfio/ccw: Convert to use vfio_register_emulated_iommu_dev()
>>
>> This series replaces this one:
>>    vfio/ccw: Make the FSM complete and synchronize it to the mdev
>>
>> Christoph recently re-posted this:
>> https://lore.kernel.org/kvm/20220628051435.695540-10-hch@lst.de/
>>    vfio/mdev: Consolidate all the device_api sysfs into the core code
> 
> Correct. Same for "vfio/mdev: Add mdev available instance checking to
> the core" which you originally had proposed.
> 
>>
>> So this is still left ?
>>    vfio/ccw: Remove private->mdev
> 
> This is by this series (patch 1-4).
> 
>>    vfio: Export vfio_device_try_get()
>>    vfio/ccw: Move the lifecycle of the struct vfio_ccw_private to the
>>      mdev
>>
>> IIRC Kevin's team needs those for their device FD patches?
> 
> That's my understanding too.

yes. You two have great memory. My vfio cdev patches relies on these two
patches. So far, my branch doesn't cover ccw. Do you have plan to 
incorporate them? I would like to apply your ccw series to below branch.

https://github.com/luxis1999/iommufd/commit/e6e52d0d2bba6510c0a9fec8184d5f169a50fda2

>>
>> Thanks,
>> Jason
> 

-- 
Regards,
Yi Liu