Re: [PATCH v1 1/1] pc-bios/s390-ccw: fix sclp_get_loadparm_ascii

[Christian Borntraeger]

On 28.11.19 13:45, Cornelia Huck wrote:
> On Thu, 28 Nov 2019 13:35:29 +0100
> Christian Borntraeger <address@hidden> wrote:
> 
>> Ack.
>>
>> Conny, I think this would be really nice to have for 4.2 (together with a 
>> bios rebuild)
>> as this fixes a regression. Opinions?
> 
> I fear that this is a bit late for 4.2... but this should get a
> cc:stable.

So we would rather ship a qemu regression instead of pushing a 1 line fixup?
Peter, what is the current state of 4.2? does it look like we will have an rc4
or is everything else silent.

> 
>>
>>
>>
>> On 28.11.19 13:33, Claudio Imbrenda wrote:
>>> The existing s390 bios gets the LOADPARM information from the system using
>>> an SCLP call that specifies a buffer length too small to contain all the
>>> output.
>>>
>>> The recent fixes in the SCLP code have exposed this bug, since now the
>>> SCLP call will return an error (as per architecture) instead of
>>> writing partially and completing successfully.
>>>
>>> The solution is simply to specify the full page length as the SCCB
>>> length instead of a smaller size.
>>>
>>> Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
>>> Fixes: 9a22473c70f3 ("pc-bios/s390-ccw: get LOADPARM stored in SCP Read 
>>> Info")
>>>
>>> Signed-off-by: Claudio Imbrenda <address@hidden>
>>> ---
>>>  pc-bios/s390-ccw/sclp.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
>>> index c0223fa..7251f9a 100644
>>> --- a/pc-bios/s390-ccw/sclp.c
>>> +++ b/pc-bios/s390-ccw/sclp.c
>>> @@ -112,7 +112,7 @@ void sclp_get_loadparm_ascii(char *loadparm)
>>>      ReadInfo *sccb = (void *)_sccb;
>>>  
>>>      memset((char *)_sccb, 0, sizeof(ReadInfo));
>>> -    sccb->h.length = sizeof(ReadInfo);
>>> +    sccb->h.length = SCCB_SIZE;
>>>      if (!sclp_service_call(SCLP_CMDW_READ_SCP_INFO, sccb)) {
>>>          ebcdic_to_ascii((char *) sccb->loadparm, loadparm, LOADPARM_LEN);
>>>      }
>>>   
> 
> The change seems sane.
>