[v8] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

Posted by Collin L. Walling 7 years, 8 months ago

Due to the introduction of the QemuIplParameter block and taking some time to
revist some areas, a few chunks of code have been reworked to better align with
those changes. I also think the reworkings improve readability. The same 
functionality is still there, the code just looks a little different (and 
hopefully looks better).

--- [v8] ---

The tl;dr:

    cleaned up some early patches based on review, threw zipl boot option code
    into its own patch, refactored s390_ipl_set_boot_menu to reflect latest
    changes.

The "pls explain":

    * cleaned up "s390-ccw: refactor boot map table code"

        - removed void ptr casting in a couple of spots

    * cleaned up "s390-ccw: set cp_receive mask..." patch

        - setting the mask consumes a service interrupt, so there is no need for 
          a followup sclp_print

    * fixed uitoa based on feedback from v7

    * fixed alignment concerns in QemuIplParameters and renamed flags field

        - reasoning: necessary; better readability

        - s/boot_menu_flags/qipl_flags so this field can be (potentially) used for 
          other purposes in the future

        - when setting the boot menu parms in the bios, we take care to mask out
          the appropriate qipl_flags for boot menu specific flags

        - boot menu flags defines are renamed to be prefixed with "QIPL_FLAG_"

        - also updated comment above this struct

    * cleaned up "s390-ccw: read user input..." patch

        - defines for low core external interrupt code addr and 
          clock comparator interrupt code

        - take care to make sure buf remains null terminated when passed to read_prompt
    
    * [NEW PATCH] "s390-ccw: use zipl values when no boot menu options are present"

        - reasoning: better readability; further explanation of feature

        - *nothing new added to this patch series here*

        - zipl options flag setting and parsing *moved to* this patch

        - this attempts to better explain how these fields are used and how they get
          parsed

        - the commit message gives details on how to set these fields in the zipl
          configuration file

        - the zipl options are only set for CCW type IPL devices (since no 
          other devices actually support it)

    * refactored s390_ipl_set_boot_menu

        - reasoning: better readability

        - the idea is that we should take care to appropriately set the boot menu
          flags for each IPL device type from the beginning. We should not set
          certain flags for devices that cannot support certain features (eg SCSI 
          does not support zipl menus, so we should never set the use_zipl_opts flag
          for SCSI) 

        - since there are no longer boot menu fields specific to each IPL type,
          the switch statement is simply used to detect if the IPL device type
          is capable of a boot menu

        - since zipl flags are only set for CCW type IPL devices, I reworked 
          the logic and removed some indentation

    * s/menu_check_flags/menu_is_enabled

        - reasoning: better readability

        - no parameters

        - "if menu is enabled" reads better than "if these specific flag bits are set"

    * removed menu.h

        - reasoning: file not needed; less to maintain

        - introduction of qipl and better understanding of zipl options led to 
          this decision. by the end of these changes, this file ended up 
          housing 4 function declarations and no longer seemed necessary

        - all menu related function declarations are in s390-ccw.h

        - boot menu flags are defined in iplb.h (which aligns with hw/s390x/ipl.h)

--- [Summary] ---

These patches implement a boot menu for ECKD DASD and SCSI guests on s390x. 
The menu will only appear if the disk has been configured for IPL with the 
zIPL tool and with the following QEMU command line options:

    -boot menu=on[,splash-time=X] and/or -machine loadparm='prompt'

The following must be specified for the device to be IPL'd from:

    -device ...,bootindex=1

or via the following libvirt domain xml:

    <os>
      <bootmenu enable='yes' timeout='X'/>
    </os>

    or
    
    <disk>
      ...
      <boot order='1' loadparm='PROMPT'/>
    </disk>

Where X is some positive integer representing time in milliseconds.

<boot order='1' ... > must be specified for the device to be IPL'd from

A loadparm other than 'prompt' will disable the menu and just boot 
the specified entry.

If no boot options are specified, we will attempt to use the values
provided by zipl (ECKD DASD only).

