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

Kernel/x86: Bake the Prekernel and the Kernel into one image #24280

Merged

Conversation

supercomputer7
Copy link
Member

@supercomputer7 supercomputer7 commented May 11, 2024

The new baked image is a Prekernel and a Kernel baked together now, so essentially we no longer need to pass the Prekernel as -kernel and the actual kernel image as -initrd to QEMU, leaving the option to pass an actual initrd or initramfs module later on with multiboot.

This tries to revive the work from #18027 after it was reverted in #18556. I tried to change things the least I could this time (also, I don't try to iterate on the modules' array anymore, and only pass the physical address and bytes count of the first module to the kernel so it can reserve that range for now).

cc @timschumi @ADKaster

@supercomputer7 supercomputer7 marked this pull request as ready for review May 11, 2024 04:55
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label May 11, 2024
@supercomputer7 supercomputer7 force-pushed the bake-prekernel-with-kernel branch 2 times, most recently from 7a41034 to b730cc9 Compare May 11, 2024 05:13
Kernel/CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Member

@timschumi timschumi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boots fine now on my system (which had issues with the previous revision of the patch). This was already massaged once before (and therefore still looks fine now), so I only have some small things to point out.

Kernel/Prekernel/CMakeLists.txt Outdated Show resolved Hide resolved
Kernel/Prekernel/CMakeLists.txt Outdated Show resolved Hide resolved
Kernel/Prekernel/init.cpp Outdated Show resolved Hide resolved
@supercomputer7
Copy link
Member Author

Boots fine now on my system (which had issues with the previous revision of the patch). This was already massaged once before (and therefore still looks fine now), so I only have some small things to point out.

I suspect the crashing in the previous iteration was happening because I aligned the physical address according to the mapping base of the kernel and it was incorrect? I don't really know, but this should work OK.

Anyway, I sent a fix to resolve what you asked to change, so if it passes CI, it should be good to go :)

The new baked image is a Prekernel and a Kernel baked together now, so
essentially we no longer need to pass the Prekernel as -kernel and the
actual kernel image as -initrd to QEMU, leaving the option to pass an
actual initrd or initramfs module later on with multiboot.
@supercomputer7
Copy link
Member Author

supercomputer7 commented May 14, 2024

When merged, this should close #18558.

Copy link
Member

@timschumi timschumi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work fine, let's reland this.

@timschumi timschumi merged commit d068af8 into SerenityOS:master May 14, 2024
12 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants