Change in coreboot[master]: arch/x86: Top-align .text in bootblock

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Kyösti Mälkki (Code Review)

24 Nov 2020 24 Nov '20

7:03 p.m.

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

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

arch/x86: Top-align .text in bootblock

Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/Kconfig M src/arch/x86/bootblock.ld 2 files changed, 9 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/47971/1

diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index f4ddd20..1ec0390 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -184,6 +184,9 @@ hex default 0x10000

+config FIXED_BOOTBLOCK_SIZE + bool + # Default address romstage is to be linked at config ROMSTAGE_ADDR hex diff --git a/src/arch/x86/bootblock.ld b/src/arch/x86/bootblock.ld index bb22413..52bccfc 100644 --- a/src/arch/x86/bootblock.ld +++ b/src/arch/x86/bootblock.ld @@ -14,6 +14,11 @@ SECTIONS { . = _ebootblock - CONFIG_C_ENV_BOOTBLOCK_SIZE;

+#if !CONFIG(FIXED_BOOTBLOCK_SIZE) + . = BOOTBLOCK_TOP - PROGRAM_SZ; + . = ALIGN(16); +#endif + _bootblock = .;

INCLUDE "bootblock/lib/program.ld" @@ -32,6 +37,7 @@ * may cause the total size of a section to change when the start * address gets applied. */ + PROGRAM_SZ = SIZEOF(.text) + 32; EARLYASM_SZ = SIZEOF(.init) + (CONFIG(SIPI_VECTOR_IN_ROM) ? 4096 : 16);

. = ID_SECTION;

Attachments:

attachment.htm (text/html — 3.6 KB)

Show replies by date

Kyösti Mälkki (Code Review)

25 Nov 25 Nov

3:31 a.m.

Hello build bot (Jenkins), Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/47971

to look at the new patch set (#2).

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

arch/x86: Top-align .text in bootblock

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/47971/2

Arthur Heymans (Code Review)

11:42 a.m.

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

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

Patch Set 2: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 25 Nov 2020 11:42:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

9 Dec 9 Dec

11:03 a.m.

Hello build bot (Jenkins), Julius Werner, Arthur Heymans,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/47971

to look at the new patch set (#6).

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

arch/x86: Top-align .text in bootblock

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/47971/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 6 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Kyösti Mälkki (Code Review)

4:19 p.m.

Hello build bot (Jenkins), Julius Werner, Arthur Heymans,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/47971

to look at the new patch set (#7).

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

arch/x86: Top-align .text in bootblock

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/47971/7

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 7 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Arthur Heymans (Code Review)

15 Dec 15 Dec

7:11 a.m.

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

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

Patch Set 8: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 8 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 15 Dec 2020 07:11:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans (Code Review)

11 Jan 11 Jan

7:58 p.m.

Attention is currently required from: Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47971 )

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

Patch Set 10: -Code-Review

(1 comment)

File src/arch/x86/bootblock.ld:

https://review.coreboot.org/c/coreboot/+/47971/comment/f2d41f08_7025dc92 PS10, Line 17: #if !CONFIG(FIXED_BOOTBLOCK_SIZE) : . = BOOTBLOCK_TOP - PROGRAM_SZ; : . = ALIGN(16); : #endif This soverrides the 'sanity check' on the bootblock size, that it remains below CONFIG_C_ENV_BOOTBLOCK_SIZE (as required on some platforms: Intel/APL, Qemu/i440fx)

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 10 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Mon, 11 Jan 2021 19:58:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

27 Jan 27 Jan

10:12 p.m.

Hello build bot (Jenkins), Julius Werner, Arthur Heymans,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/47971

to look at the new patch set (#11).

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

arch/x86: Top-align .text in bootblock

Move .text section closer to .init. This reduces the size of the flat bootblock binary and footprint in CBFS.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/47971/11

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 11 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Arthur Heymans (Code Review)

28 Jan 28 Jan

8:33 a.m.

Attention is currently required from: Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47971 )

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

Patch Set 11: Code-Review+2

(1 comment)

File src/arch/x86/bootblock.ld:

https://review.coreboot.org/c/coreboot/+/47971/comment/25f89ec7_3431de66 PS10, Line 17: #if !CONFIG(FIXED_BOOTBLOCK_SIZE) : . = BOOTBLOCK_TOP - PROGRAM_SZ; : . = ALIGN(16); : #endif

...

This soverrides the 'sanity check' on the bootblock size, that it remains below CONFIG_C_ENV_BOOTBLO […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 11 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Attention: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Comment-Date: Thu, 28 Jan 2021 08:33:25 +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)

8:54 a.m.

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

Change subject: arch/x86: Top-align .text in bootblock ......................................................................

arch/x86: Top-align .text in bootblock

Move .text section closer to .init. This reduces the size of the flat bootblock binary and footprint in CBFS.

Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47971 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/Kconfig M src/arch/x86/bootblock.ld 2 files changed, 18 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved

diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index 4de8c96..943bb91 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -191,6 +191,9 @@ hex default 0x10000

+config FIXED_BOOTBLOCK_SIZE + bool + # Default address romstage is to be linked at config ROMSTAGE_ADDR hex diff --git a/src/arch/x86/bootblock.ld b/src/arch/x86/bootblock.ld index 6595262..3cd0900 100644 --- a/src/arch/x86/bootblock.ld +++ b/src/arch/x86/bootblock.ld @@ -12,12 +12,25 @@ #endif

SECTIONS { + +#if CONFIG(FIXED_BOOTBLOCK_SIZE) . = _ebootblock - CONFIG_C_ENV_BOOTBLOCK_SIZE; +#else + . = BOOTBLOCK_TOP - PROGRAM_SZ; + . = ALIGN(16); +#endif

_bootblock = .;

INCLUDE "bootblock/lib/program.ld"

+ /* + * Allocation reserves extra space here. Alignment requirements + * may cause the total size of a section to change when the start + * address gets applied. + */ + PROGRAM_SZ = SIZEOF(.text) + 512; + . = MIN(_ID_SECTION, _FIT_POINTER) - EARLYASM_SZ; . = CONFIG(SIPI_VECTOR_IN_ROM) ? ALIGN(4096) : ALIGN(16); BOOTBLOCK_TOP = .; @@ -66,3 +79,5 @@ _bogus1 = ASSERT(_bootblock & 0x80000000, "_bootblock too low, invalid ld script"); _bogus2 = ASSERT(_start16bit & 0x80000000, "_start16bit too low, invalid ld script"); _bogus3 = ASSERT(_start16bit >= 0xffff0000, "_start16bit too low. Please report."); +_bogus4 = ASSERT(_ebootblock - _bootblock <= CONFIG_C_ENV_BOOTBLOCK_SIZE, + "_bootblock too low, increase C_ENV_BOOTBLOCK_SIZE");

-- To view, visit https://review.coreboot.org/c/coreboot/+/47971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64325bd633e1104853cfb928c7f801d94ff3045a Gerrit-Change-Number: 47971 Gerrit-PatchSet: 12 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1402

days inactive

1467

days old

coreboot-gerrit@coreboot.org

9 comments

2 participants

tags (0)

participants (2)

Arthur Heymans (Code Review)
Kyösti Mälkki (Code Review)