On 2/4/19 6:44 AM, Cornelia Huck wrote:
On Tue, 29 Jan 2019 08:29:19 -0500
"Jason J. Herne" <jjhe...@linux.ibm.com> wrote:

Now that we have a Channel I/O library let's modify virtio boot code to
make use of it for running channel programs.

Signed-off-by: Jason J. Herne <jjhe...@linux.ibm.com>
---
  pc-bios/s390-ccw/virtio.c | 48 +++++++++++++++++++----------------------------
  1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index aa9da72..f8d71ed 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -89,33 +89,20 @@ int drain_irqs(SubChannelId schid)
      }
  }
-static int run_ccw(VDev *vdev, int cmd, void *ptr, int len)
+static int run_ccw(VDev *vdev, int cmd, void *ptr, int len, bool sli)
  {
      Ccw1 ccw = {};
-    CmdOrb orb = {};
-    int r;
-
-    enable_subchannel(vdev->schid);
-
-    /* start subchannel command */
-    orb.fmt = 1;
-    orb.cpa = (u32)(long)&ccw;
-    orb.lpm = 0x80;
ccw.cmd_code = cmd;
      ccw.cda = (long)ptr;
      ccw.count = len;
- r = ssch(vdev->schid, &orb);
-    /*
-     * XXX Wait until device is done processing the CCW. For now we can
-     *     assume that a simple tsch will have finished the CCW processing,
-     *     but the architecture allows for asynchronous operation
-     */
-    if (!r) {
-        r = drain_irqs(vdev->schid);
+    if (sli) {
+        ccw.flags |= CCW_FLAG_SLI;
      }
-    return r;
+
+    enable_subchannel(vdev->schid);
+    return do_cio(vdev->schid, ptr2u32(&ccw), CCW_FMT1);

That still has the very odd pattern that you enable the subchannel
every time you run a channel program...


I originally agreed and removed this. But now that I've gotten around to doing some testing I've discovered that we actually rely on this for one important code path.

Here is the call chain before my patches:
main -> virtio_setup -> find_dev -> virtio_is_supported -> run_ccw

Here is the call chain after my patches:
main -> find_boot_device -> find_subch -> virtio_is_supported -> run_ccw

We end up in the same situation in both scenarios. If we remove the enable_subchannel() call from run_ccw() then we end up in run_ccw() for a disabled subchannel.

That said, I can remove the enable_subchannel call from run_ccw and instead add it to find_subch() if that seems cleaner to you.

Thoughts?

--
-- Jason J. Herne (jjhe...@linux.ibm.com)


Reply via email to