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

[Farhan Ali <alifm@linux.ibm.com>]

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 <walling@linux.ibm.com>
>>> Reported-by: Vasily Gorbik <gor@linux.ibm.com>
>>> ---
>>>   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.
> 
>