| Message ID | 1517864246-11101-7-git-send-email-walling@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05.02.2018 21:57, Collin L. Walling wrote: > Set boot menu options for an s390 guest and store them in > the iplb. These options are set via the QEMU command line > option: > > -boot menu=on|off[,splash-time=X] > > or via the libvirt domain xml: > > <os> > <bootmenu enable='yes|no' timeout='X'/> > </os> > > Where X represents some positive integer representing > milliseconds. > > Any value set for loadparm will override all boot menu options. > If loadparm=PROMPT, then the menu will be enabled without a > timeout. > > The absence of any boot options on the command line will flag > to later use the zipl boot loader values. > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > Reviewed-by: Thomas Huth <thuth@redhat.com> > --- > hw/s390x/ipl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ > hw/s390x/ipl.h | 7 ++++++- > pc-bios/s390-ccw/iplb.h | 4 +++- > 3 files changed, 61 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 3e3c3b8..227dccb 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -23,6 +23,8 @@ > #include "hw/s390x/ebcdic.h" > #include "ipl.h" > #include "qemu/error-report.h" > +#include "qemu/config-file.h" > +#include "qemu/cutils.h" > > #define KERN_IMAGE_START 0x010000UL > #define KERN_PARM_AREA 0x010480UL > @@ -219,6 +221,53 @@ static Property s390_ipl_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > +static void s390_ipl_set_boot_menu(IplParameterBlock *iplb) > +{ > + QemuOptsList *plist = qemu_find_opts("boot-opts"); > + QemuOpts *opts = QTAILQ_FIRST(&plist->head); > + uint8_t *flags; > + uint16_t *timeout; > + const char *tmp; > + unsigned long result = 0; > + > + switch (iplb->pbt) { > + case S390_IPL_TYPE_CCW: > + case S390_IPL_TYPE_QEMU_SCSI: > + flags = &iplb->qipl.boot_menu_flags; > + timeout = &iplb->qipl.boot_menu_timeout; > + break; > + default: > + error_report("boot menu is not supported for this device type."); > + return; > + } > + > + /* In the absence of -boot menu, use zipl parameters */ > + if (!qemu_opt_get(opts, "menu")) { > + *flags = BOOT_MENU_FLAG_ZIPL_OPTS; > + } else if (boot_menu) { > + *flags = BOOT_MENU_FLAG_CMD_OPTS; > + > + tmp = qemu_opt_get(opts, "splash-time"); > + > + if (tmp && qemu_strtoul(tmp, NULL, 10, &result)) { > + error_report("splash-time value is invalid, forcing it to 0."); > + *timeout = 0; > + return; > + } > + > + result = (result + 500) / 1000; /* Round and convert to seconds */ > + > + if (result > 0xffff) { > + error_report("splash-time value is greater than 65535000," > + " forcing it to 65535000."); > + *timeout = 0xffff; > + return; > + } > + > + *timeout = cpu_to_be16(result); > + } > +} > + > static bool s390_gen_initial_iplb(S390IPLState *ipl) > { > DeviceState *dev_st; > @@ -273,6 +322,9 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl) > if (!s390_ipl_set_loadparm(ipl->iplb.loadparm)) { > ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; > } > + > + s390_ipl_set_boot_menu(&ipl->iplb);> + Maybe it would be safer wrt migration to move that to s390_ipl_prepare_cpu()? > return true; > } > [...]
I'm beginning to like the usage of splash-time to represent a timeout for the boot menu less and less. It is really meant for how long a _splash_ _image_ should appear during boot. I'd like to suggest adding a new boot option "menu-timeout". An alternative would be documenting in qemu-options.hx that s390 treats "splash-time" as the menu-timeout. Thoughts? On 02/05/2018 03:57 PM, Collin L. Walling wrote: > Set boot menu options for an s390 guest and store them in > the iplb. These options are set via the QEMU command line > option: > > -boot menu=on|off[,splash-time=X] > > or via the libvirt domain xml: > > <os> > <bootmenu enable='yes|no' timeout='X'/> > </os> > > Where X represents some positive integer representing > milliseconds. > > Any value set for loadparm will override all boot menu options. > If loadparm=PROMPT, then the menu will be enabled without a > timeout. > > The absence of any boot options on the command line will flag > to later use the zipl boot loader values. > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > Reviewed-by: Thomas Huth <thuth@redhat.com> > --- > [...] > > +static void s390_ipl_set_boot_menu(IplParameterBlock *iplb) > +{ > + QemuOptsList *plist = qemu_find_opts("boot-opts"); > + QemuOpts *opts = QTAILQ_FIRST(&plist->head); > + uint8_t *flags; > + uint16_t *timeout; > + const char *tmp; > + unsigned long result = 0; > + > + switch (iplb->pbt) { > + case S390_IPL_TYPE_CCW: > + case S390_IPL_TYPE_QEMU_SCSI: > + flags = &iplb->qipl.boot_menu_flags; > + timeout = &iplb->qipl.boot_menu_timeout; > + break; > + default: > + error_report("boot menu is not supported for this device type."); > + return; > + } > + > + /* In the absence of -boot menu, use zipl parameters */ > + if (!qemu_opt_get(opts, "menu")) { > + *flags = BOOT_MENU_FLAG_ZIPL_OPTS; > + } else if (boot_menu) { > + *flags = BOOT_MENU_FLAG_CMD_OPTS; > + > + tmp = qemu_opt_get(opts, "splash-time"); > + > + if (tmp && qemu_strtoul(tmp, NULL, 10, &result)) { > + error_report("splash-time value is invalid, forcing it to 0."); > + *timeout = 0; > + return; > + } > + > + result = (result + 500) / 1000; /* Round and convert to seconds */ > + > + if (result > 0xffff) { > + error_report("splash-time value is greater than 65535000," > + " forcing it to 65535000."); > + *timeout = 0xffff; > + return; > + } > + > + *timeout = cpu_to_be16(result); > + } > +} > + > [...]
On 14.02.2018 18:46, Collin L. Walling wrote: > I'm beginning to like the usage of splash-time to represent a timeout > for the boot menu > less and less. It is really meant for how long a _splash_ _image_ > should appear during boot. > > I'd like to suggest adding a new boot option "menu-timeout". An > alternative would be > documenting in qemu-options.hx that s390 treats "splash-time" as the > menu-timeout. > > Thoughts? I think you should keep splash-time and not introduce a new option. Libvirt seems to map the timeout from <bootmenu enable='yes' timeout='X'/> to the splash-time option, and according to the libvirt documentation: "Additional attribute timeout takes the number of milliseconds the boot menu should wait until it times out." So it seems like splash-time is already expected to define the amount of time for the boot menu. We should not confuse libvirt or the users by introducing yet another option here. Thomas > On 02/05/2018 03:57 PM, Collin L. Walling wrote: >> Set boot menu options for an s390 guest and store them in >> the iplb. These options are set via the QEMU command line >> option: >> >> -boot menu=on|off[,splash-time=X] >> >> or via the libvirt domain xml: >> >> <os> >> <bootmenu enable='yes|no' timeout='X'/> >> </os> >> >> Where X represents some positive integer representing >> milliseconds. >> >> Any value set for loadparm will override all boot menu options. >> If loadparm=PROMPT, then the menu will be enabled without a >> timeout. >> >> The absence of any boot options on the command line will flag >> to later use the zipl boot loader values. >> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> --- >> [...]
On 15.02.2018 07:38, Thomas Huth wrote: > On 14.02.2018 18:46, Collin L. Walling wrote: >> I'm beginning to like the usage of splash-time to represent a timeout >> for the boot menu >> less and less. It is really meant for how long a _splash_ _image_ >> should appear during boot. >> >> I'd like to suggest adding a new boot option "menu-timeout". An >> alternative would be >> documenting in qemu-options.hx that s390 treats "splash-time" as the >> menu-timeout. >> >> Thoughts? > > I think you should keep splash-time and not introduce a new option. > Libvirt seems to map the timeout from <bootmenu enable='yes' > timeout='X'/> to the splash-time option, and according to the libvirt > documentation: "Additional attribute timeout takes the number of > milliseconds the boot menu should wait until it times out." > > So it seems like splash-time is already expected to define the amount of > time for the boot menu. We should not confuse libvirt or the users by > introducing yet another option here. I agree, the QEMU option name was probably poorly chosen, as it really describes the time a user has to interact with the BIOS before it starts booting the OS. BTW: we could have a nice ASCII art splash image (nah ... just kidding) ##### # ##### ##### ### # # # # # # # # # # # # # # # # ##### # ##### ###### # # # # # # # # # # # # # # # # # ##### # ##### ##### ### [...]
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 3e3c3b8..227dccb 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -23,6 +23,8 @@ #include "hw/s390x/ebcdic.h" #include "ipl.h" #include "qemu/error-report.h" +#include "qemu/config-file.h" +#include "qemu/cutils.h" #define KERN_IMAGE_START 0x010000UL #define KERN_PARM_AREA 0x010480UL @@ -219,6 +221,53 @@ static Property s390_ipl_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static void s390_ipl_set_boot_menu(IplParameterBlock *iplb) +{ + QemuOptsList *plist = qemu_find_opts("boot-opts"); + QemuOpts *opts = QTAILQ_FIRST(&plist->head); + uint8_t *flags; + uint16_t *timeout; + const char *tmp; + unsigned long result = 0; + + switch (iplb->pbt) { + case S390_IPL_TYPE_CCW: + case S390_IPL_TYPE_QEMU_SCSI: + flags = &iplb->qipl.boot_menu_flags; + timeout = &iplb->qipl.boot_menu_timeout; + break; + default: + error_report("boot menu is not supported for this device type."); + return; + } + + /* In the absence of -boot menu, use zipl parameters */ + if (!qemu_opt_get(opts, "menu")) { + *flags = BOOT_MENU_FLAG_ZIPL_OPTS; + } else if (boot_menu) { + *flags = BOOT_MENU_FLAG_CMD_OPTS; + + tmp = qemu_opt_get(opts, "splash-time"); + + if (tmp && qemu_strtoul(tmp, NULL, 10, &result)) { + error_report("splash-time value is invalid, forcing it to 0."); + *timeout = 0; + return; + } + + result = (result + 500) / 1000; /* Round and convert to seconds */ + + if (result > 0xffff) { + error_report("splash-time value is greater than 65535000," + " forcing it to 65535000."); + *timeout = 0xffff; + return; + } + + *timeout = cpu_to_be16(result); + } +} + static bool s390_gen_initial_iplb(S390IPLState *ipl) { DeviceState *dev_st; @@ -273,6 +322,9 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl) if (!s390_ipl_set_loadparm(ipl->iplb.loadparm)) { ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; } + + s390_ipl_set_boot_menu(&ipl->iplb); + return true; } diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index 68dcaf8..90c0ed3 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -58,8 +58,13 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; #define DIAG308_FLAGS_LP_VALID 0x80 +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 + struct QemuIplParameters { - uint8_t reserved1[4]; + uint8_t boot_menu_flags; + uint8_t reserved1; + uint16_t boot_menu_timeout; uint64_t netboot_start_addr; uint8_t reserved2[16]; } QEMU_PACKED; diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h index 12f6e40..0bbc1ac 100644 --- a/pc-bios/s390-ccw/iplb.h +++ b/pc-bios/s390-ccw/iplb.h @@ -73,7 +73,9 @@ typedef struct IplParameterBlock IplParameterBlock; extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); struct QemuIplParameters { - uint8_t reserved1[4]; + uint8_t boot_menu_flags; + uint8_t reserved1; + uint16_t boot_menu_timeout; uint64_t netboot_start_addr; uint8_t reserved2[16]; } __attribute__ ((packed));
Set boot menu options for an s390 guest and store them in the iplb. These options are set via the QEMU command line option: -boot menu=on|off[,splash-time=X] or via the libvirt domain xml: <os> <bootmenu enable='yes|no' timeout='X'/> </os> Where X represents some positive integer representing milliseconds. Any value set for loadparm will override all boot menu options. If loadparm=PROMPT, then the menu will be enabled without a timeout. The absence of any boot options on the command line will flag to later use the zipl boot loader values. Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com> --- hw/s390x/ipl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ hw/s390x/ipl.h | 7 ++++++- pc-bios/s390-ccw/iplb.h | 4 +++- 3 files changed, 61 insertions(+), 2 deletions(-)