* [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
@ 2017-11-20 9:15 Christian Borntraeger
2017-11-20 9:19 ` Alexander Graf
2017-11-20 9:20 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
0 siblings, 2 replies; 11+ messages in thread
From: Christian Borntraeger @ 2017-11-20 9:15 UTC (permalink / raw)
To: Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Alexander Graf,
Richard Henderson, Thomas Huth, Christian Borntraeger
The QEMU ELF loader does not initialize the bss segment. This has
triggered several bugs in the past, e.g. see commit 5d739a4787a5
("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
css").
Instead of fixing these things one-by-one we can build the BIOS
with -fno-zero-initialized-in-bss. This will move the zero variables
also into the data segment, which is then part of a LOAD section.
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
pc-bios/s390-ccw/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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
--
2.9.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 9:15 [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss Christian Borntraeger
@ 2017-11-20 9:19 ` Alexander Graf
2017-11-20 9:27 ` Christian Borntraeger
2017-11-20 9:20 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
1 sibling, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2017-11-20 9:19 UTC (permalink / raw)
To: Christian Borntraeger, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
> The QEMU ELF loader does not initialize the bss segment. This has
> triggered several bugs in the past, e.g. see commit 5d739a4787a5
> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
> css").
>
> Instead of fixing these things one-by-one we can build the BIOS
> with -fno-zero-initialized-in-bss. This will move the zero variables
> also into the data segment, which is then part of a LOAD section.
Doesn't this bloat the firmware? Why don't we just manually clear bss in
the firmware itself? It's what all other firmwares do :)
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 9:19 ` Alexander Graf
@ 2017-11-20 9:27 ` Christian Borntraeger
2017-11-20 9:29 ` Alexander Graf
0 siblings, 1 reply; 11+ messages in thread
From: Christian Borntraeger @ 2017-11-20 9:27 UTC (permalink / raw)
To: Alexander Graf, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 10:19 AM, Alexander Graf wrote:
> On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
>> The QEMU ELF loader does not initialize the bss segment. This has
>> triggered several bugs in the past, e.g. see commit 5d739a4787a5
>> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
>> css").
>>
>> Instead of fixing these things one-by-one we can build the BIOS
>> with -fno-zero-initialized-in-bss. This will move the zero variables
>> also into the data segment, which is then part of a LOAD section.
>
> Doesn't this bloat the firmware? Why don't we just manually clear bss in the firmware itself? It's what all other firmwares do :)
Yes the proper fix is to initialize bss in the bios itself.
I was trying to come up with something for 2.11, but since the patch does not solve the original issues, lets drop it.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 9:27 ` Christian Borntraeger
@ 2017-11-20 9:29 ` Alexander Graf
2017-11-20 10:02 ` Christian Borntraeger
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2017-11-20 9:29 UTC (permalink / raw)
To: Christian Borntraeger, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 10:27 AM, Christian Borntraeger wrote:
>
> On 11/20/2017 10:19 AM, Alexander Graf wrote:
>> On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
>>> The QEMU ELF loader does not initialize the bss segment. This has
>>> triggered several bugs in the past, e.g. see commit 5d739a4787a5
>>> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
>>> css").
>>>
>>> Instead of fixing these things one-by-one we can build the BIOS
>>> with -fno-zero-initialized-in-bss. This will move the zero variables
>>> also into the data segment, which is then part of a LOAD section.
>> Doesn't this bloat the firmware? Why don't we just manually clear bss in the firmware itself? It's what all other firmwares do :)
> Yes the proper fix is to initialize bss in the bios itself.
> I was trying to come up with something for 2.11, but since the patch does not solve the original issues, lets drop it.
Initializing bss is quite simple. You can probably even do it from C.
Just set two variables before and after .bss in the linker script and
memset(0) from start to end :).
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 9:29 ` Alexander Graf
@ 2017-11-20 10:02 ` Christian Borntraeger
2017-11-20 10:16 ` Alexander Graf
0 siblings, 1 reply; 11+ messages in thread
From: Christian Borntraeger @ 2017-11-20 10:02 UTC (permalink / raw)
To: Alexander Graf, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 10:29 AM, Alexander Graf wrote:
> On 11/20/2017 10:27 AM, Christian Borntraeger wrote:
>>
>> On 11/20/2017 10:19 AM, Alexander Graf wrote:
>>> On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
>>>> The QEMU ELF loader does not initialize the bss segment. This has
>>>> triggered several bugs in the past, e.g. see commit 5d739a4787a5
>>>> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
>>>> css").
>>>>
>>>> Instead of fixing these things one-by-one we can build the BIOS
>>>> with -fno-zero-initialized-in-bss. This will move the zero variables
>>>> also into the data segment, which is then part of a LOAD section.
>>> Doesn't this bloat the firmware? Why don't we just manually clear bss in the firmware itself? It's what all other firmwares do :)
>> Yes the proper fix is to initialize bss in the bios itself.
>> I was trying to come up with something for 2.11, but since the patch does not solve the original issues, lets drop it.
>
>
> Initializing bss is quite simple. You can probably even do it from C. Just set two variables before and after .bss in the linker script and memset(0) from start to end :).
Yes, I know but then we have to change the build process to use a linker script.(we rely on the default
linker script right now).
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 10:02 ` Christian Borntraeger
@ 2017-11-20 10:16 ` Alexander Graf
2017-11-20 10:19 ` Christian Borntraeger
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2017-11-20 10:16 UTC (permalink / raw)
To: Christian Borntraeger, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 11:02 AM, Christian Borntraeger wrote:
>
> On 11/20/2017 10:29 AM, Alexander Graf wrote:
>> On 11/20/2017 10:27 AM, Christian Borntraeger wrote:
>>> On 11/20/2017 10:19 AM, Alexander Graf wrote:
>>>> On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
>>>>> The QEMU ELF loader does not initialize the bss segment. This has
>>>>> triggered several bugs in the past, e.g. see commit 5d739a4787a5
>>>>> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
>>>>> css").
>>>>>
>>>>> Instead of fixing these things one-by-one we can build the BIOS
>>>>> with -fno-zero-initialized-in-bss. This will move the zero variables
>>>>> also into the data segment, which is then part of a LOAD section.
>>>> Doesn't this bloat the firmware? Why don't we just manually clear bss in the firmware itself? It's what all other firmwares do :)
>>> Yes the proper fix is to initialize bss in the bios itself.
>>> I was trying to come up with something for 2.11, but since the patch does not solve the original issues, lets drop it.
>>
>> Initializing bss is quite simple. You can probably even do it from C. Just set two variables before and after .bss in the linker script and memset(0) from start to end :).
> Yes, I know but then we have to change the build process to use a linker script.(we rely on the default
> linker script right now).
I'm not sure how common generic linker scripts are, but in our default
script we have existing markers for bss and end. You can look at the
default linker script using gcc <c file> -Wl,-verbose. This simple
source worked for me:
#include <stdio.h>
extern char __bss_start[];
extern char _end[];
int main(void)
{
printf("BSS size: %lx\n", (long)_end - (long)__bss_start);
return 0;
}
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 10:16 ` Alexander Graf
@ 2017-11-20 10:19 ` Christian Borntraeger
2017-11-20 10:24 ` Alexander Graf
2017-11-20 13:13 ` Richard Henderson
0 siblings, 2 replies; 11+ messages in thread
From: Christian Borntraeger @ 2017-11-20 10:19 UTC (permalink / raw)
To: Alexander Graf, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 11:16 AM, Alexander Graf wrote:
> On 11/20/2017 11:02 AM, Christian Borntraeger wrote:
>>
>> On 11/20/2017 10:29 AM, Alexander Graf wrote:
>>> On 11/20/2017 10:27 AM, Christian Borntraeger wrote:
>>>> On 11/20/2017 10:19 AM, Alexander Graf wrote:
>>>>> On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
>>>>>> The QEMU ELF loader does not initialize the bss segment. This has
>>>>>> triggered several bugs in the past, e.g. see commit 5d739a4787a5
>>>>>> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
>>>>>> css").
>>>>>>
>>>>>> Instead of fixing these things one-by-one we can build the BIOS
>>>>>> with -fno-zero-initialized-in-bss. This will move the zero variables
>>>>>> also into the data segment, which is then part of a LOAD section.
>>>>> Doesn't this bloat the firmware? Why don't we just manually clear bss in the firmware itself? It's what all other firmwares do :)
>>>> Yes the proper fix is to initialize bss in the bios itself.
>>>> I was trying to come up with something for 2.11, but since the patch does not solve the original issues, lets drop it.
>>>
>>> Initializing bss is quite simple. You can probably even do it from C. Just set two variables before and after .bss in the linker script and memset(0) from start to end :).
>> Yes, I know but then we have to change the build process to use a linker script.(we rely on the default
>> linker script right now).
>
>
> I'm not sure how common generic linker scripts are, but in our default script we have existing markers for bss and end. You can look at the default linker script using gcc <c file> -Wl,-verbose. This simple source worked for me:
Are we sure that the range between __bss_start and _end does not include other elements (besides bss)?
>
>
> #include <stdio.h>
>
> extern char __bss_start[];
> extern char _end[];
>
> int main(void)
> {
> printf("BSS size: %lx\n", (long)_end - (long)__bss_start);
> return 0;
> }
>
>
>
> Alex
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 10:19 ` Christian Borntraeger
@ 2017-11-20 10:24 ` Alexander Graf
2017-11-20 10:27 ` Christian Borntraeger
2017-11-20 13:13 ` Richard Henderson
1 sibling, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2017-11-20 10:24 UTC (permalink / raw)
To: Christian Borntraeger, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 11:19 AM, Christian Borntraeger wrote:
>
> On 11/20/2017 11:16 AM, Alexander Graf wrote:
>> On 11/20/2017 11:02 AM, Christian Borntraeger wrote:
>>> On 11/20/2017 10:29 AM, Alexander Graf wrote:
>>>> On 11/20/2017 10:27 AM, Christian Borntraeger wrote:
>>>>> On 11/20/2017 10:19 AM, Alexander Graf wrote:
>>>>>> On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
>>>>>>> The QEMU ELF loader does not initialize the bss segment. This has
>>>>>>> triggered several bugs in the past, e.g. see commit 5d739a4787a5
>>>>>>> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
>>>>>>> css").
>>>>>>>
>>>>>>> Instead of fixing these things one-by-one we can build the BIOS
>>>>>>> with -fno-zero-initialized-in-bss. This will move the zero variables
>>>>>>> also into the data segment, which is then part of a LOAD section.
>>>>>> Doesn't this bloat the firmware? Why don't we just manually clear bss in the firmware itself? It's what all other firmwares do :)
>>>>> Yes the proper fix is to initialize bss in the bios itself.
>>>>> I was trying to come up with something for 2.11, but since the patch does not solve the original issues, lets drop it.
>>>> Initializing bss is quite simple. You can probably even do it from C. Just set two variables before and after .bss in the linker script and memset(0) from start to end :).
>>> Yes, I know but then we have to change the build process to use a linker script.(we rely on the default
>>> linker script right now).
>>
>> I'm not sure how common generic linker scripts are, but in our default script we have existing markers for bss and end. You can look at the default linker script using gcc <c file> -Wl,-verbose. This simple source worked for me:
> Are we sure that the range between __bss_start and _end does not include other elements (besides bss)?
It seems to be the intended semantic for the linker script I see as
default, but I'm not an expert here :). Can you check with Uli?
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 10:24 ` Alexander Graf
@ 2017-11-20 10:27 ` Christian Borntraeger
0 siblings, 0 replies; 11+ messages in thread
From: Christian Borntraeger @ 2017-11-20 10:27 UTC (permalink / raw)
To: Alexander Graf, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Richard Henderson,
Thomas Huth
On 11/20/2017 11:24 AM, Alexander Graf wrote:
> On 11/20/2017 11:19 AM, Christian Borntraeger wrote:
>>
>> On 11/20/2017 11:16 AM, Alexander Graf wrote:
>>> On 11/20/2017 11:02 AM, Christian Borntraeger wrote:
>>>> On 11/20/2017 10:29 AM, Alexander Graf wrote:
>>>>> On 11/20/2017 10:27 AM, Christian Borntraeger wrote:
>>>>>> On 11/20/2017 10:19 AM, Alexander Graf wrote:
>>>>>>> On 11/20/2017 10:15 AM, Christian Borntraeger wrote:
>>>>>>>> The QEMU ELF loader does not initialize the bss segment. This has
>>>>>>>> triggered several bugs in the past, e.g. see commit 5d739a4787a5
>>>>>>>> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
>>>>>>>> css").
>>>>>>>>
>>>>>>>> Instead of fixing these things one-by-one we can build the BIOS
>>>>>>>> with -fno-zero-initialized-in-bss. This will move the zero variables
>>>>>>>> also into the data segment, which is then part of a LOAD section.
>>>>>>> Doesn't this bloat the firmware? Why don't we just manually clear bss in the firmware itself? It's what all other firmwares do :)
>>>>>> Yes the proper fix is to initialize bss in the bios itself.
>>>>>> I was trying to come up with something for 2.11, but since the patch does not solve the original issues, lets drop it.
>>>>> Initializing bss is quite simple. You can probably even do it from C. Just set two variables before and after .bss in the linker script and memset(0) from start to end :).
>>>> Yes, I know but then we have to change the build process to use a linker script.(we rely on the default
>>>> linker script right now).
>>>
>>> I'm not sure how common generic linker scripts are, but in our default script we have existing markers for bss and end. You can look at the default linker script using gcc <c file> -Wl,-verbose. This simple source worked for me:
>> Are we sure that the range between __bss_start and _end does not include other elements (besides bss)?
>
> It seems to be the intended semantic for the linker script I see as default, but I'm not an expert here :). Can you check with Uli?
I will.
But the "I am not sure" is exactly the reason why I prefer Thomas "band-aid" for 2.11.
Doing the bss zeroing is certainly my preferred long term solution (and
it was my first comment to Thomas' patch)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 10:19 ` Christian Borntraeger
2017-11-20 10:24 ` Alexander Graf
@ 2017-11-20 13:13 ` Richard Henderson
1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2017-11-20 13:13 UTC (permalink / raw)
To: Christian Borntraeger, Alexander Graf, Cornelia Huck
Cc: qemu-devel, qemu-s390x, Halil Pasic, Thomas Huth
On 11/20/2017 11:19 AM, Christian Borntraeger wrote:
> Are we sure that the range between __bss_start and _end does not include
> other elements (besides bss)?
Yes, we are sure.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss
2017-11-20 9:15 [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss Christian Borntraeger
2017-11-20 9:19 ` Alexander Graf
@ 2017-11-20 9:20 ` Thomas Huth
1 sibling, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2017-11-20 9:20 UTC (permalink / raw)
To: Christian Borntraeger, Cornelia Huck
Cc: Alexander Graf, Halil Pasic, qemu-devel, qemu-s390x,
Richard Henderson
On 20.11.2017 10:15, Christian Borntraeger wrote:
> The QEMU ELF loader does not initialize the bss segment. This has
> triggered several bugs in the past, e.g. see commit 5d739a4787a5
> ("s390-ccw.img: Fix sporadic errors with ccw boot image - initialize
> css").
>
> Instead of fixing these things one-by-one we can build the BIOS
> with -fno-zero-initialized-in-bss. This will move the zero variables
> also into the data segment, which is then part of a LOAD section.
It fixes the problem only for variables that are explicitely intialized
to 0 along with their declaration - it does not fix the problem for
uninitialized variables.
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-11-20 13:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-20 9:15 [Qemu-devel] [PATCH] pc-bios/s390-ccw: build s390 bios with -fno-zero-initialized-in-bss Christian Borntraeger
2017-11-20 9:19 ` Alexander Graf
2017-11-20 9:27 ` Christian Borntraeger
2017-11-20 9:29 ` Alexander Graf
2017-11-20 10:02 ` Christian Borntraeger
2017-11-20 10:16 ` Alexander Graf
2017-11-20 10:19 ` Christian Borntraeger
2017-11-20 10:24 ` Alexander Graf
2017-11-20 10:27 ` Christian Borntraeger
2017-11-20 13:13 ` Richard Henderson
2017-11-20 9:20 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).