diff mbox series

[05/15] s390-bios: Factor finding boot device out of virtio code path

Message ID 1548768562-20007-6-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
Make a new routine find_boot_device to locate the boot device for all
cases. not just virtio.

In one case no boot device is specified and a suitable boot device can not
be auto detected. The error message for this case was specific to virtio
devices. We update this message to remove virtio specific wording.

Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
 pc-bios/s390-ccw/main.c  | 87 ++++++++++++++++++++++++++----------------------
 tests/boot-serial-test.c |  2 +-
 2 files changed, 49 insertions(+), 40 deletions(-)

Comments

Farhan Ali Jan. 31, 2019, 1:44 p.m. UTC | #1
On 01/29/2019 08:29 AM, Jason J. Herne wrote:
> Make a new routine find_boot_device to locate the boot device for all
> cases. not just virtio.
> 
> In one case no boot device is specified and a suitable boot device can not
> be auto detected. The error message for this case was specific to virtio
> devices. We update this message to remove virtio specific wording.
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/main.c  | 87 ++++++++++++++++++++++++++----------------------
>   tests/boot-serial-test.c |  2 +-
>   2 files changed, 49 insertions(+), 40 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 7e3f65e..2457752 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -55,17 +55,18 @@ unsigned int get_loadparm_index(void)
>    * NOTE: The global variable blk_schid is updated to contain the subchannel
>    * information.
>    */
> -static bool find_dev(Schib *schib, int dev_no)
> +static bool find_subch(int dev_no)
>   {
> +    Schib schib;
>       int i, r;
>   
>       for (i = 0; i < 0x10000; i++) {
>           blk_schid.sch_no = i;
> -        r = stsch_err(blk_schid, schib);
> +        r = stsch_err(blk_schid, &schib);
>           if ((r == 3) || (r == -EIO)) {
>               break;
>           }
> -        if (!schib->pmcw.dnv) {
> +        if (!schib.pmcw.dnv) {
>               continue;
>           }
>   
> @@ -77,7 +78,7 @@ static bool find_dev(Schib *schib, int dev_no)
>               continue;
>           }
>   
> -        if ((dev_no < 0) || (schib->pmcw.dev == dev_no)) {
> +        if ((dev_no < 0) || (schib.pmcw.dev == dev_no)) {
>               return true;
>           }
>       }
> @@ -133,56 +134,63 @@ static void boot_setup(void)
>       have_iplb = store_iplb(&iplb);
>   }
>   
> -static void virtio_setup(void)
> +static void find_boot_device(void)
>   {
> -    Schib schib;
> -    int ssid;
> -    bool found = false;
> -    uint16_t dev_no;
>       VDev *vdev = virtio_get_device();
> -    QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
> -
> -    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
> +    int ssid;
> +    bool found;
>   
> -    if (have_iplb) {
> -        switch (iplb.pbt) {
> -        case S390_IPL_TYPE_CCW:
> -            dev_no = iplb.ccw.devno;
> -            debug_print_int("device no. ", dev_no);
> -            blk_schid.ssid = iplb.ccw.ssid & 0x3;
> -            debug_print_int("ssid ", blk_schid.ssid);
> -            found = find_dev(&schib, dev_no);
> -            break;
> -        case S390_IPL_TYPE_QEMU_SCSI:
> -            vdev->scsi_device_selected = true;
> -            vdev->selected_scsi_device.channel = iplb.scsi.channel;
> -            vdev->selected_scsi_device.target = iplb.scsi.target;
> -            vdev->selected_scsi_device.lun = iplb.scsi.lun;
> -            blk_schid.ssid = iplb.scsi.ssid & 0x3;
> -            found = find_dev(&schib, iplb.scsi.devno);
> -            break;
> -        default:
> -            panic("List-directed IPL not supported yet!\n");
> -        }
> -        menu_setup();
> -    } else {
> +    if (!have_iplb) {
>           for (ssid = 0; ssid < 0x3; ssid++) {
>               blk_schid.ssid = ssid;
> -            found = find_dev(&schib, -1);
> +            found = find_subch(-1);
>               if (found) {
> -                break;
> +                return;
>               }
>           }
> +        panic("Could not find a suitable boot device (none specified)\n");
>       }
>   
> -    IPL_assert(found, "No virtio device found");
> +    switch (iplb.pbt) {
> +    case S390_IPL_TYPE_CCW:
> +        debug_print_int("device no. ", iplb.ccw.devno);
> +        blk_schid.ssid = iplb.ccw.ssid & 0x3;
> +        debug_print_int("ssid ", blk_schid.ssid);
> +        found = find_subch(iplb.ccw.devno);
> +        break;
> +    case S390_IPL_TYPE_QEMU_SCSI:
> +        vdev->scsi_device_selected = true;
> +        vdev->selected_scsi_device.channel = iplb.scsi.channel;
> +        vdev->selected_scsi_device.target = iplb.scsi.target;
> +        vdev->selected_scsi_device.lun = iplb.scsi.lun;
> +        blk_schid.ssid = iplb.scsi.ssid & 0x3;
> +        found = find_subch(iplb.scsi.devno);
> +        break;
> +    default:
> +        panic("List-directed IPL not supported yet!\n");
> +    }
> +
> +    if (!found) {
> +        IPL_assert(found, "Boot device not found\n");
> +    }
> +}
> +
> +static void virtio_setup(void)
> +{
> +    VDev *vdev = virtio_get_device();
> +    QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
> +
> +    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
> +
> +    if (have_iplb) {
> +        menu_setup();
> +    }
>   
>       if (virtio_get_device_type() == VIRTIO_ID_NET) {
>           sclp_print("Network boot device detected\n");
>           vdev->netboot_start_addr = qipl.netboot_start_addr;
>       } else {
>           virtio_blk_setup_device(blk_schid);
> -
>           IPL_assert(virtio_ipl_disk_is_valid(), "No valid IPL device detected");
>       }
>   }
> @@ -192,8 +200,9 @@ int main(void)
>       sclp_setup();
>       css_setup();
>       boot_setup();
> -    virtio_setup();
> +    find_boot_device();
>   
> +    virtio_setup();
>       zipl_load(); /* no return */
>   
>       panic("Failed to load OS from hard disk\n");
> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> index 58a48f3..9daf2cb 100644
> --- a/tests/boot-serial-test.c
> +++ b/tests/boot-serial-test.c
> @@ -112,7 +112,7 @@ static testdef_t tests[] = {
>       { "sparc", "SS-4", "", "MB86904" },
>       { "sparc", "SS-600MP", "", "TMS390Z55" },
>       { "sparc64", "sun4u", "", "UltraSPARC" },
> -    { "s390x", "s390-ccw-virtio", "", "virtio device" },
> +    { "s390x", "s390-ccw-virtio", "", "device" },
>       { "m68k", "mcf5208evb", "", "TT", sizeof(kernel_mcf5208), kernel_mcf5208 },
>       { "microblaze", "petalogix-s3adsp1800", "", "TT",
>         sizeof(kernel_pls3adsp1800), kernel_pls3adsp1800 },
> 


Reviewed-by: Farhan Ali <alifm@linux.ibm.com>
Cornelia Huck Feb. 4, 2019, 10:45 a.m. UTC | #2
On Tue, 29 Jan 2019 08:29:12 -0500
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:

> Make a new routine find_boot_device to locate the boot device for all
> cases. not just virtio.

s/cases./cases,/

> 
> In one case no boot device is specified and a suitable boot device can not
> be auto detected. The error message for this case was specific to virtio
> devices. We update this message to remove virtio specific wording.

"The error message for the case where no boot device has been specified
and a suitable boot device cannot be auto detected was specific to
virtio devices. We update..."

> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/main.c  | 87 ++++++++++++++++++++++++++----------------------
>  tests/boot-serial-test.c |  2 +-
>  2 files changed, 49 insertions(+), 40 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 7e3f65e..2457752 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -55,17 +55,18 @@ unsigned int get_loadparm_index(void)
>   * NOTE: The global variable blk_schid is updated to contain the subchannel
>   * information.
>   */
> -static bool find_dev(Schib *schib, int dev_no)
> +static bool find_subch(int dev_no)

I'm wondering why you drop passing in the schib here? But OTOH, the
usage of global variables or not is a bit confused in the bios anyway...

>  {
> +    Schib schib;
>      int i, r;
>  
>      for (i = 0; i < 0x10000; i++) {
>          blk_schid.sch_no = i;
> -        r = stsch_err(blk_schid, schib);
> +        r = stsch_err(blk_schid, &schib);
>          if ((r == 3) || (r == -EIO)) {
>              break;
>          }
> -        if (!schib->pmcw.dnv) {
> +        if (!schib.pmcw.dnv) {
>              continue;
>          }
>  
> @@ -77,7 +78,7 @@ static bool find_dev(Schib *schib, int dev_no)
>              continue;
>          }
>  
> -        if ((dev_no < 0) || (schib->pmcw.dev == dev_no)) {
> +        if ((dev_no < 0) || (schib.pmcw.dev == dev_no)) {
>              return true;
>          }
>      }
> @@ -133,56 +134,63 @@ static void boot_setup(void)
>      have_iplb = store_iplb(&iplb);
>  }
>  
> -static void virtio_setup(void)
> +static void find_boot_device(void)
>  {
> -    Schib schib;
> -    int ssid;
> -    bool found = false;
> -    uint16_t dev_no;
>      VDev *vdev = virtio_get_device();
> -    QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
> -
> -    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
> +    int ssid;
> +    bool found;
>  
> -    if (have_iplb) {
> -        switch (iplb.pbt) {
> -        case S390_IPL_TYPE_CCW:
> -            dev_no = iplb.ccw.devno;
> -            debug_print_int("device no. ", dev_no);
> -            blk_schid.ssid = iplb.ccw.ssid & 0x3;
> -            debug_print_int("ssid ", blk_schid.ssid);
> -            found = find_dev(&schib, dev_no);
> -            break;
> -        case S390_IPL_TYPE_QEMU_SCSI:
> -            vdev->scsi_device_selected = true;
> -            vdev->selected_scsi_device.channel = iplb.scsi.channel;
> -            vdev->selected_scsi_device.target = iplb.scsi.target;
> -            vdev->selected_scsi_device.lun = iplb.scsi.lun;
> -            blk_schid.ssid = iplb.scsi.ssid & 0x3;
> -            found = find_dev(&schib, iplb.scsi.devno);
> -            break;
> -        default:
> -            panic("List-directed IPL not supported yet!\n");
> -        }
> -        menu_setup();
> -    } else {
> +    if (!have_iplb) {
>          for (ssid = 0; ssid < 0x3; ssid++) {
>              blk_schid.ssid = ssid;
> -            found = find_dev(&schib, -1);
> +            found = find_subch(-1);
>              if (found) {
> -                break;
> +                return;
>              }
>          }
> +        panic("Could not find a suitable boot device (none specified)\n");
>      }
>  
> -    IPL_assert(found, "No virtio device found");
> +    switch (iplb.pbt) {
> +    case S390_IPL_TYPE_CCW:
> +        debug_print_int("device no. ", iplb.ccw.devno);
> +        blk_schid.ssid = iplb.ccw.ssid & 0x3;
> +        debug_print_int("ssid ", blk_schid.ssid);
> +        found = find_subch(iplb.ccw.devno);
> +        break;
> +    case S390_IPL_TYPE_QEMU_SCSI:
> +        vdev->scsi_device_selected = true;
> +        vdev->selected_scsi_device.channel = iplb.scsi.channel;
> +        vdev->selected_scsi_device.target = iplb.scsi.target;
> +        vdev->selected_scsi_device.lun = iplb.scsi.lun;
> +        blk_schid.ssid = iplb.scsi.ssid & 0x3;
> +        found = find_subch(iplb.scsi.devno);
> +        break;
> +    default:
> +        panic("List-directed IPL not supported yet!\n");
> +    }
> +
> +    if (!found) {
> +        IPL_assert(found, "Boot device not found\n");

You can simply call IPL_assert(found, ...) here, as it does the check
already.

> +    }
> +}
Jason J. Herne Feb. 11, 2019, 5:57 p.m. UTC | #3
On 2/4/19 5:45 AM, Cornelia Huck wrote:
> On Tue, 29 Jan 2019 08:29:12 -0500
> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> 
>> Make a new routine find_boot_device to locate the boot device for all
>> cases. not just virtio.
> 
> s/cases./cases,/
> 
>>
>> In one case no boot device is specified and a suitable boot device can not
>> be auto detected. The error message for this case was specific to virtio
>> devices. We update this message to remove virtio specific wording.
> 
> "The error message for the case where no boot device has been specified
> and a suitable boot device cannot be auto detected was specific to
> virtio devices. We update..."
> 
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> ---
>>   pc-bios/s390-ccw/main.c  | 87 ++++++++++++++++++++++++++----------------------
>>   tests/boot-serial-test.c |  2 +-
>>   2 files changed, 49 insertions(+), 40 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
>> index 7e3f65e..2457752 100644
>> --- a/pc-bios/s390-ccw/main.c
>> +++ b/pc-bios/s390-ccw/main.c
>> @@ -55,17 +55,18 @@ unsigned int get_loadparm_index(void)
>>    * NOTE: The global variable blk_schid is updated to contain the subchannel
>>    * information.
>>    */
>> -static bool find_dev(Schib *schib, int dev_no)
>> +static bool find_subch(int dev_no)
> 
> I'm wondering why you drop passing in the schib here? But OTOH, the
> usage of global variables or not is a bit confused in the bios anyway...
> 

I dropped it as an argument because the schib was never used outside of find_dev. Seems to 
make sense to make it a local variable in this case.
Cornelia Huck Feb. 12, 2019, 9:32 a.m. UTC | #4
On Mon, 11 Feb 2019 12:57:36 -0500
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:

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

> >> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> >> index 7e3f65e..2457752 100644
> >> --- a/pc-bios/s390-ccw/main.c
> >> +++ b/pc-bios/s390-ccw/main.c
> >> @@ -55,17 +55,18 @@ unsigned int get_loadparm_index(void)
> >>    * NOTE: The global variable blk_schid is updated to contain the subchannel
> >>    * information.
> >>    */
> >> -static bool find_dev(Schib *schib, int dev_no)
> >> +static bool find_subch(int dev_no)  
> > 
> > I'm wondering why you drop passing in the schib here? But OTOH, the
> > usage of global variables or not is a bit confused in the bios anyway...
> >   
> 
> I dropped it as an argument because the schib was never used outside of find_dev. Seems to 
> make sense to make it a local variable in this case.
> 

Fair enough.
diff mbox series

Patch

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 7e3f65e..2457752 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -55,17 +55,18 @@  unsigned int get_loadparm_index(void)
  * NOTE: The global variable blk_schid is updated to contain the subchannel
  * information.
  */
-static bool find_dev(Schib *schib, int dev_no)
+static bool find_subch(int dev_no)
 {
+    Schib schib;
     int i, r;
 
     for (i = 0; i < 0x10000; i++) {
         blk_schid.sch_no = i;
-        r = stsch_err(blk_schid, schib);
+        r = stsch_err(blk_schid, &schib);
         if ((r == 3) || (r == -EIO)) {
             break;
         }
-        if (!schib->pmcw.dnv) {
+        if (!schib.pmcw.dnv) {
             continue;
         }
 
@@ -77,7 +78,7 @@  static bool find_dev(Schib *schib, int dev_no)
             continue;
         }
 
-        if ((dev_no < 0) || (schib->pmcw.dev == dev_no)) {
+        if ((dev_no < 0) || (schib.pmcw.dev == dev_no)) {
             return true;
         }
     }
@@ -133,56 +134,63 @@  static void boot_setup(void)
     have_iplb = store_iplb(&iplb);
 }
 
-static void virtio_setup(void)
+static void find_boot_device(void)
 {
-    Schib schib;
-    int ssid;
-    bool found = false;
-    uint16_t dev_no;
     VDev *vdev = virtio_get_device();
-    QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
-
-    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
+    int ssid;
+    bool found;
 
-    if (have_iplb) {
-        switch (iplb.pbt) {
-        case S390_IPL_TYPE_CCW:
-            dev_no = iplb.ccw.devno;
-            debug_print_int("device no. ", dev_no);
-            blk_schid.ssid = iplb.ccw.ssid & 0x3;
-            debug_print_int("ssid ", blk_schid.ssid);
-            found = find_dev(&schib, dev_no);
-            break;
-        case S390_IPL_TYPE_QEMU_SCSI:
-            vdev->scsi_device_selected = true;
-            vdev->selected_scsi_device.channel = iplb.scsi.channel;
-            vdev->selected_scsi_device.target = iplb.scsi.target;
-            vdev->selected_scsi_device.lun = iplb.scsi.lun;
-            blk_schid.ssid = iplb.scsi.ssid & 0x3;
-            found = find_dev(&schib, iplb.scsi.devno);
-            break;
-        default:
-            panic("List-directed IPL not supported yet!\n");
-        }
-        menu_setup();
-    } else {
+    if (!have_iplb) {
         for (ssid = 0; ssid < 0x3; ssid++) {
             blk_schid.ssid = ssid;
-            found = find_dev(&schib, -1);
+            found = find_subch(-1);
             if (found) {
-                break;
+                return;
             }
         }
+        panic("Could not find a suitable boot device (none specified)\n");
     }
 
-    IPL_assert(found, "No virtio device found");
+    switch (iplb.pbt) {
+    case S390_IPL_TYPE_CCW:
+        debug_print_int("device no. ", iplb.ccw.devno);
+        blk_schid.ssid = iplb.ccw.ssid & 0x3;
+        debug_print_int("ssid ", blk_schid.ssid);
+        found = find_subch(iplb.ccw.devno);
+        break;
+    case S390_IPL_TYPE_QEMU_SCSI:
+        vdev->scsi_device_selected = true;
+        vdev->selected_scsi_device.channel = iplb.scsi.channel;
+        vdev->selected_scsi_device.target = iplb.scsi.target;
+        vdev->selected_scsi_device.lun = iplb.scsi.lun;
+        blk_schid.ssid = iplb.scsi.ssid & 0x3;
+        found = find_subch(iplb.scsi.devno);
+        break;
+    default:
+        panic("List-directed IPL not supported yet!\n");
+    }
+
+    if (!found) {
+        IPL_assert(found, "Boot device not found\n");
+    }
+}
+
+static void virtio_setup(void)
+{
+    VDev *vdev = virtio_get_device();
+    QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
+
+    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
+
+    if (have_iplb) {
+        menu_setup();
+    }
 
     if (virtio_get_device_type() == VIRTIO_ID_NET) {
         sclp_print("Network boot device detected\n");
         vdev->netboot_start_addr = qipl.netboot_start_addr;
     } else {
         virtio_blk_setup_device(blk_schid);
-
         IPL_assert(virtio_ipl_disk_is_valid(), "No valid IPL device detected");
     }
 }
@@ -192,8 +200,9 @@  int main(void)
     sclp_setup();
     css_setup();
     boot_setup();
-    virtio_setup();
+    find_boot_device();
 
+    virtio_setup();
     zipl_load(); /* no return */
 
     panic("Failed to load OS from hard disk\n");
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 58a48f3..9daf2cb 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -112,7 +112,7 @@  static testdef_t tests[] = {
     { "sparc", "SS-4", "", "MB86904" },
     { "sparc", "SS-600MP", "", "TMS390Z55" },
     { "sparc64", "sun4u", "", "UltraSPARC" },
-    { "s390x", "s390-ccw-virtio", "", "virtio device" },
+    { "s390x", "s390-ccw-virtio", "", "device" },
     { "m68k", "mcf5208evb", "", "TT", sizeof(kernel_mcf5208), kernel_mcf5208 },
     { "microblaze", "petalogix-s3adsp1800", "", "TT",
       sizeof(kernel_pls3adsp1800), kernel_pls3adsp1800 },