diff mbox series

[07/15] s390-bios: Decouple channel i/o logic from virtio

Message ID 1548768562-20007-8-git-send-email-jjherne@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390: vfio-ccw dasd ipl support | expand

Commit Message

Jason J. Herne Jan. 29, 2019, 1:29 p.m. UTC
Create a separate library for channel i/o related code. This decouples
channel i/o operations from virtio and allows us to make use of them for
the real dasd boot path.

Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
 pc-bios/s390-ccw/Makefile        |  2 +-
 pc-bios/s390-ccw/cio.c           | 41 ++++++++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/cio.h           |  3 +++
 pc-bios/s390-ccw/main.c          |  1 +
 pc-bios/s390-ccw/netboot.mak     |  2 +-
 pc-bios/s390-ccw/netmain.c       |  1 +
 pc-bios/s390-ccw/s390-ccw.h      |  1 -
 pc-bios/s390-ccw/virtio-blkdev.c |  1 +
 pc-bios/s390-ccw/virtio.c        | 27 ++------------------------
 9 files changed, 51 insertions(+), 28 deletions(-)
 create mode 100644 pc-bios/s390-ccw/cio.c

Comments

Farhan Ali Jan. 31, 2019, 2:38 p.m. UTC | #1
On 01/29/2019 08:29 AM, Jason J. Herne wrote:
> Create a separate library for channel i/o related code. This decouples
> channel i/o operations from virtio and allows us to make use of them for
> the real dasd boot path.
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/Makefile        |  2 +-
>   pc-bios/s390-ccw/cio.c           | 41 ++++++++++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/cio.h           |  3 +++
>   pc-bios/s390-ccw/main.c          |  1 +
>   pc-bios/s390-ccw/netboot.mak     |  2 +-
>   pc-bios/s390-ccw/netmain.c       |  1 +
>   pc-bios/s390-ccw/s390-ccw.h      |  1 -
>   pc-bios/s390-ccw/virtio-blkdev.c |  1 +
>   pc-bios/s390-ccw/virtio.c        | 27 ++------------------------
>   9 files changed, 51 insertions(+), 28 deletions(-)
>   create mode 100644 pc-bios/s390-ccw/cio.c
> 
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index 1eb316b..12ad9c1 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -10,7 +10,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
>   .PHONY : all clean build-all
>   
>   OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o \
> -	  virtio.o virtio-scsi.o virtio-blkdev.o libc.o
> +	  virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o
>   
>   QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
>   QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
> new file mode 100644
> index 0000000..095f79b
> --- /dev/null
> +++ b/pc-bios/s390-ccw/cio.c
> @@ -0,0 +1,41 @@
> +/*
> + * S390 Channel I/O
> + *
> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +

shouldn't the year be 2019 now? :)
Jason J. Herne Jan. 31, 2019, 2:45 p.m. UTC | #2
On 1/31/19 9:38 AM, Farhan Ali wrote:
> 
> 
> On 01/29/2019 08:29 AM, Jason J. Herne wrote:
>> Create a separate library for channel i/o related code. This decouples
>> channel i/o operations from virtio and allows us to make use of them for
>> the real dasd boot path.
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> ---
>>   pc-bios/s390-ccw/Makefile        |  2 +-
>>   pc-bios/s390-ccw/cio.c           | 41 ++++++++++++++++++++++++++++++++++++++++
>>   pc-bios/s390-ccw/cio.h           |  3 +++
>>   pc-bios/s390-ccw/main.c          |  1 +
>>   pc-bios/s390-ccw/netboot.mak     |  2 +-
>>   pc-bios/s390-ccw/netmain.c       |  1 +
>>   pc-bios/s390-ccw/s390-ccw.h      |  1 -
>>   pc-bios/s390-ccw/virtio-blkdev.c |  1 +
>>   pc-bios/s390-ccw/virtio.c        | 27 ++------------------------
>>   9 files changed, 51 insertions(+), 28 deletions(-)
>>   create mode 100644 pc-bios/s390-ccw/cio.c
>>
>> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
>> index 1eb316b..12ad9c1 100644
>> --- a/pc-bios/s390-ccw/Makefile
>> +++ b/pc-bios/s390-ccw/Makefile
>> @@ -10,7 +10,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
>>   .PHONY : all clean build-all
>>   OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o \
>> -      virtio.o virtio-scsi.o virtio-blkdev.o libc.o
>> +      virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o
>>   QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
>>   QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
>> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
>> new file mode 100644
>> index 0000000..095f79b
>> --- /dev/null
>> +++ b/pc-bios/s390-ccw/cio.c
>> @@ -0,0 +1,41 @@
>> +/*
>> + * S390 Channel I/O
>> + *
>> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
>> + * your option) any later version. See the COPYING file in the top-level
>> + * directory.
>> + */
>> +
> 
> shouldn't the year be 2019 now? :)

