diff mbox

[qemu-s390x,for-2.11] pc-bios/s390-ccw: Fix problem with invalid virtio-scsi LUN when rebooting

Message ID e40a4f25-3384-f670-7cc9-ca6334213f1b@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Huth Nov. 20, 2017, 9:18 a.m. UTC
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:

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

Comments

Christian Borntraeger Nov. 20, 2017, 9:20 a.m. UTC | #1
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
>
Cornelia Huck Nov. 20, 2017, 9:26 a.m. UTC | #2
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.
diff mbox

Patch

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;