Re: [qemu-s390x] [PATCH v6 01/12] s390-ccw: refactor boot map table code

[Viktor Mihajlovski]

On 16.02.2018 11:42, Thomas Huth wrote:
> On 15.02.2018 23:54, Collin L. Walling wrote:
>> Some ECKD bootmap code was using structs designed for SCSI.
>> Even though this works, it confuses readability. Add a new
>> BootMapTable struct to assist with readability in bootmap
>> entry code. Also:
>>
>> - replace ScsiMbr in ECKD code with appropriate structs
>> - fix read_block messages to reflect BootMapTable
>> - fixup ipl_scsi to use BootMapTable (referred to as Program Table)
>> - defined value for maximum table entries
>>
>> Signed-off-by: Collin L. Walling <address@hidden>
>> ---
>>  pc-bios/s390-ccw/bootmap.c | 60 
>> +++++++++++++++++++++-------------------------
>>  pc-bios/s390-ccw/bootmap.h | 14 +++++++++--
>>  2 files changed, 39 insertions(+), 35 deletions(-)
> [...]
>> diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
>> index cf99a4c..850b655 100644
>> --- a/pc-bios/s390-ccw/bootmap.h
>> +++ b/pc-bios/s390-ccw/bootmap.h
>> @@ -53,6 +53,15 @@ typedef union BootMapPointer {
>>      ExtEckdBlockPtr xeckd;
>>  } __attribute__ ((packed)) BootMapPointer;
>>  
>> +#define MAX_TABLE_ENTRIES  30
>> +
>> +/* aka Program Table */
>> +typedef struct BootMapTable {
>> +    uint8_t magic[4];
>> +    uint8_t reserved[12];
>> +    BootMapPointer entry[];
>> +} __attribute__ ((packed)) BootMapTable;
>> +
>>  typedef struct ComponentEntry {
>>      ScsiBlockPtr data;
>>      uint8_t pad[7];
>> @@ -69,8 +78,9 @@ typedef struct ComponentHeader {
>>  typedef struct ScsiMbr {
>>      uint8_t magic[4];
>>      uint32_t version_id;
>> -    uint8_t reserved[8];
>> -    ScsiBlockPtr blockptr[];
>> +    uint8_t reserved1[8];
>> +    ScsiBlockPtr pt;   /* block pointer to program table */
>> +    uint8_t reserved2[120];
> 
> Did you want to pad the struct to 512 bytes here? If so, I think that
> should have been "uint32_t" instead of "uint8_t" or "480" instead of "120" ?
> 
Should probably be uint8_t[480] then to stay in style with other
reserved field definitions.
>>  } __attribute__ ((packed)) ScsiMbr;
>>  
>>  #define ZIPL_MAGIC              "zIPL"
>>
> 
>  Thomas
> 


-- 
Regards,
 Viktor Mihajlovski