On 16.02.2018 23:07, 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 | 11 ++++++++-
> 2 files changed, 37 insertions(+), 34 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> index 67a6123..286de40 100644
> --- a/pc-bios/s390-ccw/bootmap.c
> +++ b/pc-bios/s390-ccw/bootmap.c
> @@ -182,24 +182,24 @@ static block_number_t load_eckd_segments(block_number_t
> blk, uint64_t *address)
> return block_nr;
> }
>
> -static void run_eckd_boot_script(block_number_t mbr_block_nr)
> +static void run_eckd_boot_script(block_number_t bmt_block_nr)
> {
> int i;
> unsigned int loadparm = get_loadparm_index();
> block_number_t block_nr;
> uint64_t address;
> - ScsiMbr *bte = (void *)sec; /* Eckd bootmap table entry */
> + BootMapTable *bmt = (void *)sec;
> BootMapScript *bms = (void *)sec;
>
> debug_print_int("loadparm", loadparm);
> - IPL_assert(loadparm < 31, "loadparm value greater than"
> + IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
> " maximum number of boot entries allowed");
>
> memset(sec, FREE_SPACE_FILLER, sizeof(sec));
> - read_block(mbr_block_nr, sec, "Cannot read MBR");
> + read_block(bmt_block_nr, sec, "Cannot read Boot Map Table");
>
> - block_nr = eckd_block_num((void *)&(bte->blockptr[loadparm]));
> - IPL_assert(block_nr != -1, "No Boot Map");
> + block_nr = eckd_block_num((void *)&bmt->entry[loadparm]);
I think you could drop the "(void *)" cast here now.
> + IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry");
>
> memset(sec, FREE_SPACE_FILLER, sizeof(sec));
> read_block(block_nr, sec, "Cannot read Boot Map Script");
> @@ -223,7 +223,7 @@ static void ipl_eckd_cdl(void)
> XEckdMbr *mbr;
> Ipl2 *ipl2 = (void *)sec;
> IplVolumeLabel *vlbl = (void *)sec;
> - block_number_t block_nr;
> + block_number_t bmt_block_nr;
>
> /* we have just read the block #0 and recognized it as "IPL1" */
> sclp_print("CDL\n");
> @@ -238,8 +238,8 @@ static void ipl_eckd_cdl(void)
> IPL_assert(mbr->dev_type == DEV_TYPE_ECKD,
> "Non-ECKD device type in zIPL section of IPL2 record.");
>
> - /* save pointer to Boot Script */
> - block_nr = eckd_block_num((void *)&(mbr->blockptr));
> + /* save pointer to Boot Map Table */
> + bmt_block_nr = eckd_block_num((void *)&mbr->blockptr);
dito, (void *) should not be needed here.
But apart from those two nits, the patch looks fine to me now, so:
Reviewed-by: Thomas Huth <th...@redhat.com>
