On 16.02.2018 15:57, Collin L. Walling wrote: > On 02/16/2018 07:19 AM, Viktor Mihajlovski wrote: >> 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 <wall...@linux.vnet.ibm.com> >>>> --- >>>> 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. > > The idea was to mimic the Scsi MBR struct defined in zipl. It has > reserved space for what would appear to be for 5 more ScsiBlockPtrs > (which are not used in zipl) and a boot_info struct (which we don't need > in QEMU). > > Alternatively, I can just omit the reserved2 field. I placed it there > for "completeness" but it is not at all necessary.
I'd say just drop it. It looks rather confusing than helpful to me. Thomas