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

[Viktor Mihajlovski]

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