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

Final PVH fixups #4073

Merged
merged 6 commits into from
Nov 11, 2024
Merged

Final PVH fixups #4073

merged 6 commits into from
Nov 11, 2024

Conversation

roypat
Copy link
Contributor

@roypat roypat commented Aug 24, 2023

Bring the feature branch up-to-date with all changes that happened in main since it was last updated, and which didn't show up as a merge conflict. Also do some minor doc fixes, and add integration testing.

Since these days all our CI kernels are compiled with CONFIG_PVH=y, integration testing is just a matter of adding an assert to verify that PVH boot mode is indeed being used. We are not adding FreeBSD tests to our CI, as we have no easy means for maintaining FreeBSD artifacts, and last time I checked (which admittedly was August 23), it also didn't boot on AMD instances.

See also #3041

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following
Developer Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • 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.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet contribution quality standards.

  • This functionality cannot be added in rust-vmm.

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.08%. Comparing base (142c9b6) to head (f84e347).
Report is 6 commits behind head on feature/pvh.

Additional details and impacted files
@@               Coverage Diff               @@
##           feature/pvh    #4073      +/-   ##
===============================================
- Coverage        84.86%   84.08%   -0.79%     
===============================================
  Files              223      251      +28     
  Lines            25772    28274    +2502     
===============================================
+ Hits             21871    23773    +1902     
- Misses            3901     4501     +600     
Flag Coverage Δ
5.10-c5n.metal 84.66% <100.00%> (-0.01%) ⬇️
5.10-m5n.metal 84.65% <100.00%> (-0.01%) ⬇️
5.10-m6a.metal 83.96% <100.00%> (-0.02%) ⬇️
5.10-m6g.metal 80.68% <100.00%> (?)
5.10-m6i.metal 84.64% <100.00%> (-0.01%) ⬇️
5.10-m7g.metal 80.68% <100.00%> (?)
6.1-c5n.metal 84.66% <100.00%> (-0.01%) ⬇️
6.1-m5n.metal 84.65% <100.00%> (-0.01%) ⬇️
6.1-m6a.metal 83.96% <100.00%> (-0.02%) ⬇️
6.1-m6g.metal 80.68% <100.00%> (?)
6.1-m6i.metal 84.64% <100.00%> (-0.01%) ⬇️
6.1-m7g.metal 80.68% <100.00%> (?)

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

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

@roypat roypat force-pushed the pvh-tests branch 2 times, most recently from de3083a to 8be04a4 Compare August 24, 2023 14:55
@roypat roypat force-pushed the pvh-tests branch 3 times, most recently from ce3237e to b096346 Compare August 25, 2023 13:27
@roypat roypat force-pushed the pvh-tests branch 7 times, most recently from 18cdd9a to 9a2db96 Compare September 7, 2023 14:40
@roypat roypat force-pushed the pvh-tests branch 4 times, most recently from 1efb582 to 64b1bb5 Compare September 7, 2023 15:15
@roypat roypat marked this pull request as ready for review September 7, 2023 15:15
@roypat roypat marked this pull request as draft September 11, 2023 13:07
@roypat
Copy link
Contributor Author

roypat commented Sep 11, 2023

I'm putting this back into draft until the initrd issues are resolved

Enabling CONFIG_XEN_PVH=y is for booting on the actual Xen hypervisor.
To boot using PVH on a non-Xen hypervisor, set CONFIG_PVH=y.

Signed-off-by: Patrick Roy <[email protected]>
The feature branch is so old that during rebase it got moved into the
1.8 section. Move it back to the unreleased section.

Signed-off-by: Patrick Roy <[email protected]>
With PVH support, we are passing around an `EntryPoint` structure, not
just an address. Extract the address from the struct, and pass it to
gdb::gdb_thread instead to fix compilation.

A aarch64 unittest was missing a `GuestAddress` import.

Signed-off-by: Patrick Roy <[email protected]>
@roypat roypat force-pushed the pvh-tests branch 2 times, most recently from 316b7ea to b5eaf92 Compare November 11, 2024 13:17
@roypat roypat changed the title Add PVH related integration tests Final PVH fixups Nov 11, 2024
Mainly clippy and mdformat have changed since the feature branch was
created.

Signed-off-by: Patrick Roy <[email protected]>
Use test_api_happy_start to assert that the log message that indicates
usage of PVH boot protocol is present.

Only x86_64 guests support PVH boot, and on ARM we do not emit any log
messages, so restrict the assertion to x86_64 platforms.

Signed-off-by: Patrick Roy <[email protected]>
Hardcoding main means the test looks at the wrong range of commits for
feature branches, which can result in problem if feature branches are
long-lived.

Signed-off-by: Patrick Roy <[email protected]>
@roypat roypat marked this pull request as ready for review November 11, 2024 13:46
@roypat roypat added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Nov 11, 2024
@roypat roypat merged commit 3b5d49a into firecracker-microvm:feature/pvh Nov 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting review Indicates that a pull request is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants