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

2019-11-28 Thread Claudio Imbrenda 
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 
---
 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);
 }
-- 
2.7.4

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

2019-11-28 Thread Christian Borntraeger 
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?



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 
> ---
>  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);
>  }
>

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

2019-11-28 Thread Janosch Frank 
On 11/28/19 1:33 PM, 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")


"pc-bios/s390-ccw: fix sclp readinfo buffer length indication"?

Reviewed-by: Janosch Frank 

> 
> Signed-off-by: Claudio Imbrenda 
> ---
>  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);
>  }
> 




signature.asc
Description: OpenPGP digital signature

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

2019-11-28 Thread Claudio Imbrenda 
On Thu, 28 Nov 2019 13:33:57 +0100
Claudio Imbrenda  wrote:

[...]

> Signed-off-by: Claudio Imbrenda 

I forgot this:

Reported-by: Marc Hartmayer 

[...]

please add the reported-by when merging :)

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

2019-11-28 Thread Christian Borntraeger 



On 28.11.19 13:45, Cornelia Huck wrote:
> On Thu, 28 Nov 2019 13:35:29 +0100
> Christian Borntraeger  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 
>>> ---
>>>  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.
>

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

2019-11-28 Thread Cornelia Huck 
On Thu, 28 Nov 2019 13:35:29 +0100
Christian Borntraeger  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.

> 
> 
> 
> 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 
> > ---
> >  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.

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

2019-11-28 Thread Thomas Huth 

On 28/11/2019 13.35, Christian Borntraeger 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?


If we do another rc of 4.2, I think we definitely want this to be 
included, otherwise quite a bunch of things don't work anymore as 
expected, e.g. "-boot menu=on"...



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);
  }


I gave it a quick try, and this fixes "-boot menu=on" for me, so:

Tested-by: Thomas Huth

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

2019-11-28 Thread Cornelia Huck 
On Thu, 28 Nov 2019 14:11:38 +0100
Thomas Huth  wrote:

> On 28/11/2019 13.35, Christian Borntraeger 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?  
> 
> If we do another rc of 4.2, I think we definitely want this to be 
> included, otherwise quite a bunch of things don't work anymore as 
> expected, e.g. "-boot menu=on"...

I do agree we want this if possible; the question is really the
"possible" part...

> 
> >> 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);
> >>   }  
> 
> I gave it a quick try, and this fixes "-boot menu=on" for me, so:
> 
> Tested-by: Thomas Huth 

Thanks.

FWIW, I'm currently working to put this + the rebuild on my s390-fixes
branch.

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

2019-11-28 Thread Cornelia Huck 
On Thu, 28 Nov 2019 13:33:57 +0100
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 
> ---
>  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);
>  }

Acked-by: Cornelia Huck

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

2019-11-28 Thread Marc Hartmayer 
On Thu, Nov 28, 2019 at 01:33 PM +0100, 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 
> ---
>  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);
>  }
> -- 
> 2.7.4

Tested-by: Marc Hartmayer 

Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

2019-11-28 Thread Christian Borntraeger 



On 28.11.19 14:16, Cornelia Huck wrote:
> On Thu, 28 Nov 2019 14:11:38 +0100
> Thomas Huth  wrote:
> 
>> On 28/11/2019 13.35, Christian Borntraeger 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?  
>>
>> If we do another rc of 4.2, I think we definitely want this to be 
>> included, otherwise quite a bunch of things don't work anymore as 
>> expected, e.g. "-boot menu=on"...
> 
> I do agree we want this if possible; the question is really the
> "possible" part...


Given the issues that we see without that fix, I think this would qualify do
an rc4 (or even to apply it on top of rc3 to become 4.2)
Maybe just do a pull request?
> 
>>
 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);
   }  
>>
>> I gave it a quick try, and this fixes "-boot menu=on" for me, so:
>>
>> Tested-by: Thomas Huth 
> 
> Thanks.
> 
> FWIW, I'm currently working to put this + the rebuild on my s390-fixes
> branch.
>

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

2019-11-28 Thread Peter Maydell 
On Thu, 28 Nov 2019 at 12:48, Christian Borntraeger
 wrote:
>
>
>
> On 28.11.19 13:45, Cornelia Huck wrote:
> > On Thu, 28 Nov 2019 13:35:29 +0100
> > Christian Borntraeger  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.

There isn't currently anything else that would need an rc4.

thanks
-- PMM

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

2019-11-28 Thread Christian Borntraeger 



On 28.11.19 16:05, Peter Maydell wrote:
> On Thu, 28 Nov 2019 at 12:48, Christian Borntraeger
>  wrote:
>>
>>
>>
>> On 28.11.19 13:45, Cornelia Huck wrote:
>>> On Thu, 28 Nov 2019 13:35:29 +0100
>>> Christian Borntraeger  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.
> 
> There isn't currently anything else that would need an rc4.

I would vote for getting this patch still in. And I think we probably do not 
need an
rc4 for that, the fix seems pretty straight forward.

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

2019-11-29 Thread Thomas Huth 
On 29/11/2019 08.38, Christian Borntraeger wrote:
> 
> 
> On 28.11.19 16:05, Peter Maydell wrote:
>> On Thu, 28 Nov 2019 at 12:48, Christian Borntraeger
>>  wrote:
>>>
>>>
>>>
>>> On 28.11.19 13:45, Cornelia Huck wrote:
 On Thu, 28 Nov 2019 13:35:29 +0100
 Christian Borntraeger  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.
>>
>> There isn't currently anything else that would need an rc4.
> 
> I would vote for getting this patch still in. And I think we probably do not 
> need an
> rc4 for that, the fix seems pretty straight forward.

Ok, I'm going to build the binaries and send a pull request today.

 Thomas