Change in coreboot[master]: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in ...coreboot[master]:...

Krystian Hebel (Code Review)

19 Nov 2019 19 Nov '19

5:03 p.m.

Krystian Hebel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36976 )

Change subject: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss ......................................................................

vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss

This fixes issue that became visible after implementing post-CAR stage on top of `340e4b80904f lib/cbmem_top: Add a common cbmem_top implementation`. Compilation error was:

Forbidden global variables in romstage: ffffff00 d top.2205

Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db --- M src/vendorcode/amd/pi/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/36976/1

diff --git a/src/vendorcode/amd/pi/Makefile.inc b/src/vendorcode/amd/pi/Makefile.inc index 4e8787b..90714aa 100644 --- a/src/vendorcode/amd/pi/Makefile.inc +++ b/src/vendorcode/amd/pi/Makefile.inc @@ -68,7 +68,7 @@ AGESA_INC += -I$(src)/commonlib/include AGESA_INC += -I$(VBOOT_SOURCE)/firmware/include

-AGESA_CFLAGS += -march=amdfam10 -mno-3dnow -fno-zero-initialized-in-bss +AGESA_CFLAGS += -march=amdfam10 -mno-3dnow AGESA_CFLAGS += -fno-strict-aliasing -D__LIBAGESA__ CFLAGS_x86_32 += $(AGESA_CFLAGS) CFLAGS_x86_64 += $(AGESA_CFLAGS)

Attachments:

attachment.htm (text/html — 3.0 KB)

Show replies by date

Michał Żygowski (Code Review)

20 Nov 20 Nov

7:39 a.m.

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36976 )

Change subject: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss ......................................................................

Patch Set 1:

Arthur could you look at this? We had a small problem with postcar implementation on fam16h due to the unified cbmem_top implementation. Please confirm if this is the correct approach to solve it

-- To view, visit https://review.coreboot.org/c/coreboot/+/36976 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Gerrit-Change-Number: 36976 Gerrit-PatchSet: 1 Gerrit-Owner: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Wed, 20 Nov 2019 07:39:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

9:47 a.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36976 )

Change subject: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss ......................................................................

Patch Set 1:

It looks like things end up in the .illegal_globals in arch/x86/car.ld. I'm not sure if this is a correct solution.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36976 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Gerrit-Change-Number: 36976 Gerrit-PatchSet: 1 Gerrit-Owner: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Wed, 20 Nov 2019 09:47:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

10:12 a.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36976 )

Change subject: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss ......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/36976/1/src/vendorcode/amd/pi/Makef... File src/vendorcode/amd/pi/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36976/1/src/vendorcode/amd/pi/Makef... PS1, Line 71: -fno-zero-initialized-in-bss It looks like CAR is cleared indeed so that should be fine.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36976 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Gerrit-Change-Number: 36976 Gerrit-PatchSet: 1 Gerrit-Owner: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Wed, 20 Nov 2019 10:12:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

23 Nov 23 Nov

5:11 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36976 )

Change subject: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss ......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/36976/1/src/vendorcode/amd/pi/Makef... File src/vendorcode/amd/pi/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36976/1/src/vendorcode/amd/pi/Makef... PS1, Line 71: -fno-zero-initialized-in-bss

...

It looks like CAR is cleared indeed so that should be fine.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/36976 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Gerrit-Change-Number: 36976 Gerrit-PatchSet: 2 Gerrit-Owner: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 23 Nov 2019 17:11:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

5:19 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36976 )

Change subject: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss ......................................................................

Patch Set 2:

POSTCAR_STAGE=y => CAR_GLOBAL_MIGRATION=n => ENV_STAGE_HAS_BSS_SECTION=1 (for romstage)

=> MAYBE_STATIC_BSS becomes 'static'

But, with -no-zero-initialized-bss, zero-initialized static variables are .data, not .bss. And romstage .data is currently forbidden.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36976 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Gerrit-Change-Number: 36976 Gerrit-PatchSet: 2 Gerrit-Owner: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 23 Nov 2019 17:19:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

5:20 p.m.

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36976 )

Change subject: vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss ......................................................................

vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bss

This fixes issue that became visible after implementing post-CAR stage on top of `340e4b80904f lib/cbmem_top: Add a common cbmem_top implementation`. Compilation error was:

Forbidden global variables in romstage: ffffff00 d top.2205

Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Reviewed-on: https://review.coreboot.org/c/coreboot/+/36976 Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/vendorcode/amd/pi/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Arthur Heymans: Looks good to me, approved

-- To view, visit https://review.coreboot.org/c/coreboot/+/36976 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Gerrit-Change-Number: 36976 Gerrit-PatchSet: 3 Gerrit-Owner: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Krystian Hebel krystian.hebel@3mdeb.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1828

days inactive

1832

days old

coreboot-gerrit@coreboot.org

6 comments

4 participants

tags (0)

participants (4)

Arthur Heymans (Code Review)
Krystian Hebel (Code Review)
Kyösti Mälkki (Code Review)
Michał Żygowski (Code Review)