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

[Collin L. Walling]

On 02/06/2018 12:52 AM, Thomas Huth wrote:
> On 05.02.2018 21:57, Collin L. Walling wrote:
> > - replace ScsiMbr in ECKD code with BootMapTable
> > - fix read_block messages to reflect BMT
> > - reduce ipl_scsi code with BMT struct
> >
> > Signed-off-by: Collin L. Walling <address@hidden>
> > ---
> [...]
> > @@ -449,10 +451,8 @@ static void zipl_run(ScsiBlockPtr *pte)
> >   static void ipl_scsi(void)
> >   {
> >       ScsiMbr *mbr = (void *)sec;
> > -    uint8_t *ns, *ns_end;
> >       int program_table_entries = 0;
> > -    const int pte_len = sizeof(ScsiBlockPtr);
> > -    ScsiBlockPtr *prog_table_entry = NULL;
> > +    BootMapTable *bmt = (void *)sec;
> >       unsigned int loadparm = get_loadparm_index();
> >   
> >       /* Grab the MBR */
> > @@ -467,34 +467,23 @@ static void ipl_scsi(void)
> >       debug_print_int("MBR Version", mbr->version_id);
> >       IPL_check(mbr->version_id == 1,
> >                 "Unknown MBR layout version, assuming version 1");
> > -    debug_print_int("program table", mbr->blockptr[0].blockno);
> > -    IPL_assert(mbr->blockptr[0].blockno, "No Program Table");
> > +    debug_print_int("program table", mbr->bmt.blockno);
> > +    IPL_assert(mbr->bmt.blockno, "No Program Table");
> >   
> >       /* Parse the program table */
> > -    read_block(mbr->blockptr[0].blockno, sec,
> > -               "Error reading Program Table");
> > +    read_block(mbr->bmt.blockno, sec, "Error reading Program Table");
> >   
> >       IPL_assert(magic_match(sec, ZIPL_MAGIC), "No zIPL magic in PT");
> >   
> >       debug_print_int("loadparm index", loadparm);
> > -    ns_end = sec + virtio_get_block_size();
> > -    for (ns = (sec + pte_len); (ns + pte_len) < ns_end; ns += pte_len) {
> > -        prog_table_entry = (ScsiBlockPtr *)ns;
> > -        if (!prog_table_entry->blockno) {
> > -            break;
> > -        }
> >   
> > +    while (bmt->bte[program_table_entries].scsi.blockno) {
> >           program_table_entries++;
> I think it might make sense to somehow limit this loop like it was done
> for the original for-loop, so that in case the sector only contains
> non-zero garbage you do not loop here forever.

It should be easy enough to implement a loop condition like it was before
without getting too messy.

I'll also dig into the docs (or more likely through the zipl code) and see
if there exists a limit on the number of program entires for SCSI.


> > -        if (program_table_entries == loadparm + 1) {
> > -            break; /* selected entry found */
> > -        }
> >       }
> >   
> >       debug_print_int("program table entries", program_table_entries);
> >   
> > -    IPL_assert(program_table_entries != 0, "Empty Program Table");
> Don't want to keep the IPL_assert now that you've kept the
> program_table_entries variable?


True.  I'll re-add the assertion.


> > -    zipl_run(prog_table_entry); /* no return */
> > +    zipl_run(&bmt->bte[loadparm].scsi); /* no return */
> >   }
>   Thomas


--
- Collin L Walling