I suppose. I did write it in 2018 :-P I'll update it.
Cornelia Huck Feb. 4, 2019, 10:57 a.m. UTC | #3
On Tue, 29 Jan 2019 08:29:14 -0500
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:

> Create a separate library for channel i/o related code. This decouples
> channel i/o operations from virtio and allows us to make use of them for
> the real dasd boot path.
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/Makefile        |  2 +-
>  pc-bios/s390-ccw/cio.c           | 41 ++++++++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/cio.h           |  3 +++
>  pc-bios/s390-ccw/main.c          |  1 +
>  pc-bios/s390-ccw/netboot.mak     |  2 +-
>  pc-bios/s390-ccw/netmain.c       |  1 +
>  pc-bios/s390-ccw/s390-ccw.h      |  1 -
>  pc-bios/s390-ccw/virtio-blkdev.c |  1 +
>  pc-bios/s390-ccw/virtio.c        | 27 ++------------------------
>  9 files changed, 51 insertions(+), 28 deletions(-)
>  create mode 100644 pc-bios/s390-ccw/cio.c
> 

> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
> new file mode 100644
> index 0000000..095f79b
> --- /dev/null
> +++ b/pc-bios/s390-ccw/cio.c
> @@ -0,0 +1,41 @@
> +/*
> + * S390 Channel I/O
> + *
> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */

Not sure that copyright header is correct. You moved some code that
probably should be copyright IBM (although that was never added to the
header in the first place...) Also not sure if Alex has some copyrights
on the code you moved, or if that is only trivial stuff. (Don't want to
be difficult, but we should try to get this right.)

> +
> +#include "libc.h"
> +#include "s390-ccw.h"
> +#include "cio.h"
> +
> +static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
> +
> +int enable_mss_facility(void)
> +{
> +    int ret;
> +    ChscAreaSda *sda_area = (ChscAreaSda *) chsc_page;
> +
> +    memset(sda_area, 0, PAGE_SIZE);
> +    sda_area->request.length = 0x0400;
> +    sda_area->request.code = 0x0031;
> +    sda_area->operation_code = 0x2;
> +
> +    ret = chsc(sda_area);
> +    if ((ret == 0) && (sda_area->response.code == 0x0001)) {
> +        return 0;
> +    }
> +    return -EIO;
> +}
> +
> +void enable_subchannel(SubChannelId schid)
> +{
> +    Schib schib;
> +
> +    stsch_err(schid, &schib);
> +    schib.pmcw.ena = 1;
> +    msch(schid, &schib);
> +}

(...)

> diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
> index 11c5626..d2e7fcd 100644
> --- a/pc-bios/s390-ccw/virtio-blkdev.c
> +++ b/pc-bios/s390-ccw/virtio-blkdev.c
> @@ -10,6 +10,7 @@
>  
>  #include "libc.h"
>  #include "s390-ccw.h"
> +#include "cio.h"

Not sure why you need to add this here?

>  #include "virtio.h"
>  #include "virtio-scsi.h"
>  

