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

[Viktor Mihajlovski]

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.
>> -    uint8_t  reserved2[16];
>> +    uint8_t  reserved2[14];
>>  } QEMU_PACKED;
>>  typedef struct QemuIplParameters QemuIplParameters;
> 
>  Thomas
> 


-- 
Regards,
 Viktor Mihajlovski