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

[Thomas Huth]

On 12.04.2018 23:04, Farhan Ali wrote:
> 
> 
> 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.

Ah, great, thanks to both of you for the explanation, that was the part
that I was missing. I was only looking at the tables in
include/hw/s390x/ebcdic.h (since I thought that the dots were already
created on the QEMU side), and did not expect that the pc-bios could
create them.

The patch now makes sense to me:

Reviewed-by: Thomas Huth <address@hidden>