Collin L. Walling (13):
  s390-ccw: refactor boot map table code
  s390-ccw: refactor eckd_block_num to use CHS
  s390-ccw: refactor IPL structs
  s390-ccw: update libc
  s390-ccw: move auxiliary IPL data to separate location
  s390-ccw: parse and set boot menu options
  s390-ccw: set up interactive boot menu parameters
  s390-ccw: read stage2 boot loader data to find menu
  s390-ccw: print zipl boot menu
  s390-ccw: read user input for boot index via the SCLP console
  s390-ccw: set cp_receive mask only when needed and consume pending
    service irqs
  s390-ccw: use zipl values when no boot menu options are present
  s390-ccw: interactive boot menu for scsi

 hw/s390x/ipl.c              |  68 ++++++++++++-
 hw/s390x/ipl.h              |  31 +++++-
 pc-bios/s390-ccw/Makefile   |   2 +-
 pc-bios/s390-ccw/bootmap.c  | 184 +++++++++++++++++++++++----------
 pc-bios/s390-ccw/bootmap.h  |  91 ++++++++++-------
 pc-bios/s390-ccw/iplb.h     |  24 ++++-
 pc-bios/s390-ccw/libc.c     |  88 ++++++++++++++++
 pc-bios/s390-ccw/libc.h     |  37 ++++++-
 pc-bios/s390-ccw/main.c     |  49 +++++----
 pc-bios/s390-ccw/menu.c     | 243 ++++++++++++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/s390-ccw.h |   9 ++
 pc-bios/s390-ccw/sclp.c     |  39 ++++---
 pc-bios/s390-ccw/virtio.c   |   2 +-
 13 files changed, 741 insertions(+), 126 deletions(-)
 create mode 100644 pc-bios/s390-ccw/libc.c
 create mode 100644 pc-bios/s390-ccw/menu.c

-- 
2.7.4

Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

Posted by Christian Borntraeger 7 years, 8 months ago

Series
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>


menu on scsi and dasd bootmaps tested successfully.

There is one thing that we might want to fix (can be an addon patch since this is a non-customer
scenario (no libvirt)).

If you start QEMU manually without a bootindex, the -boot menu=on is ignored
if no drive has a bootindex.

For example:

-drive file=/dev/dasda,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
does work

-drive file=/dev/dasda -boot menu=on
does not

instead it prints:
qemu-system-s390x: boot menu is not supported for this device type.

and the boots up the default entry.




