| Message ID | 1518735273-16089-7-git-send-email-walling@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15.02.2018 23:54, 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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > hw/s390x/ipl.h | 9 +++++++-- > pc-bios/s390-ccw/iplb.h | 6 ++++-- > 3 files changed, 59 insertions(+), 4 deletions(-) [....] > diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h > index cab8a97..7c3cab8 100644 > --- a/hw/s390x/ipl.h > +++ b/hw/s390x/ipl.h > @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; > > #define QIPL_ADDRESS 0xcc > > +#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; > + uint32_t boot_menu_timeout; > uint64_t netboot_start_addr; The netboot_start_addr field is now never aligned anymore, neither on the host side, nor in guest memory. Not a big problem since the struct is declared with "QEMU_PACKED", but still ... it's always nicer to try to align fields to their natural boundaries. So maybe move boot_menu_flags and reserved1 after netboot_start_addr ? > - uint8_t reserved2[16]; > + uint8_t reserved2[14]; > } QEMU_PACKED; > typedef struct QemuIplParameters QemuIplParameters; Thomas
On 02/16/2018 11:20 AM, Thomas Huth wrote: > On 15.02.2018 23:54, 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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >> hw/s390x/ipl.h | 9 +++++++-- >> pc-bios/s390-ccw/iplb.h | 6 ++++-- >> 3 files changed, 59 insertions(+), 4 deletions(-) > [....] >> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h >> index cab8a97..7c3cab8 100644 >> --- a/hw/s390x/ipl.h >> +++ b/hw/s390x/ipl.h >> @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; >> >> #define QIPL_ADDRESS 0xcc >> >> +#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; >> + uint32_t boot_menu_timeout; >> uint64_t netboot_start_addr; > The netboot_start_addr field is now never aligned anymore, neither on > the host side, nor in guest memory. Not a big problem since the struct > is declared with "QEMU_PACKED", but still ... it's always nicer to try > to align fields to their natural boundaries. So maybe move > boot_menu_flags and reserved1 after netboot_start_addr ? Makes sense. Will fixup. > >> - uint8_t reserved2[16]; >> + uint8_t reserved2[14]; >> } QEMU_PACKED; >> typedef struct QemuIplParameters QemuIplParameters; > Thomas >
On 16.02.2018 17:20, Thomas Huth wrote: [...] >> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h >> index cab8a97..7c3cab8 100644 >> --- a/hw/s390x/ipl.h >> +++ b/hw/s390x/ipl.h >> @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; >> >> #define QIPL_ADDRESS 0xcc >> >> +#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; >> + uint32_t boot_menu_timeout; >> uint64_t netboot_start_addr; > > The netboot_start_addr field is now never aligned anymore, neither on > the host side, nor in guest memory. Not a big problem since the struct > is declared with "QEMU_PACKED", but still ... it's always nicer to try > to align fields to their natural boundaries. So maybe move > boot_menu_flags and reserved1 after netboot_start_addr ? > Good catch ... we probably should document the alignment needs and state that the ipl parameters starts on a word boundary (and that the block may not be larger than 28 bytes) in a comment block. >> - uint8_t reserved2[16]; >> + uint8_t reserved2[14]; >> } QEMU_PACKED; >> typedef struct QemuIplParameters QemuIplParameters; > > Thomas >
On 02/16/2018 11:36 AM, Viktor Mihajlovski wrote: > On 16.02.2018 17:20, Thomas Huth wrote: > [...] >>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h >>> index cab8a97..7c3cab8 100644 >>> --- a/hw/s390x/ipl.h >>> +++ b/hw/s390x/ipl.h >>> @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; >>> >>> #define QIPL_ADDRESS 0xcc >>> >>> +#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; >>> + uint32_t boot_menu_timeout; >>> uint64_t netboot_start_addr; >> The netboot_start_addr field is now never aligned anymore, neither on >> the host side, nor in guest memory. Not a big problem since the struct >> is declared with "QEMU_PACKED", but still ... it's always nicer to try >> to align fields to their natural boundaries. So maybe move >> boot_menu_flags and reserved1 after netboot_start_addr ? >> > Good catch ... we probably should document the alignment needs and state > that the ipl parameters starts on a word boundary (and that the block > may not be larger than 28 bytes) in a comment block. How does this sound? /* word aligned and cannot exceed 28 bytes */ >>> - uint8_t reserved2[16]; >>> + uint8_t reserved2[14]; >>> } QEMU_PACKED; >>> typedef struct QemuIplParameters QemuIplParameters; >> Thomas >> >
On 16.02.2018 17:44, Collin L. Walling wrote: > On 02/16/2018 11:36 AM, Viktor Mihajlovski wrote: >> On 16.02.2018 17:20, Thomas Huth wrote: >> [...] >>>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h >>>> index cab8a97..7c3cab8 100644 >>>> --- a/hw/s390x/ipl.h >>>> +++ b/hw/s390x/ipl.h >>>> @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; >>>> #define QIPL_ADDRESS 0xcc >>>> +#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; >>>> + uint32_t boot_menu_timeout; >>>> uint64_t netboot_start_addr; >>> The netboot_start_addr field is now never aligned anymore, neither on >>> the host side, nor in guest memory. Not a big problem since the struct >>> is declared with "QEMU_PACKED", but still ... it's always nicer to try >>> to align fields to their natural boundaries. So maybe move >>> boot_menu_flags and reserved1 after netboot_start_addr ? >>> >> Good catch ... we probably should document the alignment needs and state >> that the ipl parameters starts on a word boundary (and that the block >> may not be larger than 28 bytes) in a comment block. > > How does this sound? > > /* word aligned and cannot exceed 28 bytes */ > Maybe: /* * The Qemu IPL Parameters will be stored 32-bit word aligned. * Placement of data fields in this area must account for * their alignment needs. * The entire structure must not be larger than 28 bytes. */ > >>>> - uint8_t reserved2[16]; >>>> + uint8_t reserved2[14]; >>>> } QEMU_PACKED; >>>> typedef struct QemuIplParameters QemuIplParameters; >>> Thomas >>> >> > >
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 31565ce..c8109f5 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -23,6 +23,9 @@ #include "hw/s390x/ebcdic.h" #include "ipl.h" #include "qemu/error-report.h" +#include "qemu/config-file.h" +#include "qemu/cutils.h" +#include "qemu/option.h" #define KERN_IMAGE_START 0x010000UL #define KERN_PARM_AREA 0x010480UL @@ -219,6 +222,50 @@ 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; + uint32_t *timeout; + const char *tmp; + unsigned long splash_time = 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, &splash_time)) { + error_report("splash-time is invalid, forcing it to 0."); + splash_time = 0; + return; + } + + if (splash_time > 0xffffffff) { + error_report("splash-time is too large, forcing it to max value."); + splash_time = 0xffffffff; + return; + } + + *timeout = cpu_to_be32(splash_time); + } +} + static bool s390_gen_initial_iplb(S390IPLState *ipl) { DeviceState *dev_st; @@ -435,6 +482,7 @@ void s390_ipl_prepare_cpu(S390CPU *cpu) } ipl->iplb.qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr); } + s390_ipl_set_boot_menu(&ipl->iplb); s390_ipl_prepare_qipl(cpu); } diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index cab8a97..7c3cab8 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; #define QIPL_ADDRESS 0xcc +#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; + uint32_t boot_menu_timeout; uint64_t netboot_start_addr; - uint8_t reserved2[16]; + uint8_t reserved2[14]; } QEMU_PACKED; typedef struct QemuIplParameters QemuIplParameters; diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h index 1266884..e8e442d 100644 --- a/pc-bios/s390-ccw/iplb.h +++ b/pc-bios/s390-ccw/iplb.h @@ -75,9 +75,11 @@ extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); #define QIPL_ADDRESS 0xcc struct QemuIplParameters { - uint8_t reserved1[4]; + uint8_t boot_menu_flags; + uint8_t reserved1; + uint32_t boot_menu_timeout; uint64_t netboot_start_addr; - uint8_t reserved2[16]; + uint8_t reserved2[14]; } __attribute__ ((packed)); typedef struct QemuIplParameters QemuIplParameters;
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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ hw/s390x/ipl.h | 9 +++++++-- pc-bios/s390-ccw/iplb.h | 6 ++++-- 3 files changed, 59 insertions(+), 4 deletions(-)