On 16.02.2018 17:44, Collin L. Walling wrote: > On 02/16/2018 11:36 AM, Viktor Mihajlovski wrote: >> On 16.02.2018 17:20, Thomas Huth wrote: >> [...] >>>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h >>>> index cab8a97..7c3cab8 100644 >>>> --- a/hw/s390x/ipl.h >>>> +++ b/hw/s390x/ipl.h >>>> @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; >>>> #define QIPL_ADDRESS 0xcc >>>> +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 >>>> +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 >>>> + >>>> struct QemuIplParameters { >>>> - uint8_t reserved1[4]; >>>> + uint8_t boot_menu_flags; >>>> + uint8_t reserved1; >>>> + uint32_t boot_menu_timeout; >>>> uint64_t netboot_start_addr; >>> The netboot_start_addr field is now never aligned anymore, neither on >>> the host side, nor in guest memory. Not a big problem since the struct >>> is declared with "QEMU_PACKED", but still ... it's always nicer to try >>> to align fields to their natural boundaries. So maybe move >>> boot_menu_flags and reserved1 after netboot_start_addr ? >>> >> Good catch ... we probably should document the alignment needs and state >> that the ipl parameters starts on a word boundary (and that the block >> may not be larger than 28 bytes) in a comment block. > > How does this sound? > > /* word aligned and cannot exceed 28 bytes */ > Maybe: /* * The Qemu IPL Parameters will be stored 32-bit word aligned. * Placement of data fields in this area must account for * their alignment needs. * The entire structure must not be larger than 28 bytes. */ > >>>> - uint8_t reserved2[16]; >>>> + uint8_t reserved2[14]; >>>> } QEMU_PACKED; >>>> typedef struct QemuIplParameters QemuIplParameters; >>> Thomas >>> >> > >
-- Regards, Viktor Mihajlovski