[Qemu-devel] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting

Thomas Huth posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1510942228-22822-1-git-send-email-thuth@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Thomas Huth 6 years, 5 months ago
When rebooting a guest that has a virtio-scsi disk, the s390-ccw
bios sometimes bails out with an error message like this:

! SCSI cannot report LUNs: STATUS=02 RSPN=70 KEY=05 CODE=25 QLFR=00, sure !

Enabling the scsi_req* tracing in QEMU shows that the ccw bios is
trying to execute the REPORT LUNS SCSI command with a LUN != 0, and
this causes the SCSI command to fail.
Looks like we neither clear the BSS of the s390-ccw bios during reboot,
nor do we explicitly set the default_scsi_device.lun value to 0, so
this variable can contain random values from the OS after the reboot.
By setting this variable explicitly to 0, the problem is fixed and
the reboots always succeed.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1514352
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
index c92f5d3..4fe4b9d 100644
--- a/pc-bios/s390-ccw/virtio-scsi.c
+++ b/pc-bios/s390-ccw/virtio-scsi.c
@@ -223,7 +223,8 @@ static void virtio_scsi_locate_device(VDev *vdev)
 
     for (target = 0; target <= vdev->config.scsi.max_target; target++) {
         sdev->channel = channel;
-        sdev->target = target; /* sdev->lun will be 0 here */
+        sdev->target = target;
+        sdev->lun = 0;          /* LUN has to be 0 for REPORT LUNS */
         if (!scsi_report_luns(vdev, data, sizeof(data))) {
             if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
                 continue;
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Christian Borntraeger 6 years, 5 months ago

On 11/17/2017 07:10 PM, Thomas Huth wrote:
> When rebooting a guest that has a virtio-scsi disk, the s390-ccw
> bios sometimes bails out with an error message like this:
> 
> ! SCSI cannot report LUNs: STATUS=02 RSPN=70 KEY=05 CODE=25 QLFR=00, sure !
> 
> Enabling the scsi_req* tracing in QEMU shows that the ccw bios is
> trying to execute the REPORT LUNS SCSI command with a LUN != 0, and
> this causes the SCSI command to fail.
> Looks like we neither clear the BSS of the s390-ccw bios during reboot,
> nor do we explicitly set the default_scsi_device.lun value to 0, so
> this variable can contain random values from the OS after the reboot.
> By setting this variable explicitly to 0, the problem is fixed and
> the reboots always succeed.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1514352
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

We had these things multile times in the past. The QEMU elf loader does not
zero the BSS (it just loads the load section).  Hmm, what about letting the 
BIOS zero its bss itself. IIRC the kernel does the same thing. I will have a
look into that for 2.12.

PS: Please do not forget to rebuild the bios
> ---
>  pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> index c92f5d3..4fe4b9d 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.c
> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> @@ -223,7 +223,8 @@ static void virtio_scsi_locate_device(VDev *vdev)
> 
>      for (target = 0; target <= vdev->config.scsi.max_target; target++) {
>          sdev->channel = channel;
> -        sdev->target = target; /* sdev->lun will be 0 here */
> +        sdev->target = target;
> +        sdev->lun = 0;          /* LUN has to be 0 for REPORT LUNS */
>          if (!scsi_report_luns(vdev, data, sizeof(data))) {
>              if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
>                  continue;
> 


Re: [Qemu-devel] [qemu-s390x] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Christian Borntraeger 6 years, 4 months ago
Thomas,

does this patch help as well?

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 6d0c2ee..2687590 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -12,7 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
 OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
 QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
-QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
+QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing -fno-zero-initialized-in-bss
 QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
 LDFLAGS += -Wl,-pie -nostdlib
 



On 11/17/2017 07:45 PM, Christian Borntraeger wrote:
> 
> 
> On 11/17/2017 07:10 PM, Thomas Huth wrote:
>> When rebooting a guest that has a virtio-scsi disk, the s390-ccw
>> bios sometimes bails out with an error message like this:
>>
>> ! SCSI cannot report LUNs: STATUS=02 RSPN=70 KEY=05 CODE=25 QLFR=00, sure !
>>
>> Enabling the scsi_req* tracing in QEMU shows that the ccw bios is
>> trying to execute the REPORT LUNS SCSI command with a LUN != 0, and
>> this causes the SCSI command to fail.
>> Looks like we neither clear the BSS of the s390-ccw bios during reboot,
>> nor do we explicitly set the default_scsi_device.lun value to 0, so
>> this variable can contain random values from the OS after the reboot.
>> By setting this variable explicitly to 0, the problem is fixed and
>> the reboots always succeed.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1514352
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> We had these things multile times in the past. The QEMU elf loader does not
> zero the BSS (it just loads the load section).  Hmm, what about letting the 
> BIOS zero its bss itself. IIRC the kernel does the same thing. I will have a
> look into that for 2.12.
> 
> PS: Please do not forget to rebuild the bios
>> ---
>>  pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
>> index c92f5d3..4fe4b9d 100644
>> --- a/pc-bios/s390-ccw/virtio-scsi.c
>> +++ b/pc-bios/s390-ccw/virtio-scsi.c
>> @@ -223,7 +223,8 @@ static void virtio_scsi_locate_device(VDev *vdev)
>>
>>      for (target = 0; target <= vdev->config.scsi.max_target; target++) {
>>          sdev->channel = channel;
>> -        sdev->target = target; /* sdev->lun will be 0 here */
>> +        sdev->target = target;
>> +        sdev->lun = 0;          /* LUN has to be 0 for REPORT LUNS */
>>          if (!scsi_report_luns(vdev, data, sizeof(data))) {
>>              if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
>>                  continue;
>>
> 
> 


Re: [Qemu-devel] [qemu-s390x] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Cornelia Huck 6 years, 4 months ago
On Mon, 20 Nov 2017 09:48:36 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Thomas,
> 
> does this patch help as well?
> 
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index 6d0c2ee..2687590 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -12,7 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
>  OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
>  QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
>  QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
> -QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
> +QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing -fno-zero-initialized-in-bss
>  QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
>  LDFLAGS += -Wl,-pie -nostdlib

If this would actually enable us to kill a whole bird swarm with one
stone, I'd prefer it over the patch below, nicely short though it is.


I plan to pack the one or the other into s390-fixes today and rebuild.

>  
> 
> 
> 
> On 11/17/2017 07:45 PM, Christian Borntraeger wrote:
> > 
> > 
> > On 11/17/2017 07:10 PM, Thomas Huth wrote:  
> >> When rebooting a guest that has a virtio-scsi disk, the s390-ccw
> >> bios sometimes bails out with an error message like this:
> >>
> >> ! SCSI cannot report LUNs: STATUS=02 RSPN=70 KEY=05 CODE=25 QLFR=00, sure !
> >>
> >> Enabling the scsi_req* tracing in QEMU shows that the ccw bios is
> >> trying to execute the REPORT LUNS SCSI command with a LUN != 0, and
> >> this causes the SCSI command to fail.
> >> Looks like we neither clear the BSS of the s390-ccw bios during reboot,
> >> nor do we explicitly set the default_scsi_device.lun value to 0, so
> >> this variable can contain random values from the OS after the reboot.
> >> By setting this variable explicitly to 0, the problem is fixed and
> >> the reboots always succeed.
> >>
> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1514352
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>  
> > 
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > 
> > We had these things multile times in the past. The QEMU elf loader does not
> > zero the BSS (it just loads the load section).  Hmm, what about letting the 
> > BIOS zero its bss itself. IIRC the kernel does the same thing. I will have a
> > look into that for 2.12.
> > 
> > PS: Please do not forget to rebuild the bios  
> >> ---
> >>  pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> >> index c92f5d3..4fe4b9d 100644
> >> --- a/pc-bios/s390-ccw/virtio-scsi.c
> >> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> >> @@ -223,7 +223,8 @@ static void virtio_scsi_locate_device(VDev *vdev)
> >>
> >>      for (target = 0; target <= vdev->config.scsi.max_target; target++) {
> >>          sdev->channel = channel;
> >> -        sdev->target = target; /* sdev->lun will be 0 here */
> >> +        sdev->target = target;
> >> +        sdev->lun = 0;          /* LUN has to be 0 for REPORT LUNS */
> >>          if (!scsi_report_luns(vdev, data, sizeof(data))) {
> >>              if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
> >>                  continue;
> >>  
> > 
> >   
> 


Re: [Qemu-devel] [qemu-s390x] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Christian Borntraeger 6 years, 4 months ago

On 11/20/2017 10:00 AM, Cornelia Huck wrote:
> On Mon, 20 Nov 2017 09:48:36 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> Thomas,
>>
>> does this patch help as well?
>>
>> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
>> index 6d0c2ee..2687590 100644
>> --- a/pc-bios/s390-ccw/Makefile
>> +++ b/pc-bios/s390-ccw/Makefile
>> @@ -12,7 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
>>  OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
>>  QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
>>  QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
>> -QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
>> +QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing -fno-zero-initialized-in-bss
>>  QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
>>  LDFLAGS += -Wl,-pie -nostdlib
> 
> If this would actually enable us to kill a whole bird swarm with one
> stone, I'd prefer it over the patch below, nicely short though it is.

I will resend with proper patch description so that Thomas can comment on a proper
patch.


> 
> 
> I plan to pack the one or the other into s390-fixes today and rebuild.
> 
>>  
>>
>>
>>
>> On 11/17/2017 07:45 PM, Christian Borntraeger wrote:
>>>
>>>
>>> On 11/17/2017 07:10 PM, Thomas Huth wrote:  
>>>> When rebooting a guest that has a virtio-scsi disk, the s390-ccw
>>>> bios sometimes bails out with an error message like this:
>>>>
>>>> ! SCSI cannot report LUNs: STATUS=02 RSPN=70 KEY=05 CODE=25 QLFR=00, sure !
>>>>
>>>> Enabling the scsi_req* tracing in QEMU shows that the ccw bios is
>>>> trying to execute the REPORT LUNS SCSI command with a LUN != 0, and
>>>> this causes the SCSI command to fail.
>>>> Looks like we neither clear the BSS of the s390-ccw bios during reboot,
>>>> nor do we explicitly set the default_scsi_device.lun value to 0, so
>>>> this variable can contain random values from the OS after the reboot.
>>>> By setting this variable explicitly to 0, the problem is fixed and
>>>> the reboots always succeed.
>>>>
>>>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1514352
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>  
>>>
>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>
>>> We had these things multile times in the past. The QEMU elf loader does not
>>> zero the BSS (it just loads the load section).  Hmm, what about letting the 
>>> BIOS zero its bss itself. IIRC the kernel does the same thing. I will have a
>>> look into that for 2.12.
>>>
>>> PS: Please do not forget to rebuild the bios  
>>>> ---
>>>>  pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
>>>> index c92f5d3..4fe4b9d 100644
>>>> --- a/pc-bios/s390-ccw/virtio-scsi.c
>>>> +++ b/pc-bios/s390-ccw/virtio-scsi.c
>>>> @@ -223,7 +223,8 @@ static void virtio_scsi_locate_device(VDev *vdev)
>>>>
>>>>      for (target = 0; target <= vdev->config.scsi.max_target; target++) {
>>>>          sdev->channel = channel;
>>>> -        sdev->target = target; /* sdev->lun will be 0 here */
>>>> +        sdev->target = target;
>>>> +        sdev->lun = 0;          /* LUN has to be 0 for REPORT LUNS */
>>>>          if (!scsi_report_luns(vdev, data, sizeof(data))) {
>>>>              if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
>>>>                  continue;
>>>>  
>>>
>>>   
>>
> 


Re: [Qemu-devel] [qemu-s390x] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Thomas Huth 6 years, 4 months ago
On 20.11.2017 09:48, Christian Borntraeger wrote:
> Thomas,
> 
> does this patch help as well?
> 
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index 6d0c2ee..2687590 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -12,7 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
>  OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
>  QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
>  QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
> -QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
> +QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing -fno-zero-initialized-in-bss
>  QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
>  LDFLAGS += -Wl,-pie -nostdlib

No, that alone does not help, the default_scsi_device variable is still
in the normal .bss section in that case (you can also check that with
objdump for example). You'd have to apply this patch on top to fix it
that way:

diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
index c92f5d3..64ab095 100644
--- a/pc-bios/s390-ccw/virtio-scsi.c
+++ b/pc-bios/s390-ccw/virtio-scsi.c
@@ -15,7 +15,7 @@
 #include "scsi.h"
 #include "virtio-scsi.h"

-static ScsiDevice default_scsi_device;
+static ScsiDevice default_scsi_device = { 0 };
 static VirtioScsiCmdReq req;
 static VirtioScsiCmdResp resp;

... then the variable is in the .data section instead.

But since the problem with other uninitialized .bss variables is then
still pending, I think we should rather go with my patch for 2.11 and
fix future problems properly in v2.12 by initializing the complete .bss
to zero in the start.S file.

 Thomas

Re: [Qemu-devel] [qemu-s390x] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Christian Borntraeger 6 years, 4 months ago

On 11/20/2017 10:18 AM, Thomas Huth wrote:
> On 20.11.2017 09:48, Christian Borntraeger wrote:
>> Thomas,
>>
>> does this patch help as well?
>>
>> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
>> index 6d0c2ee..2687590 100644
>> --- a/pc-bios/s390-ccw/Makefile
>> +++ b/pc-bios/s390-ccw/Makefile
>> @@ -12,7 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
>>  OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
>>  QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
>>  QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
>> -QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
>> +QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing -fno-zero-initialized-in-bss
>>  QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
>>  LDFLAGS += -Wl,-pie -nostdlib
> 
> No, that alone does not help, the default_scsi_device variable is still
> in the normal .bss section in that case (you can also check that with
> objdump for example). You'd have to apply this patch on top to fix it
> that way:
> 
> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> index c92f5d3..64ab095 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.c
> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> @@ -15,7 +15,7 @@
>  #include "scsi.h"
>  #include "virtio-scsi.h"
> 
> -static ScsiDevice default_scsi_device;
> +static ScsiDevice default_scsi_device = { 0 };
>  static VirtioScsiCmdReq req;
>  static VirtioScsiCmdResp resp;
> 
> ... then the variable is in the .data section instead.
> 
> But since the problem with other uninitialized .bss variables is then
> still pending, I think we should rather go with my patch for 2.11 and
> fix future problems properly in v2.12 by initializing the complete .bss
> to zero in the start.S file.

Yes. Ack.
> 
>  Thomas
> 


Re: [Qemu-devel] [qemu-s390x] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Cornelia Huck 6 years, 4 months ago
On Mon, 20 Nov 2017 10:18:38 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 20.11.2017 09:48, Christian Borntraeger wrote:
> > Thomas,
> > 
> > does this patch help as well?
> > 
> > diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> > index 6d0c2ee..2687590 100644
> > --- a/pc-bios/s390-ccw/Makefile
> > +++ b/pc-bios/s390-ccw/Makefile
> > @@ -12,7 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
> >  OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
> >  QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
> >  QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
> > -QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
> > +QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing -fno-zero-initialized-in-bss
> >  QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
> >  LDFLAGS += -Wl,-pie -nostdlib  
> 
> No, that alone does not help, the default_scsi_device variable is still
> in the normal .bss section in that case (you can also check that with
> objdump for example). You'd have to apply this patch on top to fix it
> that way:
> 
> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> index c92f5d3..64ab095 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.c
> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> @@ -15,7 +15,7 @@
>  #include "scsi.h"
>  #include "virtio-scsi.h"
> 
> -static ScsiDevice default_scsi_device;
> +static ScsiDevice default_scsi_device = { 0 };
>  static VirtioScsiCmdReq req;
>  static VirtioScsiCmdResp resp;
> 
> ... then the variable is in the .data section instead.
> 
> But since the problem with other uninitialized .bss variables is then
> still pending, I think we should rather go with my patch for 2.11 and
> fix future problems properly in v2.12 by initializing the complete .bss
> to zero in the start.S file.

OK, I'll go with that. We can (should) invest more cycles for fixing
the whole class of problems for the next release.

Re: [Qemu-devel] [qemu-s390x] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by David Hildenbrand 6 years, 4 months ago
On 17.11.2017 19:10, Thomas Huth wrote:
> When rebooting a guest that has a virtio-scsi disk, the s390-ccw
> bios sometimes bails out with an error message like this:
> 
> ! SCSI cannot report LUNs: STATUS=02 RSPN=70 KEY=05 CODE=25 QLFR=00, sure !
> 
> Enabling the scsi_req* tracing in QEMU shows that the ccw bios is
> trying to execute the REPORT LUNS SCSI command with a LUN != 0, and
> this causes the SCSI command to fail.
> Looks like we neither clear the BSS of the s390-ccw bios during reboot,
> nor do we explicitly set the default_scsi_device.lun value to 0, so
> this variable can contain random values from the OS after the reboot.
> By setting this variable explicitly to 0, the problem is fixed and
> the reboots always succeed.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1514352
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> index c92f5d3..4fe4b9d 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.c
> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> @@ -223,7 +223,8 @@ static void virtio_scsi_locate_device(VDev *vdev)
>  
>      for (target = 0; target <= vdev->config.scsi.max_target; target++) {
>          sdev->channel = channel;
> -        sdev->target = target; /* sdev->lun will be 0 here */
> +        sdev->target = target;
> +        sdev->lun = 0;          /* LUN has to be 0 for REPORT LUNS */
>          if (!scsi_report_luns(vdev, data, sizeof(data))) {
>              if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
>                  continue;
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting
Posted by Cornelia Huck 6 years, 4 months ago
On Fri, 17 Nov 2017 19:10:28 +0100
Thomas Huth <thuth@redhat.com> wrote:

> When rebooting a guest that has a virtio-scsi disk, the s390-ccw
> bios sometimes bails out with an error message like this:
> 
> ! SCSI cannot report LUNs: STATUS=02 RSPN=70 KEY=05 CODE=25 QLFR=00, sure !
> 
> Enabling the scsi_req* tracing in QEMU shows that the ccw bios is
> trying to execute the REPORT LUNS SCSI command with a LUN != 0, and
> this causes the SCSI command to fail.
> Looks like we neither clear the BSS of the s390-ccw bios during reboot,
> nor do we explicitly set the default_scsi_device.lun value to 0, so
> this variable can contain random values from the OS after the reboot.
> By setting this variable explicitly to 0, the problem is fixed and
> the reboots always succeed.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1514352
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  pc-bios/s390-ccw/virtio-scsi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> index c92f5d3..4fe4b9d 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.c
> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> @@ -223,7 +223,8 @@ static void virtio_scsi_locate_device(VDev *vdev)
>  
>      for (target = 0; target <= vdev->config.scsi.max_target; target++) {
>          sdev->channel = channel;
> -        sdev->target = target; /* sdev->lun will be 0 here */
> +        sdev->target = target;
> +        sdev->lun = 0;          /* LUN has to be 0 for REPORT LUNS */
>          if (!scsi_report_luns(vdev, data, sizeof(data))) {
>              if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
>                  continue;

Thanks, applied + rebuilt the bios and pushed to s390-fixes.