* [PATCH v4 01/11] vfio/ccw: Remove UUID from s390 debug log
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 02/11] vfio/ccw: Fix FSM state if mdev probe fails Eric Farman
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, Michael Kawano, Kirti Wankhede, 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 02/11] vfio/ccw: Fix FSM state if mdev probe fails
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
2022-07-07 13:57 ` [PATCH v4 01/11] vfio/ccw: Remove UUID from s390 debug log Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 03/11] vfio/ccw: Do not change FSM state in subchannel event Eric Farman
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 03/11] vfio/ccw: Do not change FSM state in subchannel event
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
2022-07-07 13:57 ` [PATCH v4 01/11] vfio/ccw: Remove UUID from s390 debug log Eric Farman
2022-07-07 13:57 ` [PATCH v4 02/11] vfio/ccw: Fix FSM state if mdev probe fails Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 04/11] vfio/ccw: Remove private->mdev Eric Farman
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 04/11] vfio/ccw: Remove private->mdev
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (2 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 03/11] vfio/ccw: Do not change FSM state in subchannel event Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 05/11] vfio/ccw: Pass enum to FSM event jumptable Eric Farman
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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 b7163bac8cc7..4d11ef48333e 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 05/11] vfio/ccw: Pass enum to FSM event jumptable
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (3 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 04/11] vfio/ccw: Remove private->mdev Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 06/11] vfio/ccw: Flatten MDEV device (un)register Eric Farman
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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 4d11ef48333e..5891bea8ce41 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 06/11] vfio/ccw: Flatten MDEV device (un)register
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (4 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 05/11] vfio/ccw: Pass enum to FSM event jumptable Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 07/11] vfio/ccw: Update trace data for not operational event Eric Farman
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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 5891bea8ce41..a2584c130e79 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);
-int vfio_ccw_mdev_reg(struct subchannel *sch);
-void vfio_ccw_mdev_unreg(struct subchannel *sch);
-
int vfio_ccw_sch_quiesce(struct subchannel *sch);
extern struct mdev_driver vfio_ccw_mdev_driver;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 07/11] vfio/ccw: Update trace data for not operational event
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (5 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 06/11] vfio/ccw: Flatten MDEV device (un)register Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 08/11] vfio/ccw: Create an OPEN FSM Event Eric Farman
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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>
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:
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 08/11] vfio/ccw: Create an OPEN FSM Event
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (6 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 07/11] vfio/ccw: Update trace data for not operational event Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 09/11] vfio/ccw: Create a CLOSE FSM event Eric Farman
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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>
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 a2584c130e79..93e136ba369b 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 09/11] vfio/ccw: Create a CLOSE FSM event
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (7 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 08/11] vfio/ccw: Create an OPEN FSM Event Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 10/11] vfio/ccw: Refactor vfio_ccw_mdev_reset Eric Farman
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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>
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 93e136ba369b..abac532bf03e 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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 10/11] vfio/ccw: Refactor vfio_ccw_mdev_reset
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (8 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 09/11] vfio/ccw: Create a CLOSE FSM event Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 13:57 ` [PATCH v4 11/11] vfio/ccw: Move FSM open/close to MDEV open/close Eric Farman
2022-07-07 21:32 ` [PATCH v4 00/11] s390/vfio-ccw rework Alex Williamson
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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>
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,
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 11/11] vfio/ccw: Move FSM open/close to MDEV open/close
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (9 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 10/11] vfio/ccw: Refactor vfio_ccw_mdev_reset Eric Farman
@ 2022-07-07 13:57 ` Eric Farman
2022-07-07 21:32 ` [PATCH v4 00/11] s390/vfio-ccw rework Alex Williamson
11 siblings, 0 replies; 13+ messages in thread
From: Eric Farman @ 2022-07-07 13:57 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, 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>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
drivers/s390/cio/vfio_ccw_drv.c | 11 +++--------
drivers/s390/cio/vfio_ccw_fsm.c | 34 ++++++++++++++++++++++++---------
drivers/s390/cio/vfio_ccw_ops.c | 26 +++++++++++--------------
3 files changed, 39 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..4b8b623df24f 100644
--- a/drivers/s390/cio/vfio_ccw_fsm.c
+++ b/drivers/s390/cio/vfio_ccw_fsm.c
@@ -175,6 +175,9 @@ static void fsm_notoper(struct vfio_ccw_private *private,
*/
css_sched_sch_todo(sch, SCH_TODO_UNREG);
private->state = VFIO_CCW_STATE_NOT_OPER;
+
+ /* This is usually handled during CLOSE event */
+ cp_free(&private->cp);
}
/*
@@ -379,9 +382,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 +403,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 +430,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.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 00/11] s390/vfio-ccw rework
2022-07-07 13:57 [PATCH v4 00/11] s390/vfio-ccw rework Eric Farman
` (10 preceding siblings ...)
2022-07-07 13:57 ` [PATCH v4 11/11] vfio/ccw: Move FSM open/close to MDEV open/close Eric Farman
@ 2022-07-07 21:32 ` Alex Williamson
11 siblings, 0 replies; 13+ messages in thread
From: Alex Williamson @ 2022-07-07 21:32 UTC (permalink / raw)
To: Eric Farman
Cc: Matthew Rosato, Jason Gunthorpe, Cornelia Huck, Halil Pasic, kvm,
linux-s390, Kirti Wankhede
On Thu, 7 Jul 2022 15:57:26 +0200
Eric Farman <farman@linux.ibm.com> wrote:
> Alex,
>
> Here's a final pass through some of the vfio-ccw rework.
> I'm hoping that because of the intersection with the extern
> removal, you could grab this directly? [1]
Done. Applied to vfio next branch for v5.20. Thanks,
Alex
> There were no code changes since v3, I simply rebased this
> onto your linux-vfio/next tree, currently on commit 7654a8881a54
> ("Merge branches
> 'v5.20/vfio/migration-enhancements-v3',
> 'v5.20/vfio/simplify-bus_type-determination-v3',
> 'v5.20/vfio/check-vfio_register_iommu_driver-return-v2',
> 'v5.20/vfio/check-iommu_group_set_name_return-v1',
> 'v5.20/vfio/clear-caps-buf-v3',
> 'v5.20/vfio/remove-useless-judgement-v1' and
> 'v5.20/vfio/move-device_open-count-v2'
> into
> v5.20/vfio/next")
>
> v3->v4:
> - Rebased to vfio/next tree
> - Tweak patch 6 to fit with extern removal
> - The rest applied directly
> - [MR] Added r-b's (Thank you!)
> - [EF] Add a comment about cp_free() call in fsm_notoper()
> v3: https://lore.kernel.org/r/20220630203647.2529815-1-farman@linux.ibm.com/
> 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/e1ead3e4-9e7d-f026-485b-157d7dc004d3@linux.ibm.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 | 99 ++++++++++++++++++++++++-----
> drivers/s390/cio/vfio_ccw_ops.c | 77 +++++++---------------
> drivers/s390/cio/vfio_ccw_private.h | 9 +--
> include/linux/mdev.h | 5 --
> 6 files changed, 125 insertions(+), 125 deletions(-)
>
^ permalink raw reply [flat|nested] 13+ messages in thread