diff mbox

[v8,00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

Message ID 68148fff-0c6e-0470-a0e5-0379ecfd3dd8@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Viktor Mihajlovski Feb. 23, 2018, 11:50 a.m. UTC
On 23.02.2018 11:17, Thomas Huth wrote:
> On 23.02.2018 09:53, Christian Borntraeger wrote:
>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not 
>> specify loadparm or boot menu:
>>
>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 
> 
> I can reproduce this now, too. FWIW: It only happens with
> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
> first). Collin, can you reproduce this, too?
> 
>  Thomas
> The idea is to support the zipl boot menu on ECKD disks (which have a timeout by themselves)
even without switching the boot menu on. This is to have the same behavior as one would see
on LPAR or z/VM.
The issue is that the boot code tries to interpret the program table of SCSI bootmaps as if
a boot menu has been specified.

The following patch fixes this. Collin might want to have a say in this matter as well.


The scsi program table was erroneously evaluated as implicit
boot menu. This patch adds a per-bootmap-type menu_is_enabled
function.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
---
 pc-bios/s390-ccw/bootmap.c  | 4 ++--
 pc-bios/s390-ccw/menu.c     | 7 ++++++-
 pc-bios/s390-ccw/s390-ccw.h | 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

Comments

Thomas Huth Feb. 23, 2018, 1:33 p.m. UTC | #1
On 23.02.2018 12:50, Viktor Mihajlovski wrote:
> On 23.02.2018 11:17, Thomas Huth wrote:
>> On 23.02.2018 09:53, Christian Borntraeger wrote:
>>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not 
>>> specify loadparm or boot menu:
>>>
>>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 
>>
>> I can reproduce this now, too. FWIW: It only happens with
>> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
>> first). Collin, can you reproduce this, too?
>>
>>  Thomas
>> The idea is to support the zipl boot menu on ECKD disks (which have a timeout by themselves)
> even without switching the boot menu on. This is to have the same behavior as one would see
> on LPAR or z/VM.
> The issue is that the boot code tries to interpret the program table of SCSI bootmaps as if
> a boot menu has been specified.

Ok, thanks for the explanation, makes sense now.

Collin, could you please spin a v9 with the required changes included?

 Thanks,
  Thomas
Collin L. Walling Feb. 23, 2018, 3:03 p.m. UTC | #2
On 02/23/2018 08:33 AM, Thomas Huth wrote:
> On 23.02.2018 12:50, Viktor Mihajlovski wrote:
>> On 23.02.2018 11:17, Thomas Huth wrote:
>>> On 23.02.2018 09:53, Christian Borntraeger wrote:
>>>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not
>>>> specify loadparm or boot menu:
>>>>
>>>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1
>>> I can reproduce this now, too. FWIW: It only happens with
>>> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
>>> first). Collin, can you reproduce this, too?
>>>
>>>   Thomas
>>> The idea is to support the zipl boot menu on ECKD disks (which have a timeout by themselves)
>> even without switching the boot menu on. This is to have the same behavior as one would see
>> on LPAR or z/VM.
>> The issue is that the boot code tries to interpret the program table of SCSI bootmaps as if
>> a boot menu has been specified.
> Ok, thanks for the explanation, makes sense now.
>
> Collin, could you please spin a v9 with the required changes included?
>
>   Thanks,
>    Thomas
>
>
I see.  I overlooked that we could treat a SCSI device as a CCW device.  
The proposed change makes sense.
I'll respin 9 and reply to this chain.
diff mbox

Patch

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 81dbd4a..b6fd77e 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -265,7 +265,7 @@  static void run_eckd_boot_script(block_number_t bmt_block_nr,
     BootMapTable *bmt = (void *)sec;
     BootMapScript *bms = (void *)sec;
 
-    if (menu_is_enabled()) {
+    if (menu_is_enabled_for_zipl()) {
         loadparm = eckd_get_boot_menu_index(s1b_block_nr);
     }
 
@@ -568,7 +568,7 @@  static void ipl_scsi(void)
     debug_print_int("program table entries", program_table_entries);
     IPL_assert(program_table_entries != 0, "Empty Program Table");
 
-    if (menu_is_enabled()) {
+    if (menu_is_enabled_for_enum()) {
         loadparm = menu_get_enum_boot_index(program_table_entries);
     }
 
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 16b5310..ebe5e9f 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -237,7 +237,12 @@  void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
     timeout = boot_menu_timeout;
 }
 
-bool menu_is_enabled(void)
+bool menu_is_enabled_for_zipl(void)
 {
     return flag;
 }
+
+bool menu_is_enabled_for_enum(void)
+{
+    return flag & ~QIPL_FLAG_BM_OPTS_ZIPL;
+}
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index cfcaf3c..a18381f 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -89,7 +89,8 @@  void zipl_load(void);
 
 /* menu.c */
 void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
-bool menu_is_enabled(void);
+bool menu_is_enabled_for_zipl(void);
+bool menu_is_enabled_for_enum(void);
 int menu_get_zipl_boot_index(const char *menu_data);
 int menu_get_enum_boot_index(int entries);