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

[Collin Walling <walling@linux.ibm.com>]

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.

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.


-- 
Respectfully,
- Collin Walling