Change in coreboot[master]: cbfs: allow uncompressed payloads

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

ron minnich (Code Review)

21 Feb 2020 21 Feb '20

11:14 p.m.

ron minnich has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39051 )

Change subject: cbfs: allow uncompressed payloads ......................................................................

cbfs: allow uncompressed payloads

Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M payloads/Kconfig 1 file changed, 6 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/39051/1

diff --git a/payloads/Kconfig b/payloads/Kconfig index 4e86c21..99a4391 100644 --- a/payloads/Kconfig +++ b/payloads/Kconfig @@ -60,7 +60,12 @@ depends on !PAYLOAD_NONE && !PAYLOAD_LINUX && !PAYLOAD_LINUXBOOT && !PAYLOAD_FIT help Choose the compression algorithm for the chosen payloads. - You can choose between LZMA and LZ4. + You can choose between None, LZMA, or LZ4. + +config COMPRESSED_PAYLOAD_NONE + bool "Use no compression for payloads" + help + Do not compress the payload.

config COMPRESSED_PAYLOAD_LZMA bool "Use LZMA compression for payloads"

Attachments:

attachment.htm (text/html — 2.7 KB)

Show replies by date

Patrick Georgi (Code Review)

21 Feb 21 Feb

11:22 p.m.

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

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig File payloads/Kconfig:

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig@60 PS1, Line 60: && !PAYLOAD_LINUX && !PAYLOAD_LINUXBOOT && !PAYLOAD_FIT I think those were added because they implicitly used the "no compression" mode. In that case they could be removed there (and default to COMPRESSED_PAYLOAD_NONE there), but that needn't happen in this commit.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-Comment-Date: Fri, 21 Feb 2020 22:22:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Martin Roth (Code Review)

11:25 p.m.

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39051 )

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig File payloads/Kconfig:

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig@60 PS1, Line 60: && !PAYLOAD_LINUX && !PAYLOAD_LINUXBOOT && !PAYLOAD_FIT

...

I think those were added because they implicitly used the "no compression" mode. […]

so maybe remove those and add:

default COMPRESSED_PAYLOAD_NONE if PAYLOAD_LINUX || PAYLOAD_LINUXBOOT || PAYLOAD_FIT default COMPRESSED_PAYLOAD_LZMA

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-Comment-Date: Fri, 21 Feb 2020 22:25:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

Nico Huber (Code Review)

22 Feb 22 Feb

12:40 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39051 )

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig File payloads/Kconfig:

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig@60 PS1, Line 60: && !PAYLOAD_LINUX && !PAYLOAD_LINUXBOOT && !PAYLOAD_FIT

...

so maybe remove those and add: […]

That would allow users to select one of the compression options with those payloads, would that boot?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Sat, 22 Feb 2020 11:40:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth martinroth@google.com Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

3:35 p.m.

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

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig File payloads/Kconfig:

https://review.coreboot.org/c/coreboot/+/39051/1/payloads/Kconfig@60 PS1, Line 60: && !PAYLOAD_LINUX && !PAYLOAD_LINUXBOOT && !PAYLOAD_FIT

...

That would allow users to select one of the compression options with those […]

At least for linux/linuxboot, the issue is merely that they're already compressed so it makes no sense to compress them again - it only slows down boot. There shouldn't be any functional issue.

Not sure about FIT.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Sat, 22 Feb 2020 14:35:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Martin Roth martinroth@google.com Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

ron minnich (Code Review)

10:13 p.m.

Hello Stefan Reinauer, build bot (Jenkins), Patrick Georgi, Martin Roth,

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

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

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

Change subject: cbfs: allow uncompressed payloads ......................................................................

cbfs: allow uncompressed payloads

Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M payloads/Kconfig 1 file changed, 7 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/39051/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 2 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-MessageType: newpatchset

Patrick Georgi (Code Review)

10:14 p.m.

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

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 2: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 2 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Sat, 22 Feb 2020 21:14:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

10:25 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39051 )

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39051/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39051/2//COMMIT_MSG@8 PS2, Line 8: Please add a motivation to the commit message.

...

Some payloads, like the Linux payload, are already compressed, so compressing them again does not give any gain.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 2 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 22 Feb 2020 21:25:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Georgi (Code Review)

11:11 p.m.

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

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39051/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39051/2//COMMIT_MSG@8 PS2, Line 8:

...

Please add a motivation to the commit message. […]

That's the motivation for the cutout for linux/linuxboot/fit payloads, but that already existed before. The motivation for this commit is, as the subject says, to allow uncompressed payloads.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 2 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 22 Feb 2020 22:11:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Patrick Georgi (Code Review)

11:38 p.m.

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

Change subject: cbfs: allow uncompressed payloads ......................................................................

cbfs: allow uncompressed payloads

Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Signed-off-by: Ronald G. Minnich rminnich@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39051 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M payloads/Kconfig 1 file changed, 7 insertions(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved

diff --git a/payloads/Kconfig b/payloads/Kconfig index 4e86c21..f85dce9 100644 --- a/payloads/Kconfig +++ b/payloads/Kconfig @@ -57,10 +57,16 @@ choice prompt "Payload compression algorithm" default COMPRESSED_PAYLOAD_LZMA + default COMPRESSED_PAYLOAD_NONE if PAYLOAD_LINUX || PAYLOAD_LINUXBOOT || PAYLOAD_FIT depends on !PAYLOAD_NONE && !PAYLOAD_LINUX && !PAYLOAD_LINUXBOOT && !PAYLOAD_FIT help Choose the compression algorithm for the chosen payloads. - You can choose between LZMA and LZ4. + You can choose between None, LZMA, or LZ4. + +config COMPRESSED_PAYLOAD_NONE + bool "Use no compression for payloads" + help + Do not compress the payload.

config COMPRESSED_PAYLOAD_LZMA bool "Use LZMA compression for payloads"

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 3 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

9elements QA (Code Review)

23 Feb 23 Feb

12:13 a.m.

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39051 )

Change subject: cbfs: allow uncompressed payloads ......................................................................

Patch Set 3:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/766 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/765 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/764

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/39051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8261bc28e5bc9aa32db1dccef7035486995c9873 Gerrit-Change-Number: 39051 Gerrit-PatchSet: 3 Gerrit-Owner: ron minnich rminnich@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Stefan Reinauer stefan.reinauer@coreboot.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: ron minnich rminnich@gmail.com Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Nico Huber nico.h@gmx.de Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 22 Feb 2020 23:13:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1895

days inactive

1896

days old

coreboot-gerrit@coreboot.org

10 comments

6 participants

tags (0)

participants (6)

9elements QA (Code Review)
Martin Roth (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)
ron minnich (Code Review)