Otherwise, looks good.
Jason J. Herne Feb. 13, 2019, 2:40 p.m. UTC | #4
On 2/4/19 5:57 AM, Cornelia Huck wrote:
> On Tue, 29 Jan 2019 08:29:14 -0500
> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> 
>> Create a separate library for channel i/o related code. This decouples
>> channel i/o operations from virtio and allows us to make use of them for
>> the real dasd boot path.
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> ---
>>   pc-bios/s390-ccw/Makefile        |  2 +-
>>   pc-bios/s390-ccw/cio.c           | 41 ++++++++++++++++++++++++++++++++++++++++
>>   pc-bios/s390-ccw/cio.h           |  3 +++
>>   pc-bios/s390-ccw/main.c          |  1 +
>>   pc-bios/s390-ccw/netboot.mak     |  2 +-
>>   pc-bios/s390-ccw/netmain.c       |  1 +
>>   pc-bios/s390-ccw/s390-ccw.h      |  1 -
>>   pc-bios/s390-ccw/virtio-blkdev.c |  1 +
>>   pc-bios/s390-ccw/virtio.c        | 27 ++------------------------
>>   9 files changed, 51 insertions(+), 28 deletions(-)
>>   create mode 100644 pc-bios/s390-ccw/cio.c
>>
> 
>> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
>> new file mode 100644
>> index 0000000..095f79b
>> --- /dev/null
>> +++ b/pc-bios/s390-ccw/cio.c
>> @@ -0,0 +1,41 @@
>> +/*
>> + * S390 Channel I/O
>> + *
>> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
>> + * your option) any later version. See the COPYING file in the top-level
>> + * directory.
>> + */
> 
> Not sure that copyright header is correct. You moved some code that
> probably should be copyright IBM (although that was never added to the
> header in the first place...) Also not sure if Alex has some copyrights
> on the code you moved, or if that is only trivial stuff. (Don't want to
> be difficult, but we should try to get this right.)
> 
>> +
>> +#include "libc.h"
>> +#include "s390-ccw.h"
>> +#include "cio.h"
>> +
>> +static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
>> +
>> +int enable_mss_facility(void)
>> +{
>> +    int ret;
>> +    ChscAreaSda *sda_area = (ChscAreaSda *) chsc_page;
>> +
>> +    memset(sda_area, 0, PAGE_SIZE);
>> +    sda_area->request.length = 0x0400;
>> +    sda_area->request.code = 0x0031;
>> +    sda_area->operation_code = 0x2;
>> +
>> +    ret = chsc(sda_area);
>> +    if ((ret == 0) && (sda_area->response.code == 0x0001)) {
>> +        return 0;
>> +    }
>> +    return -EIO;
>> +}
>> +
>> +void enable_subchannel(SubChannelId schid)
>> +{
>> +    Schib schib;
>> +
>> +    stsch_err(schid, &schib);
>> +    schib.pmcw.ena = 1;
>> +    msch(schid, &schib);
>> +}
> 
> (...)
> 
>> diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
>> index 11c5626..d2e7fcd 100644
>> --- a/pc-bios/s390-ccw/virtio-blkdev.c
>> +++ b/pc-bios/s390-ccw/virtio-blkdev.c
>> @@ -10,6 +10,7 @@
>>   
>>   #include "libc.h"
>>   #include "s390-ccw.h"
>> +#include "cio.h"
> 
> Not sure why you need to add this here?

Hmmm... I removed it and the code compiles cleanly... I was *sure* I needed this a while 
go. Perhaps something changed? --
-- Jason J. Herne (jjherne@linux.ibm.com)
diff mbox series

Patch

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 1eb316b..12ad9c1 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -10,7 +10,7 @@  $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
 .PHONY : all clean build-all
 
 OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o \
-	  virtio.o virtio-scsi.o virtio-blkdev.o libc.o
+	  virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o
 
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
 QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
new file mode 100644
index 0000000..095f79b
--- /dev/null
+++ b/pc-bios/s390-ccw/cio.c
@@ -0,0 +1,41 @@ 
+/*
+ * S390 Channel I/O
+ *
+ * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+#include "cio.h"
+
+static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
+
+int enable_mss_facility(void)
+{
+    int ret;
+    ChscAreaSda *sda_area = (ChscAreaSda *) chsc_page;
+
+    memset(sda_area, 0, PAGE_SIZE);
+    sda_area->request.length = 0x0400;
+    sda_area->request.code = 0x0031;
+    sda_area->operation_code = 0x2;
+
+    ret = chsc(sda_area);
+    if ((ret == 0) && (sda_area->response.code == 0x0001)) {
+        return 0;
+    }
+    return -EIO;
+}
+
+void enable_subchannel(SubChannelId schid)
+{
+    Schib schib;
+
+    stsch_err(schid, &schib);
+    schib.pmcw.ena = 1;
+    msch(schid, &schib);
+}
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index a48eee5..7b07d75 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -213,6 +213,9 @@  typedef struct irb {
     __u32 emw[8];
 }  __attribute__ ((packed, aligned(4))) Irb;
 
+int enable_mss_facility(void);
+void enable_subchannel(SubChannelId schid);
+
 /*
  * Some S390 specific IO instructions as inline
  */
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 2457752..1bc61b5 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -10,6 +10,7 @@ 
 
 #include "libc.h"
 #include "s390-ccw.h"
+#include "cio.h"
 #include "virtio.h"
 
 char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
index 14e96b2..5eefb7c 100644
--- a/pc-bios/s390-ccw/netboot.mak
+++ b/pc-bios/s390-ccw/netboot.mak
@@ -1,7 +1,7 @@ 
 
 SLOF_DIR := $(SRC_PATH)/roms/SLOF
 
-NETOBJS := start.o sclp.o virtio.o virtio-net.o jump2ipl.o netmain.o \
+NETOBJS := start.o sclp.o cio.o virtio.o virtio-net.o jump2ipl.o netmain.o \
 	   libnet.a libc.a
 
 LIBC_INC := -nostdinc -I$(SLOF_DIR)/lib/libc/include
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index 0392131..5189c0f 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -33,6 +33,7 @@ 
 #include <pxelinux.h>
 
 #include "s390-ccw.h"
+#include "cio.h"
 #include "virtio.h"
 
 #define DEFAULT_BOOT_RETRIES 10
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 241c6d0..b39ee5d 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -72,7 +72,6 @@  unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
 bool virtio_is_supported(SubChannelId schid);
 void virtio_blk_setup_device(SubChannelId schid);
 int virtio_read(ulong sector, void *load_addr);
-int enable_mss_facility(void);
 u64 get_clock(void);
 ulong get_second(void);
 
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 11c5626..d2e7fcd 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -10,6 +10,7 @@ 
 
 #include "libc.h"
 #include "s390-ccw.h"
+#include "cio.h"
 #include "virtio.h"
 #include "virtio-scsi.h"
 
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index cdb66f4..aa9da72 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -10,6 +10,7 @@ 
 
 #include "libc.h"
 #include "s390-ccw.h"
+#include "cio.h"
 #include "virtio.h"
 #include "virtio-scsi.h"
 #include "bswap.h"
@@ -20,8 +21,6 @@  static VRing block[VIRTIO_MAX_VQS];
 static char ring_area[VIRTIO_RING_SIZE * VIRTIO_MAX_VQS]
                      __attribute__((__aligned__(PAGE_SIZE)));
 
-static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
-
 static VDev vdev = {
     .nr_vqs = 1,
     .vrings = block,
@@ -94,14 +93,9 @@  static int run_ccw(VDev *vdev, int cmd, void *ptr, int len)
 {
     Ccw1 ccw = {};
     CmdOrb orb = {};
-    Schib schib;
     int r;
 
-    /* start command processing */
-    stsch_err(vdev->schid, &schib);
-    /* enable the subchannel for IPL device */
-    schib.pmcw.ena = 1;
-    msch(vdev->schid, &schib);
+    enable_subchannel(vdev->schid);
 
     /* start subchannel command */
     orb.fmt = 1;
@@ -343,20 +337,3 @@  bool virtio_is_supported(SubChannelId schid)
     }
     return false;
 }
-
-int enable_mss_facility(void)
-{
-    int ret;
-    ChscAreaSda *sda_area = (ChscAreaSda *) chsc_page;
-
-    memset(sda_area, 0, PAGE_SIZE);
-    sda_area->request.length = 0x0400;
-    sda_area->request.code = 0x0031;
-    sda_area->operation_code = 0x2;
-
-    ret = chsc(sda_area);
-    if ((ret == 0) && (sda_area->response.code == 0x0001)) {
-        return 0;
-    }
-    return -EIO;
-}