On 02/21/2018 08:35 PM, Collin L. Walling wrote:
> Due to the introduction of the QemuIplParameter block and taking some time to
> revist some areas, a few chunks of code have been reworked to better align with
> those changes. I also think the reworkings improve readability. The same 
> functionality is still there, the code just looks a little different (and 
> hopefully looks better).
> 
> --- [v8] ---
> 
> The tl;dr:
> 
>     cleaned up some early patches based on review, threw zipl boot option code
>     into its own patch, refactored s390_ipl_set_boot_menu to reflect latest
>     changes.
> 
> The "pls explain":
> 
>     * cleaned up "s390-ccw: refactor boot map table code"
> 
>         - removed void ptr casting in a couple of spots
> 
>     * cleaned up "s390-ccw: set cp_receive mask..." patch
> 
>         - setting the mask consumes a service interrupt, so there is no need for 
>           a followup sclp_print
> 
>     * fixed uitoa based on feedback from v7
> 
>     * fixed alignment concerns in QemuIplParameters and renamed flags field
> 
>         - reasoning: necessary; better readability
> 
>         - s/boot_menu_flags/qipl_flags so this field can be (potentially) used for 
>           other purposes in the future
> 
>         - when setting the boot menu parms in the bios, we take care to mask out
>           the appropriate qipl_flags for boot menu specific flags
> 
>         - boot menu flags defines are renamed to be prefixed with "QIPL_FLAG_"
> 
>         - also updated comment above this struct
> 
>     * cleaned up "s390-ccw: read user input..." patch
> 
>         - defines for low core external interrupt code addr and 
>           clock comparator interrupt code
> 
>         - take care to make sure buf remains null terminated when passed to read_prompt
>     
>     * [NEW PATCH] "s390-ccw: use zipl values when no boot menu options are present"
> 
>         - reasoning: better readability; further explanation of feature
> 
>         - *nothing new added to this patch series here*
> 
>         - zipl options flag setting and parsing *moved to* this patch
> 
>         - this attempts to better explain how these fields are used and how they get
>           parsed
> 
>         - the commit message gives details on how to set these fields in the zipl
>           configuration file
> 
>         - the zipl options are only set for CCW type IPL devices (since no 
>           other devices actually support it)
> 
>     * refactored s390_ipl_set_boot_menu
> 
>         - reasoning: better readability
> 
>         - the idea is that we should take care to appropriately set the boot menu
>           flags for each IPL device type from the beginning. We should not set
>           certain flags for devices that cannot support certain features (eg SCSI 
>           does not support zipl menus, so we should never set the use_zipl_opts flag
>           for SCSI) 
> 
>         - since there are no longer boot menu fields specific to each IPL type,
>           the switch statement is simply used to detect if the IPL device type
>           is capable of a boot menu
> 
>         - since zipl flags are only set for CCW type IPL devices, I reworked 
>           the logic and removed some indentation
> 
>     * s/menu_check_flags/menu_is_enabled
> 
>         - reasoning: better readability
> 
>         - no parameters
> 
>         - "if menu is enabled" reads better than "if these specific flag bits are set"
> 
>     * removed menu.h
> 
>         - reasoning: file not needed; less to maintain
> 
>         - introduction of qipl and better understanding of zipl options led to 
>           this decision. by the end of these changes, this file ended up 
>           housing 4 function declarations and no longer seemed necessary
> 
>         - all menu related function declarations are in s390-ccw.h
> 
>         - boot menu flags are defined in iplb.h (which aligns with hw/s390x/ipl.h)
> 
> --- [Summary] ---
> 
> These patches implement a boot menu for ECKD DASD and SCSI guests on s390x. 
> The menu will only appear if the disk has been configured for IPL with the 
> zIPL tool and with the following QEMU command line options:
> 
>     -boot menu=on[,splash-time=X] and/or -machine loadparm='prompt'
> 
> The following must be specified for the device to be IPL'd from:
> 
>     -device ...,bootindex=1
> 
> or via the following libvirt domain xml:
> 
>     <os>
>       <bootmenu enable='yes' timeout='X'/>
>     </os>
> 
>     or
>     
>     <disk>
>       ...
>       <boot order='1' loadparm='PROMPT'/>
>     </disk>
> 
> Where X is some positive integer representing time in milliseconds.
> 
> <boot order='1' ... > must be specified for the device to be IPL'd from
> 
> A loadparm other than 'prompt' will disable the menu and just boot 
> the specified entry.
> 
> If no boot options are specified, we will attempt to use the values
> provided by zipl (ECKD DASD only).
> 
> Collin L. Walling (13):
>   s390-ccw: refactor boot map table code
>   s390-ccw: refactor eckd_block_num to use CHS
>   s390-ccw: refactor IPL structs
>   s390-ccw: update libc
>   s390-ccw: move auxiliary IPL data to separate location
>   s390-ccw: parse and set boot menu options
>   s390-ccw: set up interactive boot menu parameters
>   s390-ccw: read stage2 boot loader data to find menu
>   s390-ccw: print zipl boot menu
>   s390-ccw: read user input for boot index via the SCLP console
>   s390-ccw: set cp_receive mask only when needed and consume pending
>     service irqs
>   s390-ccw: use zipl values when no boot menu options are present
>   s390-ccw: interactive boot menu for scsi
> 
>  hw/s390x/ipl.c              |  68 ++++++++++++-
>  hw/s390x/ipl.h              |  31 +++++-
>  pc-bios/s390-ccw/Makefile   |   2 +-
>  pc-bios/s390-ccw/bootmap.c  | 184 +++++++++++++++++++++++----------
>  pc-bios/s390-ccw/bootmap.h  |  91 ++++++++++-------
>  pc-bios/s390-ccw/iplb.h     |  24 ++++-
>  pc-bios/s390-ccw/libc.c     |  88 ++++++++++++++++
>  pc-bios/s390-ccw/libc.h     |  37 ++++++-
>  pc-bios/s390-ccw/main.c     |  49 +++++----
>  pc-bios/s390-ccw/menu.c     | 243 ++++++++++++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/s390-ccw.h |   9 ++
>  pc-bios/s390-ccw/sclp.c     |  39 ++++---
>  pc-bios/s390-ccw/virtio.c   |   2 +-
>  13 files changed, 741 insertions(+), 126 deletions(-)
>  create mode 100644 pc-bios/s390-ccw/libc.c
>  create mode 100644 pc-bios/s390-ccw/menu.c
>

Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

Posted by Viktor Mihajlovski 7 years, 8 months ago

On 22.02.2018 12:51, Christian Borntraeger wrote:
> Series
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> 
> menu on scsi and dasd bootmaps tested successfully.
> 
> There is one thing that we might want to fix (can be an addon patch since this is a non-customer
> scenario (no libvirt)).
> 
> If you start QEMU manually without a bootindex, the -boot menu=on is ignored
> if no drive has a bootindex.
> 
> For example:
> 
> -drive file=/dev/dasda,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
> does work
> 
> -drive file=/dev/dasda -boot menu=on
> does not
> 
> instead it prints:
> qemu-system-s390x: boot menu is not supported for this device type.
> 
> and the boots up the default entry.
> 
That should indeed be a separate patch, as it would move logic currently
in the BIOS up to QEMU (find the first defined virtio disk and select it
as boot disk).
In fact it's more complicated than that, because it would have to
properly account for -boot order=[acdn] and produce the respective IPLB.
While it makes sense, I wouldn't rush that in but rather change the
error message to indicate that -device bootindex is needed to activate
the menu, at least for the time being.
[...]

-- 
Regards,
 Viktor Mihajlovski