Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/boot/linux.go: Decompress kernel before doing kexec #2755

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

CodingVoid
Copy link
Contributor

It is possible/likely that the kernel is compressed. In order for kexec to actually find the magic value for aarch64 Linux kernels it needs to be decompressed before refering it to the kexec syscall.

Test: aarch64 Fedora 36 Server image can now be started via localboot.

@CodingVoid CodingVoid force-pushed the arm64-compressed-kernel branch 2 times, most recently from 83cfd56 to 6a5d2d0 Compare September 5, 2023 16:46
@hugelgupf
Copy link
Member

A kernel should decompress itself on kexec, shouldn't it? What kind of compression is on this thing? (Is that a feature not available on aarch64?)

u-root's kexec is in use in memory-constrained environments sometimes, so we'd likely prefer if this was either behind a flag or could be avoided altogether due to its use of memory.

@CodingVoid
Copy link
Contributor Author

On aarch64 kexec does not decompress the kernel (that may not be true for the new zimage in Linux though). As far as I know that is only happening on x86 (bzImage). That is actually what the kexec u-root command already does (so if I do "kexec ..." in u-root it works but not with the localboot cmd). No Idea why there are different code paths for kexec and kexec localboot cmd in the first place, but I guess that is something for another patch.
kexec u-root cmd decompression:

k, err := copyToFileIfNotRegular(util.TryGzipFilter(li.Kernel), verbose)

I used a aarch64 fedora server image for testing.

@hugelgupf
Copy link
Member

On aarch64 kexec does not decompress the kernel (that may not be true for the new zimage in Linux though). As far as I know that is only happening on x86 (bzImage). That is actually what the kexec u-root command already does (so if I do "kexec ..." in u-root it works but not with the localboot cmd). No Idea why there are different code paths for kexec and kexec localboot cmd in the first place, but I guess that is something for another patch. kexec u-root cmd decompression:

k, err := copyToFileIfNotRegular(util.TryGzipFilter(li.Kernel), verbose)

I used a aarch64 fedora server image for testing.

Oh this is for jsonboot. I didn't even notice that. It probably ultimately shouldn't be a different codepath, but jsonboot doesn't yet.

@hugelgupf
Copy link
Member

Ultimately, IMO any remaining localboot functionality should merge into the boot command (which uses the code path kexec uses), and localboot should be deleted.

hugelgupf
hugelgupf previously approved these changes Sep 7, 2023
@hugelgupf
Copy link
Member

Have you looked into the failing tests?

@hugelgupf
Copy link
Member

pkg/boot/linux_test.go:144:12: undefined: copyToFileIfNotRegular

@rminnich rminnich added the Awaiting author Waiting for new changes or feedback for author. label Sep 7, 2023
It is possible/likely that the kernel is compressed. In order for kexec
to actually find the magic value for aarch64 Linux kernels it needs to
be decompressed before refering it to the kexec syscall.

Test: aarch64 Fedora 36 Server image can now be started via localboot

Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>
@codecov
Copy link

codecov bot commented Sep 10, 2023

Codecov Report

Patch coverage is 42.85% of modified lines.

Files Changed Coverage
pkg/boot/jsonboot/bootconfig.go 0.00%
pkg/boot/linux.go 100.00%

📢 Thoughts on this report? Let us know!.

@rminnich rminnich merged commit 927756b into u-root:main Sep 11, 2023
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting author Waiting for new changes or feedback for author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants