| Message ID | 1513030760-26245-3-git-send-email-walling@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 11 Dec 2017 17:19:17 -0500 "Collin L. Walling" <walling@linux.vnet.ibm.com> wrote: > ECKD DASDs have different IPL structures for CDL and LDL > formats. The current Ipl1 and Ipl2 structs follow the CDL > format, so we prepend "EckdCdl" to them. Boot info for LDL > has been moved to a new struct: EckdLdlIpl1. > > Also introduce structs for IPL stages 1 and 1b and for > disk geometry. > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com> > --- > pc-bios/s390-ccw/bootmap.c | 24 ++++++++++---------- > pc-bios/s390-ccw/bootmap.h | 55 +++++++++++++++++++++++++++++++++------------- > 2 files changed, 53 insertions(+), 26 deletions(-) > > + mbr_block_nr = > + eckd_block_num((void *)&(ipl1->boot_info.bp.ipl.bm_ptr.eckd.bptr)); Let me nominate this as "crazy nested struct of the week". (Just kidding, your patch certainly improves things in general :) > +typedef struct EckdSeekarg { > + uint16_t pad; > + uint16_t cyl; > + uint16_t head; > + uint8_t sec; > + uint8_t pad2; > +} __attribute__ ((packed)) EckdSeekarg; Maybe make this EckdSeekArg?
On 12/14/2017 12:41 PM, Cornelia Huck wrote: > On Mon, 11 Dec 2017 17:19:17 -0500 > "Collin L. Walling" <walling@linux.vnet.ibm.com> wrote: > >> ECKD DASDs have different IPL structures for CDL and LDL >> formats. The current Ipl1 and Ipl2 structs follow the CDL >> format, so we prepend "EckdCdl" to them. Boot info for LDL >> has been moved to a new struct: EckdLdlIpl1. >> >> Also introduce structs for IPL stages 1 and 1b and for >> disk geometry. >> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com> >> --- >> pc-bios/s390-ccw/bootmap.c | 24 ++++++++++---------- >> pc-bios/s390-ccw/bootmap.h | 55 +++++++++++++++++++++++++++++++++------------- >> 2 files changed, 53 insertions(+), 26 deletions(-) >> >> + mbr_block_nr = >> + eckd_block_num((void *)&(ipl1->boot_info.bp.ipl.bm_ptr.eckd.bptr)); > Let me nominate this as "crazy nested struct of the week". > > (Just kidding, your patch certainly improves things in general :) Haha, well I am honored! (Cannot take full credit -- it was quite crazy before :) > > >> +typedef struct EckdSeekarg { >> + uint16_t pad; >> + uint16_t cyl; >> + uint16_t head; >> + uint8_t sec; >> + uint8_t pad2; >> +} __attribute__ ((packed)) EckdSeekarg; > Maybe make this EckdSeekArg? > I'm pretty sure "Seekarg" is not a word, so your suggestion makes sense. (plus it reads better imo)
On 12/14/2017 12:41 PM, Cornelia Huck wrote: > On Mon, 11 Dec 2017 17:19:17 -0500 > "Collin L. Walling" <walling@linux.vnet.ibm.com> wrote: > >> ECKD DASDs have different IPL structures for CDL and LDL >> formats. The current Ipl1 and Ipl2 structs follow the CDL >> format, so we prepend "EckdCdl" to them. Boot info for LDL >> has been moved to a new struct: EckdLdlIpl1. >> >> Also introduce structs for IPL stages 1 and 1b and for >> disk geometry. >> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com> >> --- >> pc-bios/s390-ccw/bootmap.c | 24 ++++++++++---------- >> pc-bios/s390-ccw/bootmap.h | 55 +++++++++++++++++++++++++++++++++------------- >> 2 files changed, 53 insertions(+), 26 deletions(-) >> >> + mbr_block_nr = >> + eckd_block_num((void *)&(ipl1->boot_info.bp.ipl.bm_ptr.eckd.bptr)); > Let me nominate this as "crazy nested struct of the week". > > (Just kidding, your patch certainly improves things in general :) FWIW: we can reduce it to just ipl1->boot_info.bp-- the way the structs are unioned and the ordering of the fields make this possible. Thoughts? > > >> +typedef struct EckdSeekarg { >> + uint16_t pad; >> + uint16_t cyl; >> + uint16_t head; >> + uint8_t sec; >> + uint8_t pad2; >> +} __attribute__ ((packed)) EckdSeekarg; > Maybe make this EckdSeekArg? >
On Mon, 18 Dec 2017 17:11:52 -0500 "Collin L. Walling" <walling@linux.vnet.ibm.com> wrote: > On 12/14/2017 12:41 PM, Cornelia Huck wrote: > > On Mon, 11 Dec 2017 17:19:17 -0500 > > "Collin L. Walling" <walling@linux.vnet.ibm.com> wrote: > > > >> ECKD DASDs have different IPL structures for CDL and LDL > >> formats. The current Ipl1 and Ipl2 structs follow the CDL > >> format, so we prepend "EckdCdl" to them. Boot info for LDL > >> has been moved to a new struct: EckdLdlIpl1. > >> > >> Also introduce structs for IPL stages 1 and 1b and for > >> disk geometry. > >> > >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > >> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com> > >> --- > >> pc-bios/s390-ccw/bootmap.c | 24 ++++++++++---------- > >> pc-bios/s390-ccw/bootmap.h | 55 +++++++++++++++++++++++++++++++++------------- > >> 2 files changed, 53 insertions(+), 26 deletions(-) > >> > >> + mbr_block_nr = > >> + eckd_block_num((void *)&(ipl1->boot_info.bp.ipl.bm_ptr.eckd.bptr)); > > Let me nominate this as "crazy nested struct of the week". > > > > (Just kidding, your patch certainly improves things in general :) > > > FWIW: we can reduce it to just ipl1->boot_info.bp-- the way the structs > are unioned > and the ordering of the fields make this possible. Thoughts? Ah, missed that one. I'd prefer to be explicit here, even if it is long.
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 6f8e30f..5546b79 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -221,9 +221,9 @@ static void run_eckd_boot_script(block_number_t mbr_block_nr) static void ipl_eckd_cdl(void) { XEckdMbr *mbr; - Ipl2 *ipl2 = (void *)sec; + EckdCdlIpl2 *ipl2 = (void *)sec; IplVolumeLabel *vlbl = (void *)sec; - block_number_t block_nr; + block_number_t mbr_block_nr; /* we have just read the block #0 and recognized it as "IPL1" */ sclp_print("CDL\n"); @@ -231,7 +231,7 @@ static void ipl_eckd_cdl(void) memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(1, ipl2, "Cannot read IPL2 record at block 1"); - mbr = &ipl2->u.x.mbr; + mbr = &ipl2->mbr; IPL_assert(magic_match(mbr, ZIPL_MAGIC), "No zIPL section in IPL2 record."); IPL_assert(block_size_ok(mbr->blockptr.xeckd.bptr.size), "Bad block size in zIPL section of IPL2 record."); @@ -239,7 +239,7 @@ static void ipl_eckd_cdl(void) "Non-ECKD device type in zIPL section of IPL2 record."); /* save pointer to Boot Script */ - block_nr = eckd_block_num((void *)&(mbr->blockptr)); + mbr_block_nr = eckd_block_num((void *)&(mbr->blockptr)); memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(2, vlbl, "Cannot read Volume Label at block 2"); @@ -249,7 +249,7 @@ static void ipl_eckd_cdl(void) "Invalid magic of volser block"); print_volser(vlbl->f.volser); - run_eckd_boot_script(block_nr); + run_eckd_boot_script(mbr_block_nr); /* no return */ } @@ -280,8 +280,8 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode) static void ipl_eckd_ldl(ECKD_IPL_mode_t mode) { - block_number_t block_nr; - BootInfo *bip = (void *)(sec + 0x70); /* BootInfo is MBR for LDL */ + block_number_t mbr_block_nr; + EckdLdlIpl1 *ipl1 = (void *)sec; if (mode != ECKD_LDL_UNLABELED) { print_eckd_ldl_msg(mode); @@ -292,15 +292,17 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode) memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(0, sec, "Cannot read block 0 to grab boot info."); if (mode == ECKD_LDL_UNLABELED) { - if (!magic_match(bip->magic, ZIPL_MAGIC)) { + if (!magic_match(ipl1->boot_info.magic, ZIPL_MAGIC)) { return; /* not applicable layout */ } sclp_print("unlabeled LDL.\n"); } - verify_boot_info(bip); + verify_boot_info(&ipl1->boot_info); + + mbr_block_nr = + eckd_block_num((void *)&(ipl1->boot_info.bp.ipl.bm_ptr.eckd.bptr)); - block_nr = eckd_block_num((void *)&(bip->bp.ipl.bm_ptr.eckd.bptr)); - run_eckd_boot_script(block_nr); + run_eckd_boot_script(mbr_block_nr); /* no return */ } diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h index 4980838..b700d08 100644 --- a/pc-bios/s390-ccw/bootmap.h +++ b/pc-bios/s390-ccw/bootmap.h @@ -226,22 +226,47 @@ typedef struct BootInfo { /* @ 0x70, record #0 */ } bp; } __attribute__ ((packed)) BootInfo; /* see also XEckdMbr */ -typedef struct Ipl1 { - unsigned char key[4]; /* == "IPL1" */ - unsigned char data[24]; -} __attribute__((packed)) Ipl1; +/* + * Structs for IPL + */ +#define STAGE2_BLK_CNT_MAX 24 /* Stage 1b can load up to 24 blocks */ -typedef struct Ipl2 { - unsigned char key[4]; /* == "IPL2" */ - union { - unsigned char data[144]; - struct { - unsigned char reserved1[92-4]; - XEckdMbr mbr; - unsigned char reserved2[144-(92-4)-sizeof(XEckdMbr)]; - } x; - } u; -} __attribute__((packed)) Ipl2; +typedef struct EckdCdlIpl1 { + uint8_t key[4]; /* == "IPL1" */ + uint8_t data[24]; +} __attribute__((packed)) EckdCdlIpl1; + +typedef struct EckdSeekarg { + uint16_t pad; + uint16_t cyl; + uint16_t head; + uint8_t sec; + uint8_t pad2; +} __attribute__ ((packed)) EckdSeekarg; + +typedef struct EckdStage1b { + uint8_t reserved[32 * STAGE2_BLK_CNT_MAX]; + struct EckdSeekarg seek[STAGE2_BLK_CNT_MAX]; + uint8_t unused[64]; +} __attribute__ ((packed)) EckdStage1b; + +typedef struct EckdStage1 { + uint8_t reserved[72]; + struct EckdSeekarg seek[2]; +} __attribute__ ((packed)) EckdStage1; + +typedef struct EckdCdlIpl2 { + uint8_t key[4]; /* == "IPL2" */ + struct EckdStage1 stage1; + XEckdMbr mbr; + uint8_t reserved[24]; +} __attribute__((packed)) EckdCdlIpl2; + +typedef struct EckdLdlIpl1 { + uint8_t reserved[24]; + struct EckdStage1 stage1; + BootInfo boot_info; /* BootInfo is MBR for LDL */ +} __attribute__((packed)) EckdLdlIpl1; typedef struct IplVolumeLabel { unsigned char key[4]; /* == "VOL1" */