coreboot patches: remove lib/jpeg patches for heap increase and alloc failure. Add https://review.coreboot.org/c/coreboot/+/83895

Repro:
rm patches/coreboot-24.02.01/0001* patches/coreboot-24.02.01/0002*
git fetch https://review.coreboot.org/coreboot refs/changes/94/83894/2 && git format-patch -1 --stdout FETCH_HEAD > patches/coreboot-24.02.01/0001-vc_wuffs-upgrade-to-Wuffs_0.4.0-alpha.8.patch
git fetch https://review.coreboot.org/coreboot refs/changes/95/83895/3 && git format-patch -1 --stdout FETCH_HEAD > patches/coreboot-24.02.01/0002-lib_jpeg-avoidcalling-malloc-and-free.patch
sed -i 's/CONFIG_HEAP_SIZE=0x400000/CONFIG_HEAP_SIZE=0x100000/g' config/coreboot-*

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
This commit is contained in:
Thierry Laurion 2024-09-11 17:11:44 -04:00
parent 4e22b503ba
commit c63114710e
27 changed files with 111535 additions and 147 deletions

View File

@ -478,7 +478,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -501,7 +501,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -501,7 +501,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -498,7 +498,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -501,7 +501,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -501,7 +501,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -498,7 +498,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -502,7 +502,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -504,7 +504,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -327,7 +327,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -324,7 +324,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -424,7 +424,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -424,7 +424,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -422,7 +422,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -424,7 +424,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -427,7 +427,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -422,7 +422,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -424,7 +424,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -413,7 +413,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -413,7 +413,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -425,7 +425,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -424,7 +424,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -401,7 +401,7 @@ CONFIG_NO_EARLY_GFX_INIT=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_LINEAR_FRAMEBUFFER=y
CONFIG_BOOTSPLASH=y
CONFIG_HEAP_SIZE=0x400000
CONFIG_HEAP_SIZE=0x100000
# end of Display
CONFIG_PCI=y

View File

@ -1,80 +0,0 @@
From 8b6fc3a877d8169091d034ea6ac6d15593cc69a0 Mon Sep 17 00:00:00 2001
From: Jonathon Hall <jonathon.hall@puri.sm>
Date: Mon, 15 Jul 2024 15:01:52 -0400
Subject: [PATCH] src/lib/malloc.c: If allocation fails, leave the heap
unchanged
If an allocation fails because it is too large for the rest of the heap,
don't consume the rest of the heap needlessly.
This started occurring with the Heads bootsplash image in 24.02.01,
following the switch to the Wuffs JPEG decoder. The work area needed
was too large for the heap. The bootsplash failed to show, but worse,
the boot failed entirely because we were then out of heap space, even
though we did not actually use the large allocation that failed.
With this change, that failure no longer prevents boot.
The error message is improved slightly also:
* missing line break is added
* "Tried to round up" now shows the beginning of the allocation before
and after rounding instead of the unrounded beginning and rounded end
(misleading, looked like it was trying to align by 1 MB when it
was actually allocating 1 MB)
Change-Id: Ie72814027d9daa517c0794f3ea7abec2b9a9d596
Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
---
src/lib/malloc.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/lib/malloc.c b/src/lib/malloc.c
index 30298064d9..281792c3d5 100644
--- a/src/lib/malloc.c
+++ b/src/lib/malloc.c
@@ -26,30 +26,29 @@ void *memalign(size_t boundary, size_t size)
MALLOCDBG("%s Enter, boundary %zu, size %zu, free_mem_ptr %p\n",
__func__, boundary, size, free_mem_ptr);
- free_mem_ptr = (void *)ALIGN_UP((unsigned long)free_mem_ptr, boundary);
+ p = (void *)ALIGN_UP((unsigned long)free_mem_ptr, boundary);
- p = free_mem_ptr;
- free_mem_ptr += size;
- /*
- * Store last allocation pointer after ALIGN, as malloc() will
- * return it. This may cause n bytes of gap between allocations
- * where n < boundary.
- */
- free_last_alloc_ptr = p;
-
- if (free_mem_ptr >= free_mem_end_ptr) {
+ if (p + size >= free_mem_end_ptr) {
printk(BIOS_ERR, "%s(boundary=%zu, size=%zu): failed: ",
__func__, boundary, size);
printk(BIOS_ERR, "Tried to round up free_mem_ptr %p to %p\n",
- p, free_mem_ptr);
+ free_mem_ptr, p);
printk(BIOS_ERR, "but free_mem_end_ptr is %p\n",
free_mem_end_ptr);
printk(BIOS_ERR, "Error! %s: Out of memory "
- "(free_mem_ptr >= free_mem_end_ptr)",
+ "(free_mem_ptr >= free_mem_end_ptr)\n",
__func__);
return NULL;
}
+ free_mem_ptr = p + size;
+ /*
+ * Store last allocation pointer after ALIGN, as malloc() will
+ * return it. This may cause n bytes of gap between allocations
+ * where n < boundary.
+ */
+ free_last_alloc_ptr = p;
+
MALLOCDBG("%s %p\n", __func__, p);
return p;
--
2.39.2

File diff suppressed because it is too large Load Diff

View File

@ -1,44 +0,0 @@
From 00d695849a5fb503d87203e3515f761fa8dac850 Mon Sep 17 00:00:00 2001
From: Jonathon Hall <jonathon.hall@puri.sm>
Date: Mon, 15 Jul 2024 16:37:40 -0400
Subject: [PATCH] bootsplash: Increase heap from 1 MB to 4 MB when bootsplash
is enabled
Since commit 1d029b40c9de ("lib/jpeg: Replace decoder with Wuffs'
implementation"), a relatively large heap allocation is needed to
decode many JPEGs for use as work area. The prior decoder did not need
this, but also had many limitations in the JPEGs it could decode.
A 1024x768 non-progressive JPEG used in Heads needs 1179648 bytes of
work area; about 1.2 MB. While the work area will also depend on the
subsampling of each channel, it's generally proportional to the image
size.
Increasing the heap size to 4 MB when bootsplash is enabled should be
enough to decode bootsplashes up to 1920x1080 with some headroom.
Change-Id: Ia4348d39effbc16c1b42ab01bcf1e4ec5d652fa9
Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
---
src/device/Kconfig | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/device/Kconfig b/src/device/Kconfig
index 243e23e52a..4dd03eba21 100644
--- a/src/device/Kconfig
+++ b/src/device/Kconfig
@@ -501,6 +501,11 @@ config BOOTSPLASH
image in the 'General' section or add it manually to CBFS, using,
for example, cbfstool.
+# The bootsplash JPEG decoder requires heap space approximately proportional to
+# the image size. This usually needs a larger heap.
+config HEAP_SIZE
+ default 0x400000 if BOOTSPLASH
+
config LINEAR_FRAMEBUFFER_MAX_WIDTH
int "Maximum width in pixels"
depends on LINEAR_FRAMEBUFFER && MAINBOARD_USE_LIBGFXINIT
--
2.39.2

View File

@ -0,0 +1,91 @@
From efad423f8472c1b9c130842e3d92625500f82d5d Mon Sep 17 00:00:00 2001
From: Nigel Tao <nigeltao@golang.org>
Date: Tue, 13 Aug 2024 22:29:21 +1000
Subject: [PATCH] lib/jpeg: avoid calling malloc and free
Since commit 1d029b40c9de ("lib/jpeg: Replace decoder with Wuffs'
implementation"), a relatively large heap allocation is needed to decode
many JPEGs for use as work area. The prior decoder did not need this,
but also had many limitations in the JPEGs it could decode, was not as
memory-safe and quickly crashed under fuzzing.
This commit keeps using Wuffs' JPEG decoder, but it no longer requires
any heap allocation (and thus configuring the heap size depending on how
big a bootsplash image you want to support).
Change-Id: Ie4c52520cbce498539517c4898ff765365a6beba
Signed-off-by: Nigel Tao <nigeltao@golang.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83895
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Felix Singer <service+coreboot-gerrit@felixsinger.de>
Reviewed-by: Jonathon Hall <jonathon.hall@puri.sm>
---
src/lib/jpeg.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/src/lib/jpeg.c b/src/lib/jpeg.c
index 242cf0ca8e..617ab0b22a 100644
--- a/src/lib/jpeg.c
+++ b/src/lib/jpeg.c
@@ -1,9 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Provide a simple API around the Wuffs JPEG decoder
- * Uses the heap (and lots of it) for the image-size specific
- * work buffer, so ramstage-only.
+ * Provide a simple API around the Wuffs JPEG decoder.
*/
#include <stdint.h>
@@ -85,6 +83,24 @@ int jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *pic,
return JPEG_DECODE_FAILED;
}
+ /* Opting in to lower quality means that we can pass an empty slice as the
+ * "work buffer" argument to wuffs_jpeg__decoder__decode_frame below.
+ *
+ * Decoding progressive (not sequential) JPEGs would still require dynamic
+ * memory allocation (and the amount of work buffer required depends on the
+ * image dimensions), but we choose to just reject progressive JPEGs. It is
+ * simpler than sometimes calling malloc (which can fail, especially for
+ * large allocations) and free.
+ *
+ * More commentary about these quirks is at
+ * https://github.com/google/wuffs/blob/beaf45650085a16780b5f708b72daaeb1aa865c8/std/jpeg/decode_quirks.wuffs
+ */
+ wuffs_jpeg__decoder__set_quirk(
+ &dec, WUFFS_BASE__QUIRK_QUALITY,
+ WUFFS_BASE__QUIRK_QUALITY__VALUE__LOWER_QUALITY);
+ wuffs_jpeg__decoder__set_quirk(
+ &dec, WUFFS_JPEG__QUIRK_REJECT_PROGRESSIVE_JPEGS, 1);
+
wuffs_base__image_config imgcfg;
wuffs_base__io_buffer src = wuffs_base__ptr_u8__reader(filedata, filesize, true);
status = wuffs_jpeg__decoder__decode_image_config(&dec, &imgcfg, &src);
@@ -104,19 +120,9 @@ int jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *pic,
return JPEG_DECODE_FAILED;
}
- uint64_t workbuf_len_min_incl = wuffs_jpeg__decoder__workbuf_len(&dec).min_incl;
- uint8_t *workbuf_array = malloc(workbuf_len_min_incl);
- if ((workbuf_array == NULL) && workbuf_len_min_incl) {
- return JPEG_DECODE_FAILED;
- }
-
- wuffs_base__slice_u8 workbuf =
- wuffs_base__make_slice_u8(workbuf_array, workbuf_len_min_incl);
status = wuffs_jpeg__decoder__decode_frame(&dec, &pixbuf, &src,
- WUFFS_BASE__PIXEL_BLEND__SRC, workbuf, NULL);
-
- free(workbuf_array);
-
+ WUFFS_BASE__PIXEL_BLEND__SRC,
+ wuffs_base__empty_slice_u8(), NULL);
if (status.repr) {
return JPEG_DECODE_FAILED;
}
--
2.39.2