Re: [qemu-s390x] [Qemu-devel] [PATCH v7 06/12] s390-ccw: parse and set boot menu options

[Viktor Mihajlovski]

On 16.02.2018 23:07, Collin L. Walling wrote:
[...]
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 74469b1..f632c59 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -60,6 +60,9 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi;
> 
>  #define QIPL_ADDRESS  0xcc
> 
> +#define BOOT_MENU_FLAG_CMD_OPTS  0x80
> +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40
> +
>  /*
>   * The QEMU IPL Parameters will be stored 32-bit word aligned.
>   * Placement of data fields in this area must account for
> @@ -67,9 +70,11 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi;
>   * The entire structure must not be larger than 28 bytes.
>   */
>  struct QemuIplParameters {
> -    uint8_t  reserved1[4];
> +    uint8_t  boot_menu_flags;
> +    uint8_t  reserved1[3];
> +    uint32_t boot_menu_timeout;
>      uint64_t netboot_start_addr;
> -    uint8_t  reserved2[16];
> +    uint8_t  reserved2[12];
>  } QEMU_PACKED;Since this has to be touched anyway to re-establish proper 
> alignment, I
could also imagine to define the struct as
  struct QemuIplParameters {
      struct {
          uint32_t flags:8;
          uint32_t timeout:24;
      } QEMU_PACKED boot_menu;
      uint64_t netboot_start_addr;
      uint8_t  reserved2[16];
  } QEMU_PACKED;
would allow to keep the boot menu stuff together without creating
unnecessary holes.
It would allow for a timeout value of more than 4 hours. The code to set
the boot menu would have to be adapted though to properly deal with the
bitfields.
>  typedef struct QemuIplParameters QemuIplParameters;
> 
> diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
> index a23237e..0e39aa0 100644
> --- a/pc-bios/s390-ccw/iplb.h
> +++ b/pc-bios/s390-ccw/iplb.h
> @@ -81,9 +81,11 @@ extern IplParameterBlock iplb 
> __attribute__((__aligned__(PAGE_SIZE)));
>   * The entire structure must not be larger than 28 bytes.
>   */
>  struct QemuIplParameters {
> -    uint8_t  reserved1[4];
> +    uint8_t  boot_menu_flags;
> +    uint8_t  reserved1[3];
> +    uint32_t boot_menu_timeout;
>      uint64_t netboot_start_addr;
> -    uint8_t  reserved2[16];
> +    uint8_t  reserved2[12];
>  } __attribute__ ((packed));
>  typedef struct QemuIplParameters QemuIplParameters;
> 
same here.

-- 
Regards,
 Viktor Mihajlovski