Re: [qemu-s390x] [PATCH v2 2/5] s390-ccw: ipl structs for eckd cdl/ldl

[Collin L. Walling]

On 12/14/2017 12:41 PM, Cornelia Huck wrote:
> On Mon, 11 Dec 2017 17:19:17 -0500
> "Collin L. Walling" <address@hidden> 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 <address@hidden>
> > Acked-by: Janosch Frank <address@hidden>
> > ---
> >   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?


--
- Collin L Walling