Attention is currently required from: Felix Singer, Krystian Hebel, Maciej Pijanowski, Martin L Roth, Michał Kopeć.
Filip Lewiński has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/83385?usp=email )
Change subject: payloads/external/iPXE/Makefile: Build iPXE for EFI target if requested
......................................................................
Patch Set 3:
(1 comment)
File payloads/external/iPXE/Makefile:
https://review.coreboot.org/c/coreboot/+/83385/comment/80679901_29ae70d0?us… :
PS1, Line 14: ifeq ($(CONFIG_IPXE_BUILD_EFI),y)
> I think you forgot to add a Kconfig option to your commit.
Right, added [here](https://review.coreboot.org/c/coreboot/+/83385/2..3/payloads/external…
--
To view, visit https://review.coreboot.org/c/coreboot/+/83385?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7f247a59a65aeb18a67475d4d543f519af88aeb9
Gerrit-Change-Number: 83385
Gerrit-PatchSet: 3
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 23 Jul 2024 11:25:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Attention is currently required from: Filip Lewiński, Krystian Hebel, Maciej Pijanowski, Martin L Roth, Michał Kopeć.
Hello Krystian Hebel, Maciej Pijanowski, Martin L Roth, Michał Kopeć, Michał Żygowski, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83385?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: payloads/external/iPXE/Makefile: Build iPXE for EFI target if requested
......................................................................
payloads/external/iPXE/Makefile: Build iPXE for EFI target if requested
Change-Id: I7f247a59a65aeb18a67475d4d543f519af88aeb9
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
---
M payloads/external/iPXE/Kconfig
M payloads/external/iPXE/Makefile
2 files changed, 17 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/83385/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/83385?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7f247a59a65aeb18a67475d4d543f519af88aeb9
Gerrit-Change-Number: 83385
Gerrit-PatchSet: 3
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Attention is currently required from: Jonathon Hall, Martin L Roth, Nico Huber, Paul Menzel.
Nigel Tao has posted comments on this change by Jonathon Hall. ( https://review.coreboot.org/c/coreboot/+/83476?usp=email )
Change subject: bootsplash: Increase heap from 1 MB to 4 MB when bootsplash is enabled
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Is 4 MB so large to be problematic? I'm familiar with Wuffs but not so familiar with coreboot.
Wuffs is a memory-safe language (but can interface with C code) and part of how it is immune to memory bug classes like use-after-free, double-free or assuming-malloc-cannot-fail is that Wuffs code is incapable of calling malloc and free, or similar. The C code (that calls into Wuffs) is responsible for allocating memory (whether via malloc or otherwise).
Wuffs' JPEG implementation is currently pretty conservative, in that it overestimates how much memory it needs. The API is simpler if Wuffs tells C *once* it needs one big, worst-case allocation instead telling it *many times* it needs many small allocations (of various sizes).
But if overestimating memory requirements (by a few MB) is problematic for coreboot, I can do some thinking about a leaner (but more complicated) API.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83476?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia4348d39effbc16c1b42ab01bcf1e4ec5d652fa9
Gerrit-Change-Number: 83476
Gerrit-PatchSet: 2
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Tue, 23 Jul 2024 11:14:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: David Wu, Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro.
Subrata Banik has posted comments on this change by David Wu. ( https://review.coreboot.org/c/coreboot/+/83614?usp=email )
Change subject: mb/google/nissa/var/riven: Add Fn key scancode
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83614?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iddedd08fc50e8e8e369ce3d73edf0f3077867e87
Gerrit-Change-Number: 83614
Gerrit-PatchSet: 1
Gerrit-Owner: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Tue, 23 Jul 2024 10:53:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83618?usp=email )
Change subject: [WIP] util/cbfstool/cbfs-payload-linux.c: Remove TODO
......................................................................
[WIP] util/cbfstool/cbfs-payload-linux.c: Remove TODO
LZMA checks at util/cbfstool/lzma/lzma.c:Write() for the output
buffer/stream size and does not write beyond it.
LZ4 TODO check
Change-Id: I41298b509b3f5e775bb4000c82c539eefa80c885
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
---
M util/cbfstool/cbfs-payload-linux.c
1 file changed, 0 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/83618/1
diff --git a/util/cbfstool/cbfs-payload-linux.c b/util/cbfstool/cbfs-payload-linux.c
index 94f6161..9ea475e 100644
--- a/util/cbfstool/cbfs-payload-linux.c
+++ b/util/cbfstool/cbfs-payload-linux.c
@@ -193,11 +193,6 @@
* add support for more parameters to trampoline:
* alt_mem_k, ext_mem_k (not strictly necessary since e820 takes precedence)
* framebuffer/console values
- *
- * larger work:
- * is compress() safe to use in a size constrained buffer? ie. do(es) the
- * compression algorithm(s) stop once the compression result reaches input
- * size (ie. incompressible data)?
*/
int parse_bzImage_to_payload(const struct buffer *input,
struct buffer *output, const char *initrd_name,
--
To view, visit https://review.coreboot.org/c/coreboot/+/83618?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I41298b509b3f5e775bb4000c82c539eefa80c885
Gerrit-Change-Number: 83618
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83616?usp=email )
Change subject: util/cbfstool/common.h Fix wrong return value doc
......................................................................
util/cbfstool/common.h Fix wrong return value doc
The compressing and decompressing functions return 0 on success and not
the other way around.
Change-Id: I9f8653aa805c62eb4bfc3560d7880921830c2c59
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
---
M util/cbfstool/common.h
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/83616/1
diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h
index 498aae6..88d5238 100644
--- a/util/cbfstool/common.h
+++ b/util/cbfstool/common.h
@@ -148,14 +148,14 @@
/* Compress in_len bytes from in, storing the result at out, returning the
* resulting length in out_len.
- * Returns 0 on error,
+ * Returns 0 on success,
* != 0 otherwise, depending on the compressing function.
*/
typedef int (*comp_func_ptr) (char *in, int in_len, char *out, int *out_len);
/* Decompress in_len bytes from in, storing the result at out, up to out_len
* bytes.
- * Returns 0 on error,
+ * Returns 0 on success,
* != 0 otherwise, depending on the decompressing function.
*/
typedef int (*decomp_func_ptr) (char *in, int in_len, char *out, int out_len,
--
To view, visit https://review.coreboot.org/c/coreboot/+/83616?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9f8653aa805c62eb4bfc3560d7880921830c2c59
Gerrit-Change-Number: 83616
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83615?usp=email )
Change subject: util/cbfstool/cbfs-payload-linux: Do not compress bzImage
......................................................................
util/cbfstool/cbfs-payload-linux: Do not compress bzImage
Compressing the already compressed bzImage does not yield any
fruit. If you are lucky it actually makes the image a little bit
smaller. If you are unlucky the image actually gets bigger and since the
compressing function is not checked for any erros, coreboot just builds
successfully even though the payload is broken through compression.
Before this patch you could possibly get this error during compilation:
```
E: LZMA: LzmaEnc_Encode failed 9.
```
and your linux payload would end up something like this in CBFS:
```
FMAP REGION: COREBOOT
Name Offset Type Size Comp
....
fallback/payload 0x1c9c0 simple elf 511 none
....
```
That doesn't stop coreboot from finishing the build though, since we
currently don't check for errors from the compression. That is an issue
for another patch though.
Change-Id: I022982667515ce721d98af534414d9e336b5f35a
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
---
M util/cbfstool/cbfs-payload-linux.c
1 file changed, 11 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/83615/1
diff --git a/util/cbfstool/cbfs-payload-linux.c b/util/cbfstool/cbfs-payload-linux.c
index 53d455c..6b4ed2b 100644
--- a/util/cbfstool/cbfs-payload-linux.c
+++ b/util/cbfstool/cbfs-payload-linux.c
@@ -139,7 +139,8 @@
}
static void bzp_output_segment(struct bzpayload *bzp, struct buffer *b,
- uint32_t type, uint64_t load_addr)
+ uint32_t type, uint64_t load_addr,
+ comp_func_ptr compress_func)
{
struct buffer out;
struct cbfs_payload_segment *seg;
@@ -163,7 +164,7 @@
seg->mem_len = buffer_size(b);
seg->offset = bzp->offset;
- bzp->compress(buffer_get(b), buffer_size(b), buffer_get(&out), &len);
+ compress_func(buffer_get(b), buffer_size(b), buffer_get(&out), &len);
seg->compression = bzp->algo;
seg->len = len;
@@ -293,26 +294,28 @@
/* parameter block */
bzp_output_segment(&bzp, &bzp.parameters,
- PAYLOAD_SEGMENT_DATA, LINUX_PARAM_LOC);
+ PAYLOAD_SEGMENT_DATA, LINUX_PARAM_LOC, bzp.compress);
/* code block */
+ // There is no point in compressing the bzImage (it is already compressed)
bzp_output_segment(&bzp, &bzp.kernel,
- PAYLOAD_SEGMENT_CODE, kernel_base);
+ PAYLOAD_SEGMENT_CODE, kernel_base,
+ compression_function(CBFS_COMPRESS_NONE));
/* trampoline */
bzp_output_segment(&bzp, &bzp.trampoline,
- PAYLOAD_SEGMENT_CODE, TRAMPOLINE_ENTRY_LOC);
+ PAYLOAD_SEGMENT_CODE, TRAMPOLINE_ENTRY_LOC, bzp.compress);
/* cmdline */
bzp_output_segment(&bzp, &bzp.cmdline,
- PAYLOAD_SEGMENT_DATA, COMMAND_LINE_LOC);
+ PAYLOAD_SEGMENT_DATA, COMMAND_LINE_LOC, bzp.compress);
/* initrd */
bzp_output_segment(&bzp, &bzp.initrd,
- PAYLOAD_SEGMENT_DATA, initrd_base);
+ PAYLOAD_SEGMENT_DATA, initrd_base, bzp.compress);
/* Terminating entry segment. */
- bzp_output_segment(&bzp, NULL, PAYLOAD_SEGMENT_ENTRY, TRAMPOLINE_ENTRY_LOC);
+ bzp_output_segment(&bzp, NULL, PAYLOAD_SEGMENT_ENTRY, TRAMPOLINE_ENTRY_LOC, bzp.compress);
/* Set size of buffer taking into account potential compression. */
buffer_set_size(&bzp.output, bzp.offset);
--
To view, visit https://review.coreboot.org/c/coreboot/+/83615?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I022982667515ce721d98af534414d9e336b5f35a
Gerrit-Change-Number: 83615
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
David Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83614?usp=email )
Change subject: mb/google/nissa/var/riven: Add Fn key scancode
......................................................................
mb/google/nissa/var/riven: Add Fn key scancode
The Fn key on riven emits a scancode of 94 (0x5e).
BUG=b:345231373
TEST=Flash riven, boot to Linux kernel, and verify that KEY_FN is
generated when pressed using `evtest`.
Change-Id: Iddedd08fc50e8e8e369ce3d73edf0f3077867e87
Signed-off-by: David Wu <david_wu(a)quanta.corp-partner.google.com>
---
M src/mainboard/google/brya/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/83614/1
diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig
index a5bae12..dfbd291 100644
--- a/src/mainboard/google/brya/Kconfig
+++ b/src/mainboard/google/brya/Kconfig
@@ -1,7 +1,7 @@
## SPDX-License-Identifier: GPL-2.0-only
config ACPI_FNKEY_GEN_SCANCODE
- default 94 if BOARD_GOOGLE_XOL
+ default 94 if (BOARD_GOOGLE_XOL || BOARD_GOOGLE_RIVEN)
config BOARD_GOOGLE_BRYA_COMMON
def_bool n
--
To view, visit https://review.coreboot.org/c/coreboot/+/83614?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iddedd08fc50e8e8e369ce3d73edf0f3077867e87
Gerrit-Change-Number: 83614
Gerrit-PatchSet: 1
Gerrit-Owner: David Wu <david_wu(a)quanta.corp-partner.google.com>