Re: [PATCH 17/18] pc-bios/s390x: Enable multi-device boot loop

[Thomas Huth <thuth@redhat.com>]

On 27/09/2024 02.51, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Allow attempts to boot from multiple IPL devices. If the first device fails to
> IPL, select the pre-built IPLB for the next device in the boot order and attempt
> to IPL from it. Continue this process until IPL is successful or there are no
> devices left to try.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> 
> ---
...
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index d7c457e0ed..e38eedd83a 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -23,7 +23,7 @@ static SubChannelId blk_schid = { .one = 1 };
>   static char loadparm_str[LOADPARM_LEN + 1];
>   QemuIplParameters qipl;
>   IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> -static bool have_iplb;
> +bool have_iplb;
>   static uint16_t cutype;
>   LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
>   
> @@ -55,6 +55,12 @@ void write_iplb_location(void)
>       }
>   }
>   
> +static void copy_qipl(void)
> +{
> +    QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
> +    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
> +}
> +
>   unsigned int get_loadparm_index(void)
>   {
>       return atoi(loadparm_str);
> @@ -152,6 +158,7 @@ static void menu_setup(void)
>   
>       /* If loadparm was set to any other value, then do not enable menu */
>       if (memcmp(loadparm_str, LOADPARM_EMPTY, LOADPARM_LEN) != 0) {
> +        menu_set_parms(qipl.qipl_flags && ~BOOT_MENU_FLAG_MASK, 0);
>           return;
>       }
>   
> @@ -183,7 +190,10 @@ static void css_setup(void)
>   static void boot_setup(void)
>   {
>       char lpmsg[] = "LOADPARM=[________]\n";
> -    have_iplb = store_iplb(&iplb);
> +
> +    if (!have_iplb) {
> +        have_iplb = store_iplb(&iplb);
> +    }
>   
>       if (memcmp(iplb.loadparm, "@@@@@@@@", LOADPARM_LEN) != 0) {
>           ebcdic_to_ascii((char *) iplb.loadparm, loadparm_str, LOADPARM_LEN);
> @@ -191,6 +201,10 @@ static void boot_setup(void)
>           sclp_get_loadparm_ascii(loadparm_str);
>       }
>   
> +    if (have_iplb) {
> +        menu_setup();
> +    }
> +
>       memcpy(lpmsg + 10, loadparm_str, 8);
>       puts(lpmsg);
>   
> @@ -208,6 +222,7 @@ static bool find_boot_device(void)
>   
>       switch (iplb.pbt) {
>       case S390_IPL_TYPE_CCW:
> +        vdev->scsi_device_selected = false;
>           debug_print_int("device no. ", iplb.ccw.devno);
>           blk_schid.ssid = iplb.ccw.ssid & 0x3;
>           debug_print_int("ssid ", blk_schid.ssid);
> @@ -231,15 +246,8 @@ static bool find_boot_device(void)
>   static int virtio_setup(void)
>   {
>       VDev *vdev = virtio_get_device();
> -    QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
>       int ret;
>   
> -    memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
> -
> -    if (have_iplb) {
> -        menu_setup();
> -    }
> -
>       switch (vdev->senseid.cu_model) {
>       case VIRTIO_ID_NET:
>           puts("Network boot device detected");
> @@ -281,41 +289,19 @@ 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 = is_dev_possibly_bootable(-1, sch_no);
> -            if (ret < 0) {
> -                break;
> -            }
> -            if (ret == true) {
> -                ipl_boot_device(); /* Only returns if unsuccessful */
> -                return;
> -            }
> -        }
> -    }
> -
> -    puts("Could not find a suitable boot device (none specified)");
> -}
> -
>   void main(void)
>   {
> +    copy_qipl();
>       sclp_setup();
>       css_setup();
> -    boot_setup();
> -    if (have_iplb && find_boot_device()) {
> -        ipl_boot_device();
> -    } else {
> -        probe_boot_device();
> -    }
> +    do {
> +        boot_setup();
> +        if (have_iplb && find_boot_device()) {
> +            ipl_boot_device();
> +        }
> +        have_iplb = load_next_iplb();
> +    } while (have_iplb);
> +
> +    panic("No suitable device for IPL. Halting...");
>   
> -    panic("Failed to IPL. Halting...");
>   }

Can we please keep the possibility to boot from any device (i.e. the 
probe_boot_device() stuff), in case the user did not specify any boot index 
property at all?

  Thanks,
   Thomas