New issue
Advanced search Search tips

Issue 873135 link

Starred by 0 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature

Blocked on:
issue 381916



Sign in to add a comment

recovery kernels: significantly increase partition size

Project Member Reported by vapier@chromium.org, Aug 10

Issue description

i understand that for shipped images we can't change the disk layout.  the kernel & rootfs sizes are fixed.  but that shouldn't have any impact on the size of the recovery kernel: it is never installed onto the device, and the partition size for the recovery kernel is independent of the partition size of the kernel copied to the device.

so what's holding us back from basically making the recovery image size entirely arbitrary ?

in years past, there were bugs in our firmware which limited the max size of the kernel, but those devices are either going EOL or already have.  i think lumpy/stumpy were the last ones ?  not sure snow/u-boot also had this limitation.
 
Cc: sjg@chromium.org jwer...@chromium.org
I don't personally know of any bugs relating to U-Boot being able to deal with a larger kernel.  I suppose someone could try it.  +sjg who might remember and +jwerner as a heads up
Curious: why do we need to increase the kernel size only for recovery? Don't we ship exactly the same kernel on recovery and normal images?
> Curious: why do we need to increase the kernel size only for recovery? Don't we ship exactly the same kernel on recovery and normal images?

This is about the recovery initramfs size, I presume.

arm64 architecture boot code in depthcharge currently enforces a hard limit of 64MB (after decompressing the kernel itself, but not the initramfs), which I naturally assumed ought to be enough for everyone at some point.

There has also always been a limit on how large the compressed kernel image loaded from disk may be. These days that is 32MB, but before 2015 it used to be 16MB (because that's what the partition size was, and it could possibly get bigger than that! Right? Riiight? :( ). On FIT image platforms this is the size of the whole FIT image.

Since recovery firmware is always RO there is no way to fix this on existing boards. So, in practice, no, I don't think we can easily support this for old boards.

(I wouldn't necessarily call these things "bugs" in the firmware, btw... you always need a buffer to hold the kernel image when you load it and of course you'll have to hardcode *some* size for that somewhere. Even if you allocate dynamically you'd probably still have a fixed heap size. But I agree that we could've paid more attention to keep these limits larger.)
yes, this is entirely because of the initramfs.  Doug ran into hell last time we did some rework, and although we've gotten it back under the hard limit, we've been pretty close ever since.  we've started doing more work recently in the recovery initramfs and once again ran into problems where we wanted to pull new libs (e.g. C++) but couldn't, so had to go back to writing everything in C/shell.

32MiB compressed seems like "enough", but on the other hand, if carving a huge unused chunk of RAM is cheap/free, it really seems like we should pick a "ridiculously" large number like 128MB.

can we get an audit to figure out what versions had what limits precisely ?  even if we can't support it today, it'd be good to figure out when we can ease these limits.

wrt sizes, really seems like it wouldn't be hard to make this work properly all the time in coreboot ... but i'm not sure getting into that debate is worth anyone's time.
Okay, I've uploaded a change to increase the buffer size to 512MB from now on: https://chromium-review.googlesource.com/c/chromiumos/platform/depthcharge/+/1179122

For older boards, it looks like the buffer has always been at least 16MB large. Since 2015 we've had 32MB: https://chromium-review.googlesource.com/c/chromiumos/platform/depthcharge/+/281806

This patch is not yet in the Veyron (RK3288) or Auron (Broadwell) branches (and presumably not anything earlier like Baytrail, Haswell, Tegra124, etc.), so those are the last devices that would not support 32MB. Everything starting with GLaDOS (Skylake), Strago (Braswell), Oak (MT8173) or Gru (RK3399) as well as Ryu (Tegra210) *do* have the patch, so they should all support 32MB. Everything that hasn't branched yet (e.g. Cheza) will support up to 512MB... you can go lobby with the firmware leads of currently branched but not yet FSI'd x86 devices (e.g. several but I think not all Poppy (Kabylake) and newer Amberlake, Whiskeylake, Krabbylake or Whatevertheycallitthesedayslake devices) if you want them to pick it up as well.
Blockedon: 381916
thanks, that helps a lot.  i guess this reiterates the existing size check in cros-kernel2.eclass doesn't make sense:
        if version_is_at_least 3.18 ${version} ; then
            kern_max=32
            kern_warn=12
        elif version_is_at_least 3.10 ${version} ; then
            kern_max=16
            kern_warn=12
        else
            kern_max=8
            kern_warn=7
        fi
we updated BayTrail to 4.4 but its firmware doesn't support 32MiB

we're going to have veyron devices until at least 2023 which means we're stuck with 16MB until then :/
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/160674ef1ac0de447dddc11cc2fbb0d3adc93235

commit 160674ef1ac0de447dddc11cc2fbb0d3adc93235
Author: Julius Werner <jwerner@chromium.org>
Date: Sat Aug 18 03:39:02 2018

image: Set proper end for FDT buffer, add overlap checks

There are up to three hardcoded memory ranges in depthcharge that are
used at the same time during kernel handoff: the depthcharge code and
data itself, the buffer that vboot loads the kernel into, and (on FDT
systems) the buffer that the FDT is passed to the kernel in. Right now
we just rely on people manually configuring these ranges to not overlap,
and that has been fine so far, but automated checking is always nicer.

This patch ads link-time overlap checks to these regions. On arm64, we
have the additional requirement that all other special memory regions
handled directly by depthcharge should be below the depthcharge code
itself. For simplicity (and since it doubles as a convenient way to
ensure we're not overlapping CBMEM either), just enforce this
requirement across all architectures. The x86 ones (which all use the
same addresses) already conform to this anyway. Some other boards
needed to have their placements slightly adjusted to conform.

Since the FDT area didn't have a well-delimited size for now, let's just
hardcode it to 8MB. Current FDTs are generally in the 50KB range, so
this should be enough forever (famous last words...).

BRANCH=None
BUG=chromium:873135
TEST=None

Change-Id: I52715f13544063549ec07a2adf2f295917298e0e
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1179121
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/oak/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/nyan/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/daisy/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/src/image/Makefile.inc
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/src/arch/mips/fit.c
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/hana/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/src/image/symbols.h
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/elm/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/src/arch/arm/fit.c
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/peach_pit/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/nyan_big/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/purin/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/src/boot/Kconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/rowan/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/nyan_blaze/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/cheza/defconfig
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/src/image/depthcharge.ldscript
[modify] https://crrev.com/160674ef1ac0de447dddc11cc2fbb0d3adc93235/board/kukui/defconfig

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/45f2be285df6c78b657304198d736e33a9992cc2

commit 45f2be285df6c78b657304198d736e33a9992cc2
Author: Julius Werner <jwerner@chromium.org>
Date: Sat Aug 18 03:39:03 2018

image: Expand kernel buffer size to 512MB

This patch expands the kernel buffer to 512MB (or to 128MB on arm32
devices where that's the Linux ABI limit anyway, or to less on some
older specialty platforms where it otherwise wouldn't fit). The kernel
partition size on disk has traditionally only been 16MB, and we don't
plan on changing that... however, on a recovery USB stick we can make
the partition as large as we want, and we may need to do that if we want
to add more stuff to the recovery initramfs. Unfortunately we'll be
stuck with the exisiting limits on old platforms since recovery firmware
is not updateable, but this CL is meant to ensure that we'll at least
have lots of headroom to grow on future platforms.

Some memory layouts had to be adjusted to fit the new buffer. Also
document the individual arch-dependent constraints for placing various
regions in the Kconfig help (some of these only apply to older Linux
versions, but since we don't update our kernels it's better to mention
everything that might apply... boards can still violate them after they
made sure that they're unaffected).

BRANCH=None
BUG=chromium:873135
TEST=Booted Cheza (not feasible to test *every* platform, I've
double-checked everything manually as best as I could).

Change-Id: I23f1a2c78f4e82c1f1e4a16d823ab36d2e90ef1d
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1179122
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/foster/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/src/boot/Kconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/gale/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/veyron_jerry/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/veyron_minnie/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/smaug/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/veyron_mickey/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/kevin/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/gru/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/src/image/Kconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/veyron_jaq/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/veyron_mighty/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/rainier/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/urara/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/bob/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/veyron_speedy/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/scarlet/defconfig
[modify] https://crrev.com/45f2be285df6c78b657304198d736e33a9992cc2/board/veyron_rialto/defconfig

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/de70fa256275af6711741c573ad44cf8e37bcd68

commit de70fa256275af6711741c573ad44cf8e37bcd68
Author: Julius Werner <jwerner@chromium.org>
Date: Sat Aug 18 03:39:03 2018

arm64: Read kernel relocation buffer size from kernel image header

If we want to boot potentially very large kernel images, we can no
longer easily hardcode the size of the kernel relocation buffer on
arm64. Instead, just take the image_size provided for this purpose in
the kernel header and find a buffer that fits exactly that size.

BRANCH=None
BUG=chromium:873135
TEST=Booted Cheza.

Change-Id: Ia03a206eb96a92cf4b076d68cdc1e797510436d9
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1179267
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

[modify] https://crrev.com/de70fa256275af6711741c573ad44cf8e37bcd68/src/arch/arm/boot64.c

Simon pointed out that the u-boot code has an 8MiB limit in its kernel loading logic on exynos systems:
https://chromium.googlesource.com/chromiumos/third_party/u-boot/+/firmware-snow-2695.90.B/cros/dts/chromeos-exynos.dtsi#28
		kernel = <0x42000000 0x00800000>;

Sign in to add a comment