If no boot device has been specified (via "bootindex=..."), the s390-ccw
bios scans through all devices to find a bootable device. But so far, it
stops at the very first block device (including virtio-scsi controllers
without attached devices) that it finds, no matter whether it is bootable
or not. That leads to some weird situatation where it is e.g. possible
to boot via:

 qemu-system-s390x -hda /path/to/disk.qcow2

but not if there is e.g. a virtio-scsi controller specified before:

 qemu-system-s390x -device virtio-scsi -hda /path/to/disk.qcow2

While using "bootindex=..." is clearly the preferred way of booting
on s390x, we still can make the life for the users at least a little
bit easier if we look at all available devices to find a bootable one.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1846975
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 pc-bios/s390-ccw/main.c | 46 +++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 3cd01cd80f..0af872f9e3 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -182,20 +182,8 @@ static void boot_setup(void)
 static void find_boot_device(void)
 {
     VDev *vdev = virtio_get_device();
-    int ssid;
     bool found;
 
-    if (!have_iplb) {
-        for (ssid = 0; ssid < 0x3; ssid++) {
-            blk_schid.ssid = ssid;
-            found = find_subch(-1);
-            if (found) {
-                return;
-            }
-        }
-        panic("Could not find a suitable boot device (none specified)\n");
-    }
-
     switch (iplb.pbt) {
     case S390_IPL_TYPE_CCW:
         debug_print_int("device no. ", iplb.ccw.devno);
@@ -260,14 +248,42 @@ static void ipl_boot_device(void)
     }
 }
 
+/*
+ * No boot device has been specified, so we have to scan through the
+ * channels to find one.
+ */
+static void probe_boot_device(void)
+{
+    int ssid, sch_no, ret;
+
+    for (ssid = 0; ssid < 0x3; ssid++) {
+        blk_schid.ssid = ssid;
+        for (sch_no = 0; sch_no < 0x10000; sch_no++) {
+            ret = check_sch_no(-1, sch_no);
+            if (ret < 0) {
+                break;
+            }
+            if (ret == true) {
+                ipl_boot_device();      /* Only returns if unsuccessful */
+            }
+        }
+    }
+
+    sclp_print("Could not find a suitable boot device (none specified)\n");
+}
+
 int main(void)
 {
     sclp_setup();
     css_setup();
     boot_setup();
-    find_boot_device();
-    enable_subchannel(blk_schid);
-    ipl_boot_device();
+    if (have_iplb) {
+        find_boot_device();
+        enable_subchannel(blk_schid);
+        ipl_boot_device();
+    } else {
+        probe_boot_device();
+    }
 
     panic("Failed to load OS from hard disk\n");
     return 0; /* make compiler happy */
-- 
2.18.1

On Tue, 28 Jul 2020 20:37:33 +0200
Thomas Huth <thuth@redhat.com> wrote:

> If no boot device has been specified (via "bootindex=..."), the
> s390-ccw bios scans through all devices to find a bootable device.

maybe a better title for the patch is "scan through all devices if no
boot device specified" then, since it seems we will scan all
devices, not just "boot" devices?

> But so far, it stops at the very first block device (including
> virtio-scsi controllers without attached devices) that it finds, no
> matter whether it is bootable or not. That leads to some weird
> situatation where it is e.g. possible to boot via:
> 
>  qemu-system-s390x -hda /path/to/disk.qcow2
> 
> but not if there is e.g. a virtio-scsi controller specified before:
> 
>  qemu-system-s390x -device virtio-scsi -hda /path/to/disk.qcow2
> 
> While using "bootindex=..." is clearly the preferred way of booting
> on s390x, we still can make the life for the users at least a little
> bit easier if we look at all available devices to find a bootable one.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1846975
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  pc-bios/s390-ccw/main.c | 46
> +++++++++++++++++++++++++++-------------- 1 file changed, 31
> insertions(+), 15 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 3cd01cd80f..0af872f9e3 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -182,20 +182,8 @@ static void boot_setup(void)
>  static void find_boot_device(void)
>  {
>      VDev *vdev = virtio_get_device();
> -    int ssid;
>      bool found;
>  
> -    if (!have_iplb) {
> -        for (ssid = 0; ssid < 0x3; ssid++) {
> -            blk_schid.ssid = ssid;
> -            found = find_subch(-1);
> -            if (found) {
> -                return;
> -            }
> -        }
> -        panic("Could not find a suitable boot device (none
> specified)\n");
> -    }
> -
>      switch (iplb.pbt) {
>      case S390_IPL_TYPE_CCW:
>          debug_print_int("device no. ", iplb.ccw.devno);
> @@ -260,14 +248,42 @@ static void ipl_boot_device(void)
>      }
>  }
>  
> +/*
> + * No boot device has been specified, so we have to scan through the
> + * channels to find one.
> + */
> +static void probe_boot_device(void)
> +{
> +    int ssid, sch_no, ret;
> +
> +    for (ssid = 0; ssid < 0x3; ssid++) {
> +        blk_schid.ssid = ssid;
> +        for (sch_no = 0; sch_no < 0x10000; sch_no++) {
> +            ret = check_sch_no(-1, sch_no);
> +            if (ret < 0) {
> +                break;
> +            }
> +            if (ret == true) {
> +                ipl_boot_device();      /* Only returns if
> unsuccessful */
> +            }
> +        }
> +    }
> +
> +    sclp_print("Could not find a suitable boot device (none
> specified)\n"); +}
> +
>  int main(void)
>  {
>      sclp_setup();
>      css_setup();
>      boot_setup();
> -    find_boot_device();
> -    enable_subchannel(blk_schid);
> -    ipl_boot_device();
> +    if (have_iplb) {
> +        find_boot_device();
> +        enable_subchannel(blk_schid);
> +        ipl_boot_device();
> +    } else {
> +        probe_boot_device();
> +    }
>  
>      panic("Failed to load OS from hard disk\n");
>      return 0; /* make compiler happy */

On Tue, 28 Jul 2020 20:37:33 +0200
Thomas Huth <thuth@redhat.com> wrote:

> If no boot device has been specified (via "bootindex=..."), the s390-ccw
> bios scans through all devices to find a bootable device. But so far, it
> stops at the very first block device (including virtio-scsi controllers
> without attached devices) that it finds, no matter whether it is bootable
> or not. That leads to some weird situatation where it is e.g. possible
> to boot via:
> 
>  qemu-system-s390x -hda /path/to/disk.qcow2
> 
> but not if there is e.g. a virtio-scsi controller specified before:
> 
>  qemu-system-s390x -device virtio-scsi -hda /path/to/disk.qcow2
> 
> While using "bootindex=..." is clearly the preferred way of booting
> on s390x, we still can make the life for the users at least a little
> bit easier if we look at all available devices to find a bootable one.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1846975
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  pc-bios/s390-ccw/main.c | 46 +++++++++++++++++++++++++++--------------
>  1 file changed, 31 insertions(+), 15 deletions(-)

(...)

>  int main(void)
>  {
>      sclp_setup();
>      css_setup();
>      boot_setup();
> -    find_boot_device();
> -    enable_subchannel(blk_schid);
> -    ipl_boot_device();
> +    if (have_iplb) {
> +        find_boot_device();
> +        enable_subchannel(blk_schid);
> +        ipl_boot_device();
> +    } else {
> +        probe_boot_device();
> +    }

The one thing that's a bit surprising with the code is that
enable_subchannel() sticking out now. The code looking for a boot
device does that for all subchannels it looks at... but I think
find_boot_device() did that for specified devices already as well, so
it seems redundant?

Anyway, that's something that can be looked at later.

>  
>      panic("Failed to load OS from hard disk\n");
>      return 0; /* make compiler happy */

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

On 05/08/2020 11.36, Cornelia Huck wrote:
> On Tue, 28 Jul 2020 20:37:33 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> If no boot device has been specified (via "bootindex=..."), the s390-ccw
>> bios scans through all devices to find a bootable device. But so far, it
>> stops at the very first block device (including virtio-scsi controllers
>> without attached devices) that it finds, no matter whether it is bootable
>> or not. That leads to some weird situatation where it is e.g. possible
>> to boot via:
>>
>>  qemu-system-s390x -hda /path/to/disk.qcow2
>>
>> but not if there is e.g. a virtio-scsi controller specified before:
>>
>>  qemu-system-s390x -device virtio-scsi -hda /path/to/disk.qcow2
>>
>> While using "bootindex=..." is clearly the preferred way of booting
>> on s390x, we still can make the life for the users at least a little
>> bit easier if we look at all available devices to find a bootable one.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1846975
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  pc-bios/s390-ccw/main.c | 46 +++++++++++++++++++++++++++--------------
>>  1 file changed, 31 insertions(+), 15 deletions(-)
> 
> (...)
> 
>>  int main(void)
>>  {
>>      sclp_setup();
>>      css_setup();
>>      boot_setup();
>> -    find_boot_device();
>> -    enable_subchannel(blk_schid);
>> -    ipl_boot_device();
>> +    if (have_iplb) {
>> +        find_boot_device();
>> +        enable_subchannel(blk_schid);
>> +        ipl_boot_device();
>> +    } else {
>> +        probe_boot_device();
>> +    }
> 
> The one thing that's a bit surprising with the code is that
> enable_subchannel() sticking out now. The code looking for a boot
> device does that for all subchannels it looks at... but I think
> find_boot_device() did that for specified devices already as well, so
> it seems redundant?
> 
> Anyway, that's something that can be looked at later.

Yes, I noticed that, too ... but yes, one clean-up step at a time. I've
put it on my todo-list for later.

>>  
>>      panic("Failed to load OS from hard disk\n");
>>      return 0; /* make compiler happy */
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Thanks!

 Thomas