1. Patchew
  2. QEMU
  3. [PATCH V2 0/18] s390x: Add Full Boot Order Support
  4. View patch

[PATCH 11/18] pc-bios/s390-ccw: Remove panics from Netboot IPL path

jrossi@linux.ibm.com posted 18 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH 11/18] pc-bios/s390-ccw: Remove panics from Netboot IPL path
Posted by jrossi@linux.ibm.com 1 month, 4 weeks ago 
From: Jared Rossi <jrossi@linux.ibm.com>

Remove panic-on-error from Netboot specific functions so that error recovery
may be possible in the future.

Functions that would previously panic now provide a return code.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>

---
 pc-bios/s390-ccw/s390-ccw.h   |  2 +-
 pc-bios/s390-ccw/bootmap.c    |  1 +
 pc-bios/s390-ccw/netmain.c    | 22 +++++++++++++++-------
 pc-bios/s390-ccw/virtio-net.c |  7 +++++--
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index cbd92f3671..8dac070257 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -57,7 +57,7 @@ unsigned int get_loadparm_index(void);
 void main(void);
 
 /* netmain.c */
-void netmain(void);
+int netmain(void);
 
 /* sclp.c */
 void sclp_print(const char *string);
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index dc7200c264..9459e19ffb 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -1059,6 +1059,7 @@ void zipl_load(void)
 
     if (virtio_get_device_type() == VIRTIO_ID_NET) {
         netmain();
+        panic("\n! Cannot IPL from this network !\n");
     }
 
     if (ipl_scsi()) {
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index bc6ad8695f..013f94d932 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -464,7 +464,7 @@ static bool find_net_dev(Schib *schib, int dev_no)
     return false;
 }
 
-static void virtio_setup(void)
+static int virtio_setup(void)
 {
     Schib schib;
     int ssid;
@@ -479,7 +479,10 @@ static void virtio_setup(void)
     enable_mss_facility();
 
     if (store_iplb(&iplb)) {
-        IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
+        if (iplb.pbt != S390_IPL_TYPE_CCW) {
+            puts("IPL_TYPE_CCW expected");
+        }
+
         dev_no = iplb.ccw.devno;
         debug_print_int("device no. ", dev_no);
         net_schid.ssid = iplb.ccw.ssid & 0x3;
@@ -495,10 +498,10 @@ static void virtio_setup(void)
         }
     }
 
-    IPL_assert(found, "No virtio net device found");
+    return found;
 }
 
-void netmain(void)
+int netmain(void)
 {
     filename_ip_t fn_ip;
     int rc, fnlen;
@@ -506,11 +509,15 @@ void netmain(void)
     sclp_setup();
     puts("Network boot starting...");
 
-    virtio_setup();
+    if (!virtio_setup()) {
+        puts("No virtio net device found.");
+        return 1;
+    }
 
     rc = net_init(&fn_ip);
     if (rc) {
-        panic("Network initialization failed. Halting.");
+        puts("Network initialization failed.");
+        return 1;
     }
 
     fnlen = strlen(fn_ip.filename);
@@ -528,5 +535,6 @@ void netmain(void)
         jump_to_low_kernel();
     }
 
-    panic("Failed to load OS from network.");
+    puts("Failed to load OS from network.");
+    return 1;
 }
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index 2fcb0a58c5..f9854a22c3 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -54,8 +54,11 @@ int virtio_net_init(void *mac_addr)
     vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
     virtio_setup_ccw(vdev);
 
-    IPL_assert(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT,
-               "virtio-net device does not support the MAC address feature");
+    if (!(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT)) {
+        puts("virtio-net device does not support the MAC address feature");
+        return -1;
+    }
+
     memcpy(mac_addr, vdev->config.net.mac, ETH_ALEN);
 
     for (i = 0; i < 64; i++) {
-- 
2.45.1
Re: [PATCH 11/18] pc-bios/s390-ccw: Remove panics from Netboot IPL path
Posted by Thomas Huth 1 month, 3 weeks ago 
On 27/09/2024 02.51, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Remove panic-on-error from Netboot specific functions so that error recovery
> may be possible in the future.
> 
> Functions that would previously panic now provide a return code.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> 
> ---
...
> index bc6ad8695f..013f94d932 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
> @@ -464,7 +464,7 @@ static bool find_net_dev(Schib *schib, int dev_no)
>       return false;
>   }
>   
> -static void virtio_setup(void)
> +static int virtio_setup(void)
>   {
>       Schib schib;
>       int ssid;
> @@ -479,7 +479,10 @@ static void virtio_setup(void)
>       enable_mss_facility();
>   
>       if (store_iplb(&iplb)) {
> -        IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
> +        if (iplb.pbt != S390_IPL_TYPE_CCW) {
> +            puts("IPL_TYPE_CCW expected");
> +        }

I think in this case, the IPL_assert() could maybe even stay: If we end up 
here without the correct type in iplb.pbt, there was likely a bug elsewhere 
in the earlier setup code already, or do you see a way we could end up here 
with another type?

Anyway, if you want to change it, shouldn't there be a "return false" after 
the puts() statement?

  Thomas
Re: [PATCH 11/18] pc-bios/s390-ccw: Remove panics from Netboot IPL path
Posted by Jared Rossi 1 month, 3 weeks ago 

On 9/30/24 5:39 AM, Thomas Huth wrote:
> On 27/09/2024 02.51, jrossi@linux.ibm.com wrote:
>> From: Jared Rossi <jrossi@linux.ibm.com>
>>
>> Remove panic-on-error from Netboot specific functions so that error 
>> recovery
>> may be possible in the future.
>>
>> Functions that would previously panic now provide a return code.
>>
>> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
>>
>> ---
> ...
>> index bc6ad8695f..013f94d932 100644
>> --- a/pc-bios/s390-ccw/netmain.c
>> +++ b/pc-bios/s390-ccw/netmain.c
>> @@ -464,7 +464,7 @@ static bool find_net_dev(Schib *schib, int dev_no)
>>       return false;
>>   }
>>   -static void virtio_setup(void)
>> +static int virtio_setup(void)
>>   {
>>       Schib schib;
>>       int ssid;
>> @@ -479,7 +479,10 @@ static void virtio_setup(void)
>>       enable_mss_facility();
>>         if (store_iplb(&iplb)) {
>> -        IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW 
>> expected");
>> +        if (iplb.pbt != S390_IPL_TYPE_CCW) {
>> +            puts("IPL_TYPE_CCW expected");
>> +        }
>
> I think in this case, the IPL_assert() could maybe even stay: If we 
> end up here without the correct type in iplb.pbt, there was likely a 
> bug elsewhere in the earlier setup code already, or do you see a way 
> we could end up here with another type?
>
I agree that the panic can stay in this case. The only way that the PBT 
could
be wrong at this stage is if there were in error earlier when building the
IPLB, so it is appropriate to terminate the entire IPL in that case.

Jared Rossi

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.