| Message ID | 20240927005117.1679506-12-jrossi@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Add Full Boot Order Support | expand |
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
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
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++) {