Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Dong Jia Shi]

* Sebastian Ott  [2017-03-23 12:51:40 +0100]:

> On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > For future code reuse purpose, this decouples the cio code with
> > the ccw device specific parts from ccw_device_cancel_halt_clear,
> > and makes a new common I/O interface named cio_cancel_halt_clear.
> > 
> > Reviewed-by: Pierre Morel 
> > Signed-off-by: Dong Jia Shi 
> > Cc: Sebastian Ott 
> > Cc: Peter Oberparleiter 
> [...]
> > +/**
> > + * cio_cancel_halt_clear - Cancel running I/O by performing cancel, halt
> > + * and clear ordinally if subchannel is valid.
> > + * @sch: subchannel on which to perform the cancel_halt_clear operation
> > + * @iretry: the number of the times remained to retry the next operation
> > + *
> > + * This should be called repeatedly since halt/clear are asynchronous
> > + * operations. We do one try with cio_cancel, two tries with cio_halt,
>  ^
>  three
Ok. Nice catch!

> 
> Acked-by: Sebastian Ott 
Thanks.

-- 
Dong Jia Shi

Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Sebastian Ott]

On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> For future code reuse purpose, this decouples the cio code with
> the ccw device specific parts from ccw_device_cancel_halt_clear,
> and makes a new common I/O interface named cio_cancel_halt_clear.
> 
> Reviewed-by: Pierre Morel 
> Signed-off-by: Dong Jia Shi 
> Cc: Sebastian Ott 
> Cc: Peter Oberparleiter 
[...]
> +/**
> + * cio_cancel_halt_clear - Cancel running I/O by performing cancel, halt
> + * and clear ordinally if subchannel is valid.
> + * @sch: subchannel on which to perform the cancel_halt_clear operation
> + * @iretry: the number of the times remained to retry the next operation
> + *
> + * This should be called repeatedly since halt/clear are asynchronous
> + * operations. We do one try with cio_cancel, two tries with cio_halt,
 ^
 three

Acked-by: Sebastian Ott 

Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Dong Jia Shi]

* Sebastian Ott  [2017-03-17 10:51:20 +0100]:

> On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > * Sebastian Ott  [2017-03-17 10:26:51 +0100]:
> > 
> > > On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > > > For future code reuse purpose, this decouples the cio code with
> > > > the ccw device specific parts from ccw_device_cancel_halt_clear,
> > > > and makes a new common I/O interface named cio_cancel_halt_clear.
> > > 
> > > What would the user of cio_cancel_halt_clear be?
> > Hi Sebastian,
> > 
> > We are trying to introduce a new I/O subchannel driver to enable usage
> > of I/O subchannels via VFIO-CCW, so we can passthru a subchannel to a
> > KVM guest. And that new vfio-ccw subchannel driver is the user. :>
> 
> OK, thanks for the explanation. May I suggest that you put these patches
> in the series that actually introduces the new subchannel driver.
> 
Hi Sebastian,

For your reference, information of the current patch series could be
found by searching/accessing:
Subject: [PATCH v4 00/16] basic vfio-ccw infrastructure
Message-Id: <20170317031743.40128-1-bjsdj...@linux.vnet.ibm.com>
https://www.mail-archive.com/qemu-devel@nongnu.org/msg438029.html

I could put you on Cc, if you would like to recieve updates of the whole
patch series.

> Sebastian
-- 
Dong Jia

Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Sebastian Ott]

On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> * Sebastian Ott  [2017-03-17 10:26:51 +0100]:
> 
> > On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > > For future code reuse purpose, this decouples the cio code with
> > > the ccw device specific parts from ccw_device_cancel_halt_clear,
> > > and makes a new common I/O interface named cio_cancel_halt_clear.
> > 
> > What would the user of cio_cancel_halt_clear be?
> Hi Sebastian,
> 
> We are trying to introduce a new I/O subchannel driver to enable usage
> of I/O subchannels via VFIO-CCW, so we can passthru a subchannel to a
> KVM guest. And that new vfio-ccw subchannel driver is the user. :>

OK, thanks for the explanation. May I suggest that you put these patches
in the series that actually introduces the new subchannel driver.

Sebastian

Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Dong Jia Shi]

* Sebastian Ott  [2017-03-17 10:26:51 +0100]:

> On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > For future code reuse purpose, this decouples the cio code with
> > the ccw device specific parts from ccw_device_cancel_halt_clear,
> > and makes a new common I/O interface named cio_cancel_halt_clear.
> 
> What would the user of cio_cancel_halt_clear be?
Hi Sebastian,

We are trying to introduce a new I/O subchannel driver to enable usage
of I/O subchannels via VFIO-CCW, so we can passthru a subchannel to a
KVM guest. And that new vfio-ccw subchannel driver is the user. :>

> 
> Sebastian

-- 
Dong Jia

Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Sebastian Ott]

On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> For future code reuse purpose, this decouples the cio code with
> the ccw device specific parts from ccw_device_cancel_halt_clear,
> and makes a new common I/O interface named cio_cancel_halt_clear.

What would the user of cio_cancel_halt_clear be?

Sebastian

[Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Dong Jia Shi]

For future code reuse purpose, this decouples the cio code with
the ccw device specific parts from ccw_device_cancel_halt_clear,
and makes a new common I/O interface named cio_cancel_halt_clear.

Reviewed-by: Pierre Morel 
Signed-off-by: Dong Jia Shi 
Cc: Sebastian Ott 
Cc: Peter Oberparleiter 
---
 drivers/s390/cio/cio.c| 59 +++
 drivers/s390/cio/cio.h|  1 +
 drivers/s390/cio/device_fsm.c | 54 ++-
 3 files changed, 68 insertions(+), 46 deletions(-)

diff --git a/drivers/s390/cio/cio.c b/drivers/s390/cio/cio.c
index 1b35066..5711e28 100644
--- a/drivers/s390/cio/cio.c
+++ b/drivers/s390/cio/cio.c
@@ -309,6 +309,65 @@ cio_cancel (struct subchannel *sch)
}
 }
 
+/**
+ * cio_cancel_halt_clear - Cancel running I/O by performing cancel, halt
+ * and clear ordinally if subchannel is valid.
+ * @sch: subchannel on which to perform the cancel_halt_clear operation
+ * @iretry: the number of the times remained to retry the next operation
+ *
+ * This should be called repeatedly since halt/clear are asynchronous
+ * operations. We do one try with cio_cancel, two tries with cio_halt,
+ * 255 tries with cio_clear. The caller should initialize @iretry with
+ * the value 255 for its first call to this, and keep using the same
+ * @iretry in the subsequent calls until it gets a non -EBUSY return.
+ *
+ * Returns 0 if device now idle, -ENODEV for device not operational,
+ * -EBUSY if an interrupt is expected (either from halt/clear or from a
+ * status pending), and -EIO if out of retries.
+ */
+int cio_cancel_halt_clear(struct subchannel *sch, int *iretry)
+{
+   int ret;
+
+   if (cio_update_schib(sch))
+   return -ENODEV;
+   if (!sch->schib.pmcw.ena)
+   /* Not operational -> done. */
+   return 0;
+   /* Stage 1: cancel io. */
+   if (!(scsw_actl(>schib.scsw) & SCSW_ACTL_HALT_PEND) &&
+   !(scsw_actl(>schib.scsw) & SCSW_ACTL_CLEAR_PEND)) {
+   if (!scsw_is_tm(>schib.scsw)) {
+   ret = cio_cancel(sch);
+   if (ret != -EINVAL)
+   return ret;
+   }
+   /*
+* Cancel io unsuccessful or not applicable (transport mode).
+* Continue with asynchronous instructions.
+*/
+   *iretry = 3;/* 3 halt retries. */
+   }
+   /* Stage 2: halt io. */
+   if (!(scsw_actl(>schib.scsw) & SCSW_ACTL_CLEAR_PEND)) {
+   if (*iretry) {
+   *iretry -= 1;
+   ret = cio_halt(sch);
+   if (ret != -EBUSY)
+   return (ret == 0) ? -EBUSY : ret;
+   }
+   /* Halt io unsuccessful. */
+   *iretry = 255;  /* 255 clear retries. */
+   }
+   /* Stage 3: clear io. */
+   if (*iretry) {
+   *iretry -= 1;
+   ret = cio_clear(sch);
+   return (ret == 0) ? -EBUSY : ret;
+   }
+   /* Function was unsuccessful */
+   return -EIO;
+}
 
 static void cio_apply_config(struct subchannel *sch, struct schib *schib)
 {
diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h
index f0e57ae..939596d 100644
--- a/drivers/s390/cio/cio.h
+++ b/drivers/s390/cio/cio.h
@@ -123,6 +123,7 @@ extern int cio_enable_subchannel(struct subchannel *, u32);
 extern int cio_disable_subchannel (struct subchannel *);
 extern int cio_cancel (struct subchannel *);
 extern int cio_clear (struct subchannel *);
+extern int cio_cancel_halt_clear(struct subchannel *, int *);
 extern int cio_resume (struct subchannel *);
 extern int cio_halt (struct subchannel *);
 extern int cio_start (struct subchannel *, struct ccw1 *, __u8);
diff --git a/drivers/s390/cio/device_fsm.c b/drivers/s390/cio/device_fsm.c
index 9afb5ce..12016e3 100644
--- a/drivers/s390/cio/device_fsm.c
+++ b/drivers/s390/cio/device_fsm.c
@@ -124,14 +124,6 @@ ccw_device_set_timeout(struct ccw_device *cdev, int 
expires)
add_timer(>private->timer);
 }
 
-/*
- * Cancel running i/o. This is called repeatedly since halt/clear are
- * asynchronous operations. We do one try with cio_cancel, two tries
- * with cio_halt, 255 tries with cio_clear. If everythings fails panic.
- * Returns 0 if device now idle, -ENODEV for device not operational and
- * -EBUSY if an interrupt is expected (either from halt/clear or from a
- * status pending).
- */
 int
 ccw_device_cancel_halt_clear(struct ccw_device *cdev)
 {
@@ -139,44 +131,14 @@ ccw_device_cancel_halt_clear(struct ccw_device *cdev)
int ret;
 
sch = to_subchannel(cdev->dev.parent);
-   if (cio_update_schib(sch))
-   return -ENODEV; 
-   if (!sch->schib.pmcw.ena)
-   /* Not operational -> done. */
-