From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
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.
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/s390x/ipl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
hw/s390x/ipl.h | 9 +++++++--
pc-bios/s390-ccw/iplb.h | 9 +++++++--
3 files changed, 66 insertions(+), 4 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 79f5a58..ee2039d 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,54 @@ static Property s390_ipl_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void s390_ipl_set_boot_menu(S390IPLState *ipl)
+{
+ QemuOptsList *plist = qemu_find_opts("boot-opts");
+ QemuOpts *opts = QTAILQ_FIRST(&plist->head);
+ uint8_t *flags = &ipl->qipl.qipl_flags;
+ uint32_t *timeout = &ipl->qipl.boot_menu_timeout;
+ const char *tmp;
+ unsigned long splash_time = 0;
+
+ if (!get_boot_device(0)) {
+ if (boot_menu) {
+ error_report("boot menu requires a bootindex to be specified for "
+ "the IPL device.");
+ }
+ return;
+ }
+
+ switch (ipl->iplb.pbt) {
+ case S390_IPL_TYPE_CCW:
+ break;
+ default:
+ error_report("boot menu is not supported for this device type.");
+ return;
+ }
+
+ if (!boot_menu) {
+ return;
+ }
+
+ *flags |= QIPL_FLAG_BM_OPTS_CMD;
+
+ 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.");
+ *timeout = 0;
+ return;
+ }
+
+ if (splash_time > 0xffffffff) {
+ error_report("splash-time is too large, forcing it to max value.");
+ *timeout = 0xffffffff;
+ return;
+ }
+
+ *timeout = cpu_to_be32(splash_time);
+}
+
static bool s390_gen_initial_iplb(S390IPLState *ipl)
{
DeviceState *dev_st;
@@ -435,6 +486,7 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
}
ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
}
+ s390_ipl_set_boot_menu(ipl);
s390_ipl_prepare_qipl(cpu);
}
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 5cc3b77..d6c6f75 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -91,6 +91,9 @@ void s390_reipl_request(void);
#define QIPL_ADDRESS 0xcc
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD 0x80
+
/*
* The QEMU IPL Parameters will be stored at absolute address
* 204 (0xcc) which means it is 32-bit word aligned but not
@@ -104,9 +107,11 @@ void s390_reipl_request(void);
* in pc-bios/s390-ccw/iplb.h.
*/
struct QemuIplParameters {
- uint8_t reserved1[4];
+ uint8_t qipl_flags;
+ uint8_t reserved1[3];
uint64_t netboot_start_addr;
- uint8_t reserved2[16];
+ uint32_t boot_menu_timeout;
+ uint8_t reserved2[12];
} QEMU_PACKED;
typedef struct QemuIplParameters QemuIplParameters;
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 31d2934..832bb94 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -74,14 +74,19 @@ extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
#define QIPL_ADDRESS 0xcc
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD 0x80
+
/*
* This definition must be kept in sync with the defininition
* in hw/s390x/ipl.h
*/
struct QemuIplParameters {
- uint8_t reserved1[4];
+ uint8_t qipl_flags;
+ uint8_t reserved1[3];
uint64_t netboot_start_addr;
- uint8_t reserved2[16];
+ uint32_t boot_menu_timeout;
+ uint8_t reserved2[12];
} __attribute__ ((packed));
typedef struct QemuIplParameters QemuIplParameters;
--
1.8.3.1
On Mon, 26 Feb 2018 11:42:29 +0100
Thomas Huth <thuth@redhat.com> wrote:
> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
>
> 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.
>
> 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>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/s390x/ipl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> hw/s390x/ipl.h | 9 +++++++--
> pc-bios/s390-ccw/iplb.h | 9 +++++++--
Updating the header, but it is not consumed in the bios?
> 3 files changed, 66 insertions(+), 4 deletions(-)
> +static void s390_ipl_set_boot_menu(S390IPLState *ipl)
> +{
> + QemuOptsList *plist = qemu_find_opts("boot-opts");
> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> + uint8_t *flags = &ipl->qipl.qipl_flags;
> + uint32_t *timeout = &ipl->qipl.boot_menu_timeout;
> + const char *tmp;
> + unsigned long splash_time = 0;
> +
> + if (!get_boot_device(0)) {
> + if (boot_menu) {
> + error_report("boot menu requires a bootindex to be specified for "
> + "the IPL device.");
> + }
> + return;
> + }
> +
> + switch (ipl->iplb.pbt) {
> + case S390_IPL_TYPE_CCW:
> + break;
> + default:
> + error_report("boot menu is not supported for this device type.");
If I specify both a bootindex for a device and a -kernel parameter, I
get this error message. Looks a tad odd, but not sure how to avoid it.
Also, error_report() should not use trailing punctuation, but we can
fix that up with a follow-on patch.
> + return;
> + }
> +
> + if (!boot_menu) {
> + return;
> + }
> +
> + *flags |= QIPL_FLAG_BM_OPTS_CMD;
> +
> + 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.");
> + *timeout = 0;
> + return;
> + }
> +
> + if (splash_time > 0xffffffff) {
> + error_report("splash-time is too large, forcing it to max value.");
> + *timeout = 0xffffffff;
> + return;
> + }
> +
> + *timeout = cpu_to_be32(splash_time);
> +}
> +
On 02/26/2018 01:48 PM, Cornelia Huck wrote:
> On Mon, 26 Feb 2018 11:42:29 +0100
> Thomas Huth <thuth@redhat.com> wrote:
>
>> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
>>
>> 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.
>>
>> 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>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/s390x/ipl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
>> hw/s390x/ipl.h | 9 +++++++--
>> pc-bios/s390-ccw/iplb.h | 9 +++++++--
> Updating the header, but it is not consumed in the bios?
>
>> 3 files changed, 66 insertions(+), 4 deletions(-)
>> +static void s390_ipl_set_boot_menu(S390IPLState *ipl)
>> +{
>> + QemuOptsList *plist = qemu_find_opts("boot-opts");
>> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
>> + uint8_t *flags = &ipl->qipl.qipl_flags;
>> + uint32_t *timeout = &ipl->qipl.boot_menu_timeout;
>> + const char *tmp;
>> + unsigned long splash_time = 0;
>> +
>> + if (!get_boot_device(0)) {
>> + if (boot_menu) {
>> + error_report("boot menu requires a bootindex to be specified for "
>> + "the IPL device.");
>> + }
>> + return;
>> + }
>> +
>> + switch (ipl->iplb.pbt) {
>> + case S390_IPL_TYPE_CCW:
>> + break;
>> + default:
>> + error_report("boot menu is not supported for this device type.");
> If I specify both a bootindex for a device and a -kernel parameter, I
> get this error message. Looks a tad odd, but not sure how to avoid it.
Hmm... perhaps an additional check if no IPLB, then skip trying to set
any boot menu data?
> [...]
--
- Collin L Walling
On 02/26/2018 02:29 PM, Collin L. Walling wrote:
> On 02/26/2018 01:48 PM, Cornelia Huck wrote:
>> On Mon, 26 Feb 2018 11:42:29 +0100
>> Thomas Huth <thuth@redhat.com> wrote:
>>
>> [...]
>>
>>> 3 files changed, 66 insertions(+), 4 deletions(-)
>>> +static void s390_ipl_set_boot_menu(S390IPLState *ipl)
>>> +{
>>> + QemuOptsList *plist = qemu_find_opts("boot-opts");
>>> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
>>> + uint8_t *flags = &ipl->qipl.qipl_flags;
>>> + uint32_t *timeout = &ipl->qipl.boot_menu_timeout;
>>> + const char *tmp;
>>> + unsigned long splash_time = 0;
>>> +
>>> + if (!get_boot_device(0)) {
>>> + if (boot_menu) {
>>> + error_report("boot menu requires a bootindex to be
>>> specified for "
>>> + "the IPL device.");
>>> + }
>>> + return;
>>> + }
>>> +
>>> + switch (ipl->iplb.pbt) {
>>> + case S390_IPL_TYPE_CCW:
>>> + break;
>>> + default:
>>> + error_report("boot menu is not supported for this device
>>> type.");
>> If I specify both a bootindex for a device and a -kernel parameter, I
>> get this error message. Looks a tad odd, but not sure how to avoid it.
>
> Hmm... perhaps an additional check if no IPLB, then skip trying to set
> any boot menu data?
>
>> [...]
>
>
Something like:
if (!ipl->iplb.len) {
return;
}
placed just below the if (!get_boot_device(0)) chunk fixed it for me.If
no IPLB was set,
then the IPLB fields should just all be zeros. Why not just check if
the length is 0 to
determine that we did not set an IPLB at all?
also:
if (!ipl->iplb.len) {
if (boot_menu) {
error_report("boot menu requires an IPLB to function");
}
return;
}
if you think an error message is needed (use a better message, mine is
not helpful but I
just wanted to demonstrate that the if (boot_menu) check should be
nested first).
Thanks for reporting this. Seems to be a few cases that I missed on my end.
--
- Collin L Walling
On Mon, 26 Feb 2018 14:44:45 -0500
"Collin L. Walling" <walling@linux.vnet.ibm.com> wrote:
> On 02/26/2018 02:29 PM, Collin L. Walling wrote:
> > On 02/26/2018 01:48 PM, Cornelia Huck wrote:
> >> On Mon, 26 Feb 2018 11:42:29 +0100
> >> Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> [...]
> >>
> >>> 3 files changed, 66 insertions(+), 4 deletions(-)
> >>> +static void s390_ipl_set_boot_menu(S390IPLState *ipl)
> >>> +{
> >>> + QemuOptsList *plist = qemu_find_opts("boot-opts");
> >>> + QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> >>> + uint8_t *flags = &ipl->qipl.qipl_flags;
> >>> + uint32_t *timeout = &ipl->qipl.boot_menu_timeout;
> >>> + const char *tmp;
> >>> + unsigned long splash_time = 0;
> >>> +
> >>> + if (!get_boot_device(0)) {
> >>> + if (boot_menu) {
> >>> + error_report("boot menu requires a bootindex to be
> >>> specified for "
> >>> + "the IPL device.");
> >>> + }
> >>> + return;
> >>> + }
> >>> +
> >>> + switch (ipl->iplb.pbt) {
> >>> + case S390_IPL_TYPE_CCW:
> >>> + break;
> >>> + default:
> >>> + error_report("boot menu is not supported for this device
> >>> type.");
> >> If I specify both a bootindex for a device and a -kernel parameter, I
> >> get this error message. Looks a tad odd, but not sure how to avoid it.
> >
> > Hmm... perhaps an additional check if no IPLB, then skip trying to set
> > any boot menu data?
> >
> >> [...]
> >
> >
> Something like:
>
> if (!ipl->iplb.len) {
> return;
> }
>
> placed just below the if (!get_boot_device(0)) chunk fixed it for me.If
> no IPLB was set,
> then the IPLB fields should just all be zeros. Why not just check if
> the length is 0 to
> determine that we did not set an IPLB at all?
Yes, that makes the message go away for me.
>
> also:
>
> if (!ipl->iplb.len) {
> if (boot_menu) {
> error_report("boot menu requires an IPLB to function");
> }
> return;
> }
>
> if you think an error message is needed (use a better message, mine is
> not helpful but I
> just wanted to demonstrate that the if (boot_menu) check should be
> nested first).
I'm not sure we want an error message for a command line that booted
without any message previously. It's not like I'd expect a boot menu
when I pass in a kernel anyway...
>
> Thanks for reporting this. Seems to be a few cases that I missed on my end.
The usual result of different people trying things :)
How shall we proceed? I'd be willing to pull this now and then apply
fixups on top, or we can have a respin. Thomas?
On 27.02.2018 10:12, Cornelia Huck wrote:
> On Mon, 26 Feb 2018 14:44:45 -0500
> "Collin L. Walling" <walling@linux.vnet.ibm.com> wrote:
>
>> On 02/26/2018 02:29 PM, Collin L. Walling wrote:
>>> On 02/26/2018 01:48 PM, Cornelia Huck wrote:
>>>> On Mon, 26 Feb 2018 11:42:29 +0100
>>>> Thomas Huth <thuth@redhat.com> wrote:
[...]
>>>>> + switch (ipl->iplb.pbt) {
>>>>> + case S390_IPL_TYPE_CCW:
>>>>> + break;
>>>>> + default:
>>>>> + error_report("boot menu is not supported for this device
>>>>> type.");
>>>> If I specify both a bootindex for a device and a -kernel parameter, I
>>>> get this error message. Looks a tad odd, but not sure how to avoid it.
>>>
>>> Hmm... perhaps an additional check if no IPLB, then skip trying to set
>>> any boot menu data?
[...]
> How shall we proceed? I'd be willing to pull this now and then apply
> fixups on top, or we can have a respin. Thomas?
Since this is about a change in hw/s390x/ and not about
pc-bios/s390-ccw/ (i.e. no need to rebuild the firmware binaries for
this), I think I'd rather prefer a fix-up patch on top instead of a re-spin.
Thomas
On 02/27/2018 04:22 AM, Thomas Huth wrote:
> On 27.02.2018 10:12, Cornelia Huck wrote:
>> On Mon, 26 Feb 2018 14:44:45 -0500
>> "Collin L. Walling" <walling@linux.vnet.ibm.com> wrote:
>>
>>> On 02/26/2018 02:29 PM, Collin L. Walling wrote:
>>>> On 02/26/2018 01:48 PM, Cornelia Huck wrote:
>>>>> On Mon, 26 Feb 2018 11:42:29 +0100
>>>>> Thomas Huth <thuth@redhat.com> wrote:
> [...]
>>>>>> + switch (ipl->iplb.pbt) {
>>>>>> + case S390_IPL_TYPE_CCW:
>>>>>> + break;
>>>>>> + default:
>>>>>> + error_report("boot menu is not supported for this device
>>>>>> type.");
>>>>> If I specify both a bootindex for a device and a -kernel parameter, I
>>>>> get this error message. Looks a tad odd, but not sure how to avoid it.
>>>> Hmm... perhaps an additional check if no IPLB, then skip trying to set
>>>> any boot menu data?
> [...]
>> How shall we proceed? I'd be willing to pull this now and then apply
>> fixups on top, or we can have a respin. Thomas?
> Since this is about a change in hw/s390x/ and not about
> pc-bios/s390-ccw/ (i.e. no need to rebuild the firmware binaries for
> this), I think I'd rather prefer a fix-up patch on top instead of a re-spin.
>
> Thomas
>
I'll get right on it and post to the qemu lists.
--
- Collin L Walling
© 2016 - 2025 Red Hat, Inc.