-
Notifications
You must be signed in to change notification settings - Fork 1.9k
loader: add support for initrd #1246
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
Conversation
Disclaimer: I am complete rust n00b! Missing things:
|
For x86_64 I am now loading the initrd at the end of the available low memory, instead of using a fixed address. |
Hi, x86_64 is now ready for review. I just need a new fixture added to S3 for initrd testing. Do you expect aarch64 be supported too, or can it be postponed in a later PR? |
I would suggest merging and postponing aarch64, so that x86_64 people can test sooner (more easily). |
e567983
to
f87dc88
Compare
Spent some more hours and added |
b6d1c34
to
a0321f5
Compare
Update: aarch64 is functional. |
58eb8b1
to
43a90ee
Compare
@marcov do you need help in addressing the outstanding review comments? |
Thank you for the ping @andreeaflorescu, pushed some updates now. |
@marcov I created a prototype on my branch on top of your commits: andreeaflorescu@994b23d. The To give you more context, it is important to have the loading of initrd in the |
The
we would make the kernel crate dependent on memory layout representation for a specific platform. Does not that break the separation of concerns we strive for?
|
Taking |
@marcov that was my initial thought as well and noticed the same problem as you. Let's keep it as is for now and we will find a better place for it in a future iteration. I am still a bit concerned regarding the rather large boot time on aarch64. I was thinking about doing some comparison with Qemu boot times (if they have something similar we can compare against). I can check that tomorrow. |
@andreeaflorescu using a non-gzipped initrd will shave off around 100ms, but even doing that we are still far aways from the max time of 160ms.
|
On the a1.metal aarch64 machine (9 MB initrd), things look like this:
|
@dianpopa try to |
If it is expected for the boot time to be so large with the provided initrd image, we can merge the PR as is. I didn't really have time to investigate this, and I don't want to block the PR. Feel free to update the time for aarch64 to a value that you're confident with. |
@dianpopa: right! An I'm going to do a final squash and set a reasonable boot time for aarch64, checking for |
Add the Serial class for serial console communication. Signed-off-by: Marco Vedovati <[email protected]>
Add support for booting the micro VM with a user provided initrd file: - For x86_64, the initrd is loaded at the end of the low (4G) memory, and passed to the kernel in the bootparams table. - For aarch64, the initrd is loaded in a memory segment just before the FDT. The initrd start and end addresses are specified in the FDT "/chosen" node. Users can specify an initrd file using the "initrd_path" property of the "boot-source" the REST API. This also adds integration tests to verify VMs boot with initrd and without a rootfs. Categories of tests added: - Functional: the VM shall boot and the "/" path should be mounted as "rootfs" filesystem type. - Performance: VM shall boot in less than 160ms (for x86). Co-developed-by: Marco Vedovati <[email protected]> Co-developed-by: Tim Deegan <[email protected]> Signed-off-by: Marco Vedovati <[email protected]>
Happy to see this merged! |
Thanks for the effort and responsiveness @marcov ! Happy holidays! |
When firecracker-microvm#1246 was merged, the changelog entry was added under 0.19.0. This seems like a mistake as 0.19.0 was already released. Signed-off-by: Angus McInnes <[email protected]>
When PR firecracker-microvm#1246 was merged, the changelog entry was added under 0.19.0. This seems like a mistake as 0.19.0 was already released. Signed-off-by: Angus McInnes <[email protected]>
When PR #1246 was merged, the changelog entry was added under 0.19.0. This seems like a mistake as 0.19.0 was already released. Signed-off-by: Angus McInnes <[email protected]>
When PR firecracker-microvm#1246 was merged, the changelog entry was added under 0.19.0. This seems like a mistake as 0.19.0 was already released. Signed-off-by: Angus McInnes <[email protected]>
Reason for This PR
The 2020 Roadmap welcomes contributions that adds initrd support. A previous PR was opened to add it (#670), but cannot be reused as is, because it also added support for loading a compressed kernel.
Hence #670 was reworked and rebased to only include initrd support.
Fixes #208 .
Description of Changes
initrd_path
property to theboot-source
API.ramdisk_image
(memory offset) andramdisk_size
to the guest bootparams table.load_initrd
loader function, to load the initrd file content into the guest memoryat a (for the moment) fixed addressat then end of the available low memory (for x86_64).License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.
PR Checklist
[Author TODO: Meet these criteria. Where there are two options, keep one.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]
git commit -s
).clearly provided.
doc changes are included in this PR. Docs in scope are all
*.md
fileslocated either in the repository root, or in the
docs/
directory.code is included in this PR.
reflected in
firecracker/swagger.yaml
.this PR have user impact and have been added to the
CHANGELOG.md
file.