My questions to this patch from the original RFC series are still
outstanding. :(
https://marc.info/?l=linux-s390&m=154223063716128&w=2
On 11/28/2018 07:41 AM, Pierre Morel wrote:
The subchannel enablement and the according setting to the
VFIO_CCW_STATE_STANDBY state should only be done when all
parts of the VFIO mediated device have been initialized
i.e. after the mediated device has been successfully opened.
Let's stay in VFIO_CCW_STATE_NOT_OPER until the mediated
device has been opened and set the VFIO_CCW_STATE_STANDBY
on a successful open.
On release the state is set back to VFIO_CCW_STATE_NOT_OPER
by vfio_ccw_sch_quiesce().
When the mediated device is closed, disable the sub channel
by calling vfio_ccw_sch_quiesce().
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
drivers/s390/cio/vfio_ccw_async.c | 11 +++++++++++
Ah, this series is built on Connie's async changes. Okay. [1]
drivers/s390/cio/vfio_ccw_drv.c | 10 +---------
drivers/s390/cio/vfio_ccw_ops.c | 27 +++++++++++++++++++++------
drivers/s390/cio/vfio_ccw_private.h | 1 +
4 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/drivers/s390/cio/vfio_ccw_async.c b/drivers/s390/cio/vfio_ccw_async.c
index 8c7f51d..033574e 100644
--- a/drivers/s390/cio/vfio_ccw_async.c
+++ b/drivers/s390/cio/vfio_ccw_async.c
@@ -76,6 +76,17 @@ const struct vfio_ccw_regops vfio_ccw_async_region_ops = {
.release = vfio_ccw_async_region_release,
};
+void vfio_ccw_unregister_async_dev_regions(struct vfio_ccw_private *private)
+{
+ int i;
+
+ for (i = 0; i < private->num_regions; i++)
+ private->region[i].ops->release(private, &private->region[i]);
+ private->num_regions = 0;
+ kfree(private->region);
+ private->region = NULL;
+} > +
int vfio_ccw_register_async_dev_regions(struct vfio_ccw_private *private)
{
return vfio_ccw_register_dev_region(private,
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index bce7fed..d3527b6 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -133,26 +133,18 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
private->sch = sch;
dev_set_drvdata(&sch->dev, private);
- spin_lock_irq(sch->lock);
private->state = VFIO_CCW_STATE_NOT_OPER;
sch->isc = VFIO_CCW_ISC;
- ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch);
- spin_unlock_irq(sch->lock);
- if (ret)
- goto out_free;
INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo);
atomic_set(&private->avail, 1);
- private->state = VFIO_CCW_STATE_STANDBY;
ret = vfio_ccw_mdev_reg(sch);
if (ret)
- goto out_disable;
+ goto out_free;
return 0;
-out_disable:
- cio_disable_subchannel(sch);
out_free:
dev_set_drvdata(&sch->dev, NULL);
if (private->cmd_region)
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 0e1f7f7b..cc7d46a 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -113,14 +113,11 @@ static int vfio_ccw_mdev_create(struct kobject *kobj, struct mdev_device *mdev)
struct vfio_ccw_private *private =
dev_get_drvdata(mdev_parent_dev(mdev));
- if (private->state == VFIO_CCW_STATE_NOT_OPER)
- return -ENODEV;
-
if (atomic_dec_if_positive(&private->avail) < 0)
return -EPERM;
private->mdev = mdev;
- private->state = VFIO_CCW_STATE_IDLE;
+ private->state = VFIO_CCW_STATE_NOT_OPER;
return 0;
}
@@ -148,6 +145,7 @@ static int vfio_ccw_mdev_open(struct mdev_device *mdev)
struct vfio_ccw_private *private =
dev_get_drvdata(mdev_parent_dev(mdev));
unsigned long events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
+ struct subchannel *sch = private->sch;
int ret;
private->nb.notifier_call = vfio_ccw_mdev_notifier;
@@ -159,8 +157,24 @@ static int vfio_ccw_mdev_open(struct mdev_device *mdev)
ret = vfio_ccw_register_async_dev_regions(private);
if (ret)
- vfio_unregister_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY,
- &private->nb);
+ goto err_regions;
+
+ spin_lock_irq(private->sch->lock);
+ ret = cio_enable_subchannel(sch, (u32)(unsigned long)sch);
+ if (ret)
+ goto err_enable;
+
+ private->state = VFIO_CCW_STATE_STANDBY;
+ spin_unlock_irq(sch->lock);
+ return 0;
+
+err_enable:
+ spin_unlock_irq(sch->lock);
+ vfio_ccw_unregister_async_dev_regions(private);
+
+err_regions:
+ vfio_unregister_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY,
+ &private->nb);
return ret;
}
@@ -170,6 +184,7 @@ static void vfio_ccw_mdev_release(struct mdev_device *mdev)
dev_get_drvdata(mdev_parent_dev(mdev));
int i;
+ vfio_ccw_sch_quiesce(private->sch);
vfio_unregister_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY,
&private->nb);
[1] If Connie's async patches go in first, then the stuff in your
"vfio_ccw_unregister_async_dev_regions" is also added here. That could
be removed and replaced with a call to your new function, yes?
diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
index 1a41a14..5d799b7 100644
--- a/drivers/s390/cio/vfio_ccw_private.h
+++ b/drivers/s390/cio/vfio_ccw_private.h
@@ -54,6 +54,7 @@ int vfio_ccw_register_dev_region(struct vfio_ccw_private *private,
size_t size, u32 flags, void *data);
int vfio_ccw_register_async_dev_regions(struct vfio_ccw_private *private);
+void vfio_ccw_unregister_async_dev_regions(struct vfio_ccw_private *private);
/**
* struct vfio_ccw_private
