Change in coreboot[master]: libpayload/defconfig: Set default heap size to 1MiB

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Nico Huber (Code Review)

4 Aug 2020 4 Aug '20

2:24 p.m.

Hello Felix Singer,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: libpayload/defconfig: Set default heap size to 1MiB ......................................................................

libpayload/defconfig: Set default heap size to 1MiB

libpayload's drivers keeps growing. With certain hardware/payload combinations (last time witnessed with Kontron/bSL6 and FILO), the default configuration runs out of memory.

As there is a lot enabled by default, also set a big default heap size.

Tested with FILO on QEMU/Q35.

Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Signed-off-by: Nico Huber nico.huber@secunet.com --- M payloads/libpayload/configs/defconfig 1 file changed, 1 insertion(+), 1 deletion(-)

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

diff --git a/payloads/libpayload/configs/defconfig b/payloads/libpayload/configs/defconfig index 896039e..4ef1ed1 100644 --- a/payloads/libpayload/configs/defconfig +++ b/payloads/libpayload/configs/defconfig @@ -5,4 +5,4 @@ CONFIG_LP_USB_OHCI=y CONFIG_LP_USB_EHCI=y CONFIG_LP_USB_XHCI=y -CONFIG_LP_HEAP_SIZE=262144 +CONFIG_LP_HEAP_SIZE=1048576

Attachments:

attachment.htm (text/html — 2.6 KB)

Show replies by date

Angel Pons (Code Review)

4 Aug 4 Aug

3:21 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44176 )

Change subject: libpayload/defconfig: Set default heap size to 1MiB ......................................................................

Patch Set 1: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/44176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Gerrit-Change-Number: 44176 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 04 Aug 2020 15:21:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Felix Singer (Code Review)

3:35 p.m.

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44176 )

Change subject: libpayload/defconfig: Set default heap size to 1MiB ......................................................................

Patch Set 1: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/44176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Gerrit-Change-Number: 44176 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 04 Aug 2020 15:35:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

9:53 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44176 )

Change subject: libpayload/defconfig: Set default heap size to 1MiB ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44176/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/44176/1//COMMIT_MSG@9 PS1, Line 9: keeps nit: keep

-- To view, visit https://review.coreboot.org/c/coreboot/+/44176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Gerrit-Change-Number: 44176 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 04 Aug 2020 21:53:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Georgi (Code Review)

17 Aug 17 Aug

6:10 a.m.

Patrick Georgi has uploaded a new patch set (#2) to the change originally created by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/44176 )

Change subject: libpayload/defconfig: Set default heap size to 1MiB ......................................................................

libpayload/defconfig: Set default heap size to 1MiB

libpayload's drivers keep growing. With certain hardware/payload combinations (last time witnessed with Kontron/bSL6 and FILO), the default configuration runs out of memory.

As there is a lot enabled by default, also set a big default heap size.

Tested with FILO on QEMU/Q35.

Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Signed-off-by: Nico Huber nico.huber@secunet.com --- M payloads/libpayload/configs/defconfig 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/44176/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/44176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Gerrit-Change-Number: 44176 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Georgi (Code Review)

6:10 a.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44176 )

Change subject: libpayload/defconfig: Set default heap size to 1MiB ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44176/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/44176/1//COMMIT_MSG@9 PS1, Line 9: keeps

...

nit: keep

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/44176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Gerrit-Change-Number: 44176 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 17 Aug 2020 06:10:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

6:10 a.m.

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44176 )

Change subject: libpayload/defconfig: Set default heap size to 1MiB ......................................................................

libpayload/defconfig: Set default heap size to 1MiB

libpayload's drivers keep growing. With certain hardware/payload combinations (last time witnessed with Kontron/bSL6 and FILO), the default configuration runs out of memory.

As there is a lot enabled by default, also set a big default heap size.

Tested with FILO on QEMU/Q35.

Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44176 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Felix Singer felixsinger@posteo.net --- M payloads/libpayload/configs/defconfig 1 file changed, 1 insertion(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved

-- To view, visit https://review.coreboot.org/c/coreboot/+/44176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51a1514097aeb8b3c835a2387db66869b81d0bcc Gerrit-Change-Number: 44176 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1591

days inactive

1604

days old

coreboot-gerrit@coreboot.org

6 comments

4 participants

tags (0)

participants (4)

Angel Pons (Code Review)
Felix Singer (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)