Add support for PVH direct boot API #3155

cperciva · 2022-10-02T19:41:06Z

Changes

This updates patches submitted two years ago (#1818). Some of that work ended up in linux-loader; much of the rest needed to be refactored to adjust for changes in the tree.

As this is my first significant contribution to Firecracker (and my first time using Rust!) this PR probably requires more careful review than most. Also, I haven't updated any documentation or added integration tests; I'm guessing it would be good to have a "does FreeBSD boot" test? Happy to do more work here but I'll need someone to lead me through the process.

Reason

Supporting PVH boot makes it possible for FreeBSD to boot in Firecracker. (cf. #3041).

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

The original author of the patches (now credited as co-authored-by) provided the same confirmation in his original PR.

PR Checklist

All commits in this PR are signed (git commit -s).
If a specific issue led to this PR, this PR closes the issue.
The description of changes is clear and encompassing.
Any required documentation changes (code and docs) are included in this PR.
[N/A] New unsafe code is documented.
[N/A] API changes follow the Runbook for Firecracker API changes.
User-facing changes are mentioned in CHANGELOG.md.
All added/changed functionality is tested.

This functionality can be added in rust-vmm.

cperciva · 2022-10-02T19:44:14Z

When I ran devtool test I had a few failures, most of which seemed like problems in the test framework. Two issues which I wasn't sure what to do about:

Non 'Co-authored-by:' string found after 1st 'Co-authored-by:'

I made sure the Co-authored-by was the last thing in each commit message so I'm guessing there's a gitlint bug here?

AssertionError: Files ['../src/arch/src/x86_64/gdt.rs', '../src/arch/src/x86_64/mod.rs', '../src/arch/src/x86_64/regs.rs'] have invalid licenses

These files gained Oracle copyrights which I'm guessing the test is objecting to?

cperciva · 2022-10-03T06:04:33Z

One user-facing change which I wasn't sure about: With this code, if a kernel specifies a PVH entry point, Firecracker will use it. If a kernel has both regular and PVH entry points, we could theoretically see a change in behaviour.

My inclination is to say that this is the right thing to do, but if the Firecracker core team prefers I'm totally fine with it being an option (probably specified at the same time as the kernel is specified, via API or JSON). I'd have to dig into the code more to figure out how to do this, though.

src/arch/src/x86_64/mod.rs

cperciva · 2022-10-03T19:16:10Z

Updated patch removed a few vestigal lines from the original work two years ago.

cperciva · 2022-10-05T19:23:05Z

Rebased on latest main in order to pick up the gitlint fix.

cperciva · 2022-11-23T00:13:31Z

Rebased to latest main; minor tweaks needed to keep clippy happy.

cperciva · 2022-12-29T00:28:19Z

Rebased on the latest main.

cperciva · 2022-12-31T01:12:35Z

Fixed a couple more style nits and added a commit to allow the Oracle copyright.

theoparis · 2023-03-04T07:29:07Z

Any updates on this? There seems to be conflicts now 😕

cperciva · 2023-03-04T15:30:15Z

Any updates on this? There seems to be conflicts now

Sorry, I've been busy with FreeBSD release engineering -- I promise I'll get back to this so it can get merged!

roypat · 2023-07-28T08:07:45Z

Hey Colin, thanks for being so patient with us. As talked about on Slack, I've pointed this PR towards a feature branch so that we can get this PR merged without having to wait for the ci artifacts work to complete. That way you won't have to continuously rebase this PR. I'll be adding all required testing (freebsd boot test using artifacts build using your instructions, a test booting linux with pvh and a performance test to for booting linux with pvh) to the feature branch later once that work is unblocked, and if all goes well we might be able to land this for the next firecracker release 🥳

CHANGELOG.md

bchalios

This looks great! I just had some style-related comments. I want to go through the boot protocol again a bit more closely, but I couldn't find a proper resource describing the ABI.
For example, I couldn't find (easily) info about the GDT setup you're doing and the memory layout. (If you have other links, it would be useful).

src/vmm/src/builder.rs

src/vmm/src/vstate/vcpu/aarch64.rs

src/vmm/src/builder.rs

src/vmm/src/arch/x86_64/gdt.rs

src/vmm/src/arch/x86_64/mod.rs

In order to properly configure the initial vCPU register state and boot parameters in guest memory, we must specify which boot protocol to use with the kernel entry point address. On x86-64 (the only architecture where multiple boot protocols are supported) we print the protocol used to load the kernel at the debug log level. Create an EntryPoint struct that contains the required information. This structure will later be used in the vCPU configuration methods to set the appropriate initial conditions for the guest. This commit also splits the load_kernel function into an x86-64 specific version and an aarch64 specific version. Signed-off-by: Colin Percival <[email protected]> Co-authored-by: Alejandro Jimenez <[email protected]>

Set the initial values of the KVM vCPU registers as specified in the PVH boot ABI: https://xenbits.xen.org/docs/unstable/misc/pvh.html Add stub bits for aarch64; PVH mode does not exist there. Signed-off-by: Colin Percival <[email protected]> Co-authored-by: Alejandro Jimenez <[email protected]>

Fill the hvm_start_info and related structures as specified in the PVH boot protocol. Write the data structures to guest memory at the GPA that will be stored in %rbx when the guest starts. Signed-off-by: Colin Percival <[email protected]> Co-authored-by: Alejandro Jimenez <[email protected]>

The PVH boot support bits are under Oracle copyright. Signed-off-by: Colin Percival <[email protected]>

While I'm here, clarify that the existing instructions are for building a Linux kernel and rootfs. Signed-off-by: Colin Percival <[email protected]>

Brief description of the PVH boot mode. We defer to Xen for technical details of how CPU registers are set up upon kernel entry. Signed-off-by: Colin Percival <[email protected]>

Firecracker now supports PVH boot as an alternative to "Linux" boot on the x86_64 architecture. This makes it possible for FreeBSD to boot, and also affects how Linux kernels compiled with the CONFIG_XEN_PVH=y option boot. Signed-off-by: Colin Percival <[email protected]>

cperciva · 2023-08-12T12:22:12Z

Patches updated and rebased on the latest main.

codecov · 2023-08-14T07:50:56Z

Codecov Report

Patch coverage: 86.72% and project coverage change: -0.01% ⚠️

Comparison is base (1a2c6ad) 82.65% compared to head (a6639ad) 82.65%.

Additional details and impacted files

@@               Coverage Diff               @@
##           feature/pvh    #3155      +/-   ##
===============================================
- Coverage        82.65%   82.65%   -0.01%     
===============================================
  Files              221      221              
  Lines            28169    28330     +161     
===============================================
+ Hits             23283    23416     +133     
- Misses            4886     4914      +28

Flag	Coverage Δ
4.14-c7g.metal	`78.07% <66.66%> (-0.03%)`	⬇️
4.14-m5d.metal	`79.90% <86.66%> (+<0.01%)`	⬆️
4.14-m6a.metal	`79.05% <86.66%> (+0.01%)`	⬆️
4.14-m6g.metal	`78.07% <66.66%> (-0.03%)`	⬇️
4.14-m6i.metal	`79.89% <86.66%> (+0.01%)`	⬆️
5.10-c7g.metal	`81.02% <66.66%> (-0.03%)`	⬇️
5.10-m5d.metal	`82.57% <86.66%> (-0.01%)`	⬇️
5.10-m6a.metal	`81.82% <86.66%> (+<0.01%)`	⬆️
5.10-m6g.metal	`81.02% <66.66%> (-0.03%)`	⬇️
5.10-m6i.metal	`82.57% <86.66%> (+<0.01%)`	⬆️
6.1-c7g.metal	`81.02% <66.66%> (-0.03%)`	⬇️
6.1-m5d.metal	`82.57% <86.66%> (-0.01%)`	⬇️
6.1-m6a.metal	`81.82% <86.66%> (+<0.01%)`	⬆️
6.1-m6g.metal	`81.02% <66.66%> (-0.03%)`	⬇️
6.1-m6i.metal	`82.57% <86.66%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/vmm/src/arch/mod.rs	`28.57% <0.00%> (-51.43%)`	⬇️
src/vmm/src/vstate/vcpu/mod.rs	`85.38% <ø> (ø)`
src/vmm/src/arch/x86_64/regs.rs	`92.25% <87.71%> (-4.27%)`	⬇️
src/vmm/src/builder.rs	`92.84% <90.32%> (-0.22%)`	⬇️
src/vmm/src/arch/x86_64/mod.rs	`94.17% <91.17%> (-3.15%)`	⬇️
src/vmm/src/arch/x86_64/gdt.rs	`98.41% <100.00%> (+0.16%)`	⬆️
src/vmm/src/vstate/vcpu/aarch64.rs	`96.07% <100.00%> (ø)`
src/vmm/src/vstate/vcpu/x86_64.rs	`88.85% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cperciva · 2023-08-14T14:57:00Z

Thank you!

bchalios · 2023-08-14T15:09:39Z

Thank you!

Actually, thank you Colin. I will work with Patrick to add the proper testing, so that we can get this merged to main ASAP.

mcgaw · 2023-09-04T17:05:41Z

@cperciva Many thanks for this. Can I ask a (perhaps silly) question? I have a FreeBSD CI pipeline that currently uses VirtualBox, and is a bit of a pain for several reasons. It uses jails. Is there any limitation around jails and vnet stacks imposed by using firecracker VMs?

cperciva · 2023-09-04T17:11:08Z

@mcgaw Firecracker won't be aware of your jails, and your jails won't know that their host is running inside Firecracker. If you're using multiple IP addresses you might need some networking magic on the Linux host to get all of those routed into the FreeBSD/Firecracker guest, but aside from that it should just work.

mcgaw · 2023-09-04T19:09:18Z

That's brilliant - I'm looking forward to road testing it. Thanks!

iMilnb · 2024-10-14T05:34:00Z

Has this work been merged yet? FYI this allows to boot both FreeBSD and NetBSD on firecracker.

andreeaflorescu reviewed Oct 3, 2022

View reviewed changes

src/arch/src/x86_64/mod.rs Outdated Show resolved Hide resolved

cperciva force-pushed the pvh-v3 branch from 6e572c1 to ffc49e0 Compare October 3, 2022 19:14

dianpopa assigned cperciva Oct 4, 2022

cperciva mentioned this pull request Oct 5, 2022

gitlint: Fix Co-authored-by handling #3165

Closed

3 tasks

cperciva force-pushed the pvh-v3 branch from ffc49e0 to e007868 Compare October 5, 2022 19:22

cperciva force-pushed the pvh-v3 branch 2 times, most recently from 61c5aa0 to 2914c95 Compare November 23, 2022 00:10

alxiord self-requested a review November 24, 2022 09:36

cperciva force-pushed the pvh-v3 branch from 2914c95 to bcbd1f5 Compare December 29, 2022 00:28

cperciva force-pushed the pvh-v3 branch 3 times, most recently from a46191f to 74e7102 Compare December 29, 2022 00:39

bchalios added Feature: Guest OS Status: Awaiting review Indicates that a pull request is ready to be reviewed labels Dec 29, 2022

cperciva force-pushed the pvh-v3 branch 3 times, most recently from 399dc9e to 9421770 Compare December 31, 2022 01:12

JonathanWoollett-Light added Type: Enhancement Indicates new feature requests and removed Feature: Guest OS labels Mar 23, 2023

cperciva force-pushed the pvh-v3 branch from 02d53a8 to 9421770 Compare June 15, 2023 00:15

cperciva force-pushed the pvh-v3 branch from 9421770 to cf16fea Compare July 1, 2023 20:54

roypat reviewed Jul 28, 2023

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

roypat added Status: Awaiting review Indicates that a pull request is ready to be reviewed and removed Status: Awaiting review Indicates that a pull request is ready to be reviewed labels Jul 28, 2023

roypat requested review from bchalios and roypat and removed request for alxiord July 28, 2023 11:21

bchalios reviewed Jul 28, 2023

View reviewed changes

cperciva and others added 7 commits August 12, 2023 05:18

test_licenses: Accept Oracle copyright

a31e862

The PVH boot support bits are under Oracle copyright. Signed-off-by: Colin Percival <[email protected]>

Add FreeBSD kernel+rootfs build instructions

5451532

While I'm here, clarify that the existing instructions are for building a Linux kernel and rootfs. Signed-off-by: Colin Percival <[email protected]>

Add docs/pvh.md

97e9766

Brief description of the PVH boot mode. We defer to Xen for technical details of how CPU registers are set up upon kernel entry. Signed-off-by: Colin Percival <[email protected]>

cperciva force-pushed the pvh-v3 branch from c68363c to a6639ad Compare August 12, 2023 12:18

roypat changed the base branch from feature/pvh to main August 14, 2023 07:45

roypat changed the base branch from main to feature/pvh August 14, 2023 07:45

roypat approved these changes Aug 14, 2023

View reviewed changes

bchalios approved these changes Aug 14, 2023

View reviewed changes

bchalios merged commit 9fa869b into firecracker-microvm:feature/pvh Aug 14, 2023

rinor mentioned this pull request Apr 24, 2024

feat(vmm): MPS support when ACPI is not supported/available nanovms/nanos#1840

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for PVH direct boot API #3155

Add support for PVH direct boot API #3155

cperciva commented Oct 2, 2022 •

edited

Loading

cperciva commented Oct 2, 2022

cperciva commented Oct 3, 2022

cperciva commented Oct 3, 2022

cperciva commented Oct 5, 2022

cperciva commented Nov 23, 2022

cperciva commented Dec 29, 2022

cperciva commented Dec 31, 2022

theoparis commented Mar 4, 2023

cperciva commented Mar 4, 2023

roypat commented Jul 28, 2023

bchalios left a comment

cperciva commented Aug 12, 2023

codecov bot commented Aug 14, 2023 •

edited

Loading

cperciva commented Aug 14, 2023

bchalios commented Aug 14, 2023

mcgaw commented Sep 4, 2023

cperciva commented Sep 4, 2023

mcgaw commented Sep 4, 2023

iMilnb commented Oct 14, 2024

Add support for PVH direct boot API #3155

Add support for PVH direct boot API #3155

Conversation

cperciva commented Oct 2, 2022 • edited Loading

Changes

Reason

License Acceptance

PR Checklist

cperciva commented Oct 2, 2022

cperciva commented Oct 3, 2022

cperciva commented Oct 3, 2022

cperciva commented Oct 5, 2022

cperciva commented Nov 23, 2022

cperciva commented Dec 29, 2022

cperciva commented Dec 31, 2022

theoparis commented Mar 4, 2023

cperciva commented Mar 4, 2023

roypat commented Jul 28, 2023

bchalios left a comment

Choose a reason for hiding this comment

cperciva commented Aug 12, 2023

codecov bot commented Aug 14, 2023 • edited Loading

Codecov Report

cperciva commented Aug 14, 2023

bchalios commented Aug 14, 2023

mcgaw commented Sep 4, 2023

cperciva commented Sep 4, 2023

mcgaw commented Sep 4, 2023

iMilnb commented Oct 14, 2024

cperciva commented Oct 2, 2022 •

edited

Loading

codecov bot commented Aug 14, 2023 •

edited

Loading