Re: [qemu-s390x] [Qemu-devel] [PATCH 2/4] pc-bios/s390-ccw: fix loadparm initialization and int conversion

[Farhan Ali]

On 04/12/2018 04:57 PM, Collin Walling wrote:
> On 04/12/2018 02:57 PM, Thomas Huth wrote:
> > On 10.04.2018 17:01, Collin Walling wrote:
> > > Rename the loadparm char array in main.c to loadparm_str and
> > > increase the size by one byte to account for a null termination
> > > when converting the loadparm string to an int via atoui. Also
> > > allow the boot menu to be enabled when loadparm is set to an
> > > empty string or a series of spaces.
> > >
> > > Signed-off-by: Collin Walling <address@hidden>
> > > Reported-by: Vasily Gorbik <address@hidden>
> > > ---
> > >   hw/s390x/ipl.c          |  2 ++
> > >   pc-bios/s390-ccw/main.c | 14 +++++++-------
> > >   2 files changed, 9 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> > > index fdeaec3..23b5b54 100644
> > > --- a/hw/s390x/ipl.c
> > > +++ b/hw/s390x/ipl.c
> > > @@ -352,6 +352,8 @@ int s390_ipl_set_loadparm(uint8_t *loadparm)
> > >               loadparm[i] = ascii2ebcdic[(uint8_t) lp[i]];
> > >           }
> > >   
> > > +        memset(loadparm + i, 0x40, 8 - i); /* fill with EBCDIC spaces */
> > > +
> > >           g_free(lp);
> > >           return 0;
> > >       }
> > > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> > > index 9d9f8cf..26f9adf 100644
> > > --- a/pc-bios/s390-ccw/main.c
> > > +++ b/pc-bios/s390-ccw/main.c
> > > @@ -15,11 +15,11 @@
> > >   char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
> > >   static SubChannelId blk_schid = { .one = 1 };
> > >   IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> > > -static char loadparm[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
> > > +static char loadparm_str[9] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 };
> > >   QemuIplParameters qipl;
> > >   
> > >   #define LOADPARM_PROMPT "PROMPT  "
> > > -#define LOADPARM_EMPTY  "........"
> > > +#define LOADPARM_EMPTY  "        "
> >
> > Sorry for my ignorance, but why was the old string containing dots?
> >
> >   Thomas
>
> No need for apologies :)
>
> If -machine loadparm is *not* present on the command line, then the loadparm in 
> the sclp
> will be a series of nulls. For whatever reason, that gets translated into a 
> series of dots.

It's because of the ebc2asc table we use for conversion, which results 
in the dots when converting from ebcdic_to_ascii.

> If -machine loadparm="" is present, then loadparm will in the sclp will be a 
> series of
> spaces. >
> We want to enable the boot menu for both of these cases and, to make things 
> easier, this
> patch replaces any nulls (dots) to spaces when setting loadparm so that we only 
> have to
> handle one case instead of two within the bios.