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.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
hw/s390x/ipl.c | 23 +++++++++++++++++++++++
hw/s390x/ipl.h | 8 ++++++--
pc-bios/s390-ccw/iplb.h | 8 ++++++--
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 0d06fc1..0ca9e37 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,23 @@ static Property s390_ipl_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void s390_ipl_set_boot_menu(uint8_t *boot_menu_enabled,
+ uint16_t *boot_menu_timeout)
+{
+ QemuOptsList *plist = qemu_find_opts("boot-opts");
+ QemuOpts *opts = QTAILQ_FIRST(&plist->head);
+ const char *p;
+ long result = 0;
+
+ *boot_menu_enabled = qemu_opt_get_bool(opts, "menu", false);
+
+ if (*boot_menu_enabled) {
+ p = qemu_opt_get(opts, "splash-time");
+ qemu_strtol(p, NULL, 10, &result);
+ *boot_menu_timeout = result;
+ }
+}
+
static bool s390_gen_initial_iplb(S390IPLState *ipl)
{
DeviceState *dev_st;
@@ -245,6 +264,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
ipl->iplb.pbt = S390_IPL_TYPE_CCW;
ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
+ s390_ipl_set_boot_menu(&ipl->iplb.ccw.boot_menu_enabled,
+ &ipl->iplb.ccw.boot_menu_timeout);
} else if (sd) {
SCSIBus *bus = scsi_bus_from_device(sd);
VirtIOSCSI *vdev = container_of(bus, VirtIOSCSI, bus);
@@ -266,6 +287,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
ipl->iplb.scsi.channel = cpu_to_be16(sd->channel);
ipl->iplb.scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
ipl->iplb.scsi.ssid = ccw_dev->sch->ssid & 3;
+ s390_ipl_set_boot_menu(&ipl->iplb.scsi.boot_menu_enabled,
+ &ipl->iplb.scsi.boot_menu_timeout);
} else {
return false; /* unknown device */
}
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 8a705e0..7c960b1 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -17,7 +17,9 @@
struct IplBlockCcw {
uint64_t netboot_start_addr;
- uint8_t reserved0[77];
+ uint8_t reserved0[74];
+ uint8_t boot_menu_enabled;
+ uint16_t boot_menu_timeout;
uint8_t ssid;
uint16_t devno;
uint8_t vm_flags;
@@ -51,7 +53,9 @@ struct IplBlockQemuScsi {
uint32_t lun;
uint16_t target;
uint16_t channel;
- uint8_t reserved0[77];
+ uint8_t reserved0[74];
+ uint8_t boot_menu_enabled;
+ uint16_t boot_menu_timeout;
uint8_t ssid;
uint16_t devno;
} QEMU_PACKED;
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 890aed9..658d163 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -14,7 +14,9 @@
struct IplBlockCcw {
uint64_t netboot_start_addr;
- uint8_t reserved0[77];
+ uint8_t reserved0[74];
+ uint8_t boot_menu_enabled;
+ uint16_t boot_menu_timeout;
uint8_t ssid;
uint16_t devno;
uint8_t vm_flags;
@@ -48,7 +50,9 @@ struct IplBlockQemuScsi {
uint32_t lun;
uint16_t target;
uint16_t channel;
- uint8_t reserved0[77];
+ uint8_t reserved0[74];
+ uint8_t boot_menu_enabled;
+ uint16_t boot_menu_timeout;
uint8_t ssid;
uint16_t devno;
} __attribute__ ((packed));
--
2.7.4
On Mon, 27 Nov 2017 15:55:34 -0500
"Collin L. Walling" <walling@linux.vnet.ibm.com> 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.
>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> hw/s390x/ipl.c | 23 +++++++++++++++++++++++
> hw/s390x/ipl.h | 8 ++++++--
> pc-bios/s390-ccw/iplb.h | 8 ++++++--
> 3 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d06fc1..0ca9e37 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,23 @@ static Property s390_ipl_properties[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +static void s390_ipl_set_boot_menu(uint8_t *boot_menu_enabled,
> + uint16_t *boot_menu_timeout)
> +{
> + QemuOptsList *plist = qemu_find_opts("boot-opts");
> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> + const char *p;
> + long result = 0;
> +
> + *boot_menu_enabled = qemu_opt_get_bool(opts, "menu", false);
This is already parsed in vl.c, isn't it? Can you simply check
boot_menu?
> +
> + if (*boot_menu_enabled) {
> + p = qemu_opt_get(opts, "splash-time");
hw/nvram/fw_cfg.c parses splash-time as well, but I'm not sure if it is
worth parsing it in vl.c as well.
> + qemu_strtol(p, NULL, 10, &result);
> + *boot_menu_timeout = result;
> + }
> +}
> +
> static bool s390_gen_initial_iplb(S390IPLState *ipl)
> {
> DeviceState *dev_st;
On 11/28/2017 06:04 AM, Cornelia Huck wrote:
> On Mon, 27 Nov 2017 15:55:34 -0500
> "Collin L. Walling" <walling@linux.vnet.ibm.com> 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.
>>
>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
>> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> ---
>> hw/s390x/ipl.c | 23 +++++++++++++++++++++++
>> hw/s390x/ipl.h | 8 ++++++--
>> pc-bios/s390-ccw/iplb.h | 8 ++++++--
>> 3 files changed, 35 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 0d06fc1..0ca9e37 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,23 @@ static Property s390_ipl_properties[] = {
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> +static void s390_ipl_set_boot_menu(uint8_t *boot_menu_enabled,
>> + uint16_t *boot_menu_timeout)
>> +{
>> + QemuOptsList *plist = qemu_find_opts("boot-opts");
>> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
>> + const char *p;
>> + long result = 0;
>> +
>> + *boot_menu_enabled = qemu_opt_get_bool(opts, "menu", false);
> This is already parsed in vl.c, isn't it? Can you simply check
> boot_menu?
Well then... how convenient! :)
>
>> +
>> + if (*boot_menu_enabled) {
>> + p = qemu_opt_get(opts, "splash-time");
> hw/nvram/fw_cfg.c parses splash-time as well, but I'm not sure if it is
> worth parsing it in vl.c as well.
It's probably best we parse it ourselves. I see that fw_cfg.c also does
some validation on the splash-time value, so I'll add something similar.
>
>> + qemu_strtol(p, NULL, 10, &result);
>> + *boot_menu_timeout = result;
>> + }
>> +}
>> +
>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>> {
>> DeviceState *dev_st;
--
- Collin L Walling
On 27.11.2017 21:55, 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.
>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> hw/s390x/ipl.c | 23 +++++++++++++++++++++++
> hw/s390x/ipl.h | 8 ++++++--
> pc-bios/s390-ccw/iplb.h | 8 ++++++--
> 3 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d06fc1..0ca9e37 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,23 @@ static Property s390_ipl_properties[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +static void s390_ipl_set_boot_menu(uint8_t *boot_menu_enabled,
> + uint16_t *boot_menu_timeout)
> +{
> + QemuOptsList *plist = qemu_find_opts("boot-opts");
> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> + const char *p;
> + long result = 0;
> +
> + *boot_menu_enabled = qemu_opt_get_bool(opts, "menu", false);
> +
> + if (*boot_menu_enabled) {
> + p = qemu_opt_get(opts, "splash-time");
> + qemu_strtol(p, NULL, 10, &result);
It's maybe better to check the return code of qemu_strtol to see whether
the option contained a valid number? An additional check for negative
numbers would also be fine, I think.
> + *boot_menu_timeout = result;
> + }
> +}
> +
> static bool s390_gen_initial_iplb(S390IPLState *ipl)
> {
> DeviceState *dev_st;
> @@ -245,6 +264,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
> ipl->iplb.pbt = S390_IPL_TYPE_CCW;
> ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
> ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
> + s390_ipl_set_boot_menu(&ipl->iplb.ccw.boot_menu_enabled,
> + &ipl->iplb.ccw.boot_menu_timeout);
> } else if (sd) {
> SCSIBus *bus = scsi_bus_from_device(sd);
> VirtIOSCSI *vdev = container_of(bus, VirtIOSCSI, bus);
> @@ -266,6 +287,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
> ipl->iplb.scsi.channel = cpu_to_be16(sd->channel);
> ipl->iplb.scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
> ipl->iplb.scsi.ssid = ccw_dev->sch->ssid & 3;
> + s390_ipl_set_boot_menu(&ipl->iplb.scsi.boot_menu_enabled,
> + &ipl->iplb.scsi.boot_menu_timeout);
> } else {
> return false; /* unknown device */
> }
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 8a705e0..7c960b1 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -17,7 +17,9 @@
>
> struct IplBlockCcw {
> uint64_t netboot_start_addr;
> - uint8_t reserved0[77];
> + uint8_t reserved0[74];
> + uint8_t boot_menu_enabled;
> + uint16_t boot_menu_timeout;
Could you please try to keep the fields aligned to their natural
alignment? I.e. swap the two new entries, so that timeout comes before
enabled?
> uint8_t ssid;
> uint16_t devno;
> uint8_t vm_flags;
> @@ -51,7 +53,9 @@ struct IplBlockQemuScsi {
> uint32_t lun;
> uint16_t target;
> uint16_t channel;
> - uint8_t reserved0[77];
> + uint8_t reserved0[74];
> + uint8_t boot_menu_enabled;
> + uint16_t boot_menu_timeout;
dito
> uint8_t ssid;
> uint16_t devno;
> } QEMU_PACKED;
Thomas
On 11/28/2017 06:45 AM, Thomas Huth wrote:
> On 27.11.2017 21:55, Collin L. Walling wrote:
>> [...]
>>
>> +static void s390_ipl_set_boot_menu(uint8_t *boot_menu_enabled,
>> + uint16_t *boot_menu_timeout)
>> +{
>> + QemuOptsList *plist = qemu_find_opts("boot-opts");
>> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
>> + const char *p;
>> + long result = 0;
>> +
>> + *boot_menu_enabled = qemu_opt_get_bool(opts, "menu", false);
>> +
>> + if (*boot_menu_enabled) {
>> + p = qemu_opt_get(opts, "splash-time");
>> + qemu_strtol(p, NULL, 10, &result);
> It's maybe better to check the return code of qemu_strtol to see whether
> the option contained a valid number? An additional check for negative
> numbers would also be fine, I think.
Agreed. I'll add some integer overflow checking as well.
>
>> [...]
>>
>> struct IplBlockCcw {
>> uint64_t netboot_start_addr;
>> - uint8_t reserved0[77];
>> + uint8_t reserved0[74];
>> + uint8_t boot_menu_enabled;
>> + uint16_t boot_menu_timeout;
> Could you please try to keep the fields aligned to their natural
> alignment? I.e. swap the two new entries, so that timeout comes before
> enabled?
Can do.
>
>> uint8_t ssid;
>> uint16_t devno;
>> uint8_t vm_flags;
>> @@ -51,7 +53,9 @@ struct IplBlockQemuScsi {
>> uint32_t lun;
>> uint16_t target;
>> uint16_t channel;
>> - uint8_t reserved0[77];
>> + uint8_t reserved0[74];
>> + uint8_t boot_menu_enabled;
>> + uint16_t boot_menu_timeout;
> dito
>
>> uint8_t ssid;
>> uint16_t devno;
>> } QEMU_PACKED;
> Thomas
>
>
>
--
- Collin L Walling
On 27.11.2017 21:55, 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. Aren't this properties usually contained in the zIPL block? (e.g. written and configured by zipl) -- Thanks, David / dhildenb
On 11/29/2017 05:28 PM, David Hildenbrand wrote: > On 27.11.2017 21:55, 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. > Aren't this properties usually contained in the zIPL block? (e.g. > written and configured by zipl) > I believe the timeout value is nestled in there somewhere, yes. However it was requested that we control the boot menu timeout value via the Qemu command line instead. :) -- - Collin L Walling
On 29.11.2017 23:33, Collin L. Walling wrote: > On 11/29/2017 05:28 PM, David Hildenbrand wrote: >> On 27.11.2017 21:55, 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. >> Aren't this properties usually contained in the zIPL block? (e.g. >> written and configured by zipl) >> > I believe the timeout value is nestled in there somewhere, yes. > However it was requested that we control the boot menu timeout value > via the Qemu command line instead. :) > Wonder if it makes sense to always act like the zIPL loader (display menu/timeout) but allow to overwrite it via cmd line. -- Thanks, David / dhildenb
On Thu, 30 Nov 2017 16:52:37 +0100 David Hildenbrand <david@redhat.com> wrote: > On 29.11.2017 23:33, Collin L. Walling wrote: > > On 11/29/2017 05:28 PM, David Hildenbrand wrote: > >> On 27.11.2017 21:55, 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. > >> Aren't this properties usually contained in the zIPL block? (e.g. > >> written and configured by zipl) > >> > > I believe the timeout value is nestled in there somewhere, yes. > > However it was requested that we control the boot menu timeout value > > via the Qemu command line instead. :) > > > > Wonder if it makes sense to always act like the zIPL loader (display > menu/timeout) but allow to overwrite it via cmd line. > I think that make quite a bit of sense.
© 2016 - 2026 Red Hat, Inc.