[[do not merge]] PR to check CI of rebase feature/secret-hiding branch #5171

roypat · 2025-04-25T13:21:23Z

No description provided.

Creating a script to build and install a modified kernel with patches applied. Signed-off-by: Jack Thomson <[email protected]>

Adding a new integration test to assert that the kernel build script will succeed. Signed-off-by: Jack Thomson <[email protected]>

Adding the secret hiding kernel as a default for the buildkite pipeline, this will mean that PR's made against the branch will now be run with the new secret hiding enabled amis. Some tests have been marked to skip as they are kernel dependent so while we are compiling our kernel in CI these could change again. Signed-off-by: Jack Thomson <[email protected]>

To make it easier to track the upstream kernels which may change as we rebase, let's mark kernels newer than 6.12 as next for now to make dashboarding easier. Signed-off-by: Jack Thomson <[email protected]>

This test is failing for ARM instances in our performance pipeline, skipping this for now until we resolve the issue. Signed-off-by: Jack Thomson <[email protected]>

Addressing a comment to move away from dir stacks in our install scripts. We now store the start directly before we move the build directory and cd back to that explicitly. Signed-off-by: Jack Thomson <[email protected]>

We previously skipped a huge page test which stoppped the test timing out, but now we get stuck on another. Skipping this and the other huge page snapshot tests in the file. Signed-off-by: Jack Thomson <[email protected]>

Run the kernel build as part of our nightly tests so we can monitor it's success. Signed-off-by: Jack Thomson <[email protected]>

Add an updated version of relevant patches from my v4 direct map removal series [1]. Updated here means - Drop all selftests patches, as they are irrelevant for our CI - Address comments from David about squashing commits - Rebase on top of Fuad's v7 [1]: https://lore.kernel.org/kvm/[email protected]/ Signed-off-by: Patrick Roy <[email protected]>

The patches are in the `patches` subdirectory of `hiding_ci`, so if only patches were added, then the check of "any files with parent directory `hiding_ci`" would be false, and the CI step for testing the build of patches wouldn't actually run. Fix this by updating the check to be "any files where any parent directory is `hiding_ci`", which will also catch patches. Reported-by: Jack Thomson <[email protected]> Signed-off-by: Patrick Roy <[email protected]>

Update the build script to allow us to install the secret hidden kernels onto Amazon Linux 2023 instances. We have to as part of this include a script to download and install ena drivers for the instance to allow us to boot. Signed-off-by: Jack Thomson <[email protected]>

The output from the build in x86 is archived so updated the script to support installing either output type from the build Signed-off-by: Jack Thomson <[email protected]>

Add an 'apt update' before `apt install`. Otherwise, we might hold an old view of the package versions and installation might fail. Signed-off-by: Babis Chalios <[email protected]>

This lint forbids using `..Default::default()` in struct initializers after all fields have already been initialized, but this is a useful pattern if you know you want to add more fields to a struct in a future PR without needing to touch a ton of initializers in unittests again (_heavy foreshadowing_). So silence the paperclip. Signed-off-by: Patrick Roy <[email protected]>

There's no need to test this through VmResources when it can be tested in isolation. Also, everytime I touch MachineConfig I get confsued by where the hell the tests are, cuz not only are they in a different module, they're also one directory level away. So move the tests into machine_config.rs, where it makes sense to have them. Signed-off-by: Patrick Roy <[email protected]>

codecov · 2025-04-25T14:27:01Z

Codecov Report

Attention: Patch coverage is 60.47431% with 200 lines in your changes missing coverage. Please review.

Project coverage is 82.64%. Comparing base (ae078ee) to head (dc53e8d).

Files with missing lines	Patch %	Lines
src/vmm/src/resources.rs	37.23%	59 Missing ⚠️
src/vmm/src/vstate/vm.rs	54.11%	39 Missing ⚠️
src/vmm/src/devices/virtio/net/device.rs	39.62%	32 Missing ⚠️
src/vmm/src/builder.rs	57.50%	17 Missing ⚠️
src/vmm/src/vstate/memory.rs	89.81%	11 Missing ⚠️
src/vmm/src/devices/virtio/block/device.rs	0.00%	10 Missing ⚠️
src/vmm/src/devices/virtio/block/virtio/device.rs	36.36%	7 Missing ⚠️
.../vmm/src/devices/virtio/block/vhost_user/device.rs	0.00%	6 Missing ⚠️
src/vmm/src/devices/virtio/vsock/unix/muxer.rs	54.54%	5 Missing ⚠️
src/vmm/src/devices/virtio/balloon/device.rs	50.00%	3 Missing ⚠️
... and 5 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5171      +/-   ##
==========================================
- Coverage   83.01%   82.64%   -0.37%     
==========================================
  Files         250      250              
  Lines       26897    27296     +399     
==========================================
+ Hits        22328    22559     +231     
- Misses       4569     4737     +168

Flag	Coverage Δ
5.10-c5n.metal	`83.06% <59.16%> (-0.50%)`	⬇️
5.10-m5n.metal	`83.06% <59.16%> (-0.50%)`	⬇️
5.10-m6a.metal	`82.28% <59.16%> (-0.50%)`	⬇️
5.10-m6g.metal	`78.86% <57.63%> (-0.49%)`	⬇️
5.10-m6i.metal	`83.06% <59.16%> (-0.50%)`	⬇️
5.10-m7a.metal-48xl	`82.26% <59.16%> (?)`
5.10-m7g.metal	`78.86% <57.63%> (-0.49%)`	⬇️
5.10-m7i.metal-24xl	`83.02% <59.16%> (?)`
5.10-m7i.metal-48xl	`83.02% <59.16%> (?)`
5.10-m8g.metal-24xl	`78.86% <57.63%> (?)`
5.10-m8g.metal-48xl	`78.85% <57.63%> (?)`
6.1-c5n.metal	`83.11% <59.16%> (-0.50%)`	⬇️
6.1-m5n.metal	`83.11% <59.16%> (-0.50%)`	⬇️
6.1-m6a.metal	`82.33% <59.16%> (-0.51%)`	⬇️
6.1-m6g.metal	`78.86% <57.63%> (-0.49%)`	⬇️
6.1-m6i.metal	`83.11% <59.16%> (-0.50%)`	⬇️
6.1-m7a.metal-48xl	`82.31% <59.16%> (?)`
6.1-m7g.metal	`78.86% <57.63%> (-0.49%)`	⬇️
6.1-m7i.metal-24xl	`83.12% <59.16%> (?)`
6.1-m7i.metal-48xl	`83.12% <59.16%> (?)`
6.1-m8g.metal-24xl	`78.85% <57.63%> (?)`
6.1-m8g.metal-48xl	`78.86% <57.63%> (?)`
6.14-c5n.metal	`83.08% <57.76%> (?)`
6.14-m5n.metal	`83.07% <57.76%> (?)`
6.14-m6a.metal	`82.29% <57.76%> (?)`
6.14-m6g.metal	`78.81% <55.82%> (?)`
6.14-m6i.metal	`83.07% <57.76%> (?)`
6.14-m7a.metal-48xl	`82.28% <57.76%> (?)`
6.14-m7g.metal	`78.82% <55.82%> (?)`
6.14-m7i.metal-24xl	`83.09% <57.76%> (?)`
6.14-m7i.metal-48xl	`83.09% <57.76%> (?)`
6.14-m8g.metal-24xl	`78.82% <55.82%> (?)`
6.14-m8g.metal-48xl	`78.82% <55.82%> (?)`

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

With secret freedom, direct accesses to guest memory from the context of the host kernel are no longer possible. This particularly means that we cannot pass pointers to guest memory to the host kernel anymore (at least if the host kernel tries to GUP them). For these scenarios, introduce a utility decorator struct `MaybeBounce` that can optionally do indirect read and write syscalls on guest memory by first memcpy-ing to firecracker userspace, and passing a pointer to firecracker heap memory into the kernel instead. Signed-off-by: Patrick Roy <[email protected]>

This is particularly useful for virtio devices, where on-demand allocation of bounce buffers leads to sever performance impacts (~80%) in synthetic throughput tests. Additionally, for virtio devices we can know approximately what the optimal size of a statically allocated bounce buffer is. Allocate bounce buffers on the heap, as trying to even temporarily place a 65k bounce buffer on the stack can lead to stack overflow errors. Signed-off-by: Patrick Roy <[email protected]>

Add support to our virtio devices to allow userspace bounce buffering of virtio buffers. This is an alternative to swiotlb. Don't implement it for vhost-user-blk and for virtio-block with async engine, because I have no idea how that would even work. Signed-off-by: Patrick Roy <[email protected]>

If the CI artifacts dont contain old firecracker releases, still succeed at setting them up after downloading them. Signed-off-by: Patrick Roy <[email protected]>

Add a utility function for creating a guest_memfd and wrapping it into a `File` object. Signed-off-by: Patrick Roy <[email protected]>

There's be a lot more things that are incompatible going forward (mostly related to secret freedom), so instead of adding a ton of error variants for each pair of incompatible features, let's just have a single one where we can insert arbitrary features via a string argument. Signed-off-by: Patrick Roy <[email protected]>

This will later indicate to Firecracker that guest memory should be backed by guest_memfd. Mark vhost-user and async block engine as incompatible, as I/O will require userspace bounce buffers. For vhost-user-blk, we would need to communicate the need for bounce buffers to the backend somehow, and for the async block engine we would need to somehow keep the bounce buffers around until io_uring finishes requests (which is not impossible, but complicated and not needed for now). Signed-off-by: Patrick Roy <[email protected]>

If secret freedom is enabled, the guest kernel and potential initrd needs to be loaded via bounce buffer, as we cannot directly do `read` syscalls that target guest memory. Signed-off-by: Patrick Roy <[email protected]>

Needed because we cannot do I/O straight into secret hidden memory - the host kernel cannot access it. Signed-off-by: Patrick Roy <[email protected]>

Fall back to kvm_user_memory_region in case the 2 version of the struct isnt supported. Signed-off-by: Patrick Roy <[email protected]>

vm-memory has faulty validation logic that prevents us from mmap-ing guest_memfds, so just bypass that by calling mmap ourselves for the time being. See also rust-vmm/vm-memory#320 Signed-off-by: Patrick Roy <[email protected]>

Have the `struct Vm` constructor take an argument to indicate whether the VM should be secret free. Use this to determine the correct vm type for guest_memfd support, and store it inside the VM so that we don't have to pass bools to various functions. Signed-off-by: Patrick Roy <[email protected]>

If the `secret_free` field of the memory_config is set to true in the /machine-config endpoint, back all memory regions using guest_memfd. For our setup, this means both setting the guest_memfd[_offset] fields in kvm_user_memory_region2, as well as mmaping the guest memory and reflecting this VMA back into the memslot's userspace_addr (which is how kvm internal accesses to guest memory will work for these guest_memfd regions, such as mmio emulation on x86). Signed-off-by: Patrick Roy <[email protected]>

To take snapshots of secret hidden VMs, we need to bounce guest memory through a userspace buffer. Reuse the `Bounce` wrapper type that is already in use for loading the guest kernel / initrd. Signed-off-by: Patrick Roy <[email protected]>

The current version of the mmap-support patches require that on x86, memory attributes have to be set to private even if the guest_memfd VMA is short-circuited back into the memslot (on ARM, memory attributes are not even supported in this scenario). Signed-off-by: Patrick Roy <[email protected]>

kvm-clock is incompatible with direct map removal for now. This is because kvm-clock tries to access guest memory through the direct map. Additionally, it does not handle failures during guest-attempted activations of kvm-clock gracefully (e.g. it cannot/does not communicate these back to the guest). This means a guest will unconditionally assume that if it wrote to the kvm-clock MSR to activate kvm-clock, it will work. But if KVM internally fails to activate kvm-clock, KVM will never write the information the guest expects into guest memory, resulting in the guest reading garbage data (generally, zeros), resulting in division by zero panics in the guest. Hence, explicitly tells guests that they shouldn't even try to enable kvm-clock, if they value their lives. Signed-off-by: Patrick Roy <[email protected]>

Previously this would fail on x86 as we set -e. By setting the || true this means the script will continue. The grubby step next will fail if it failed to find the image. Signed-off-by: Jack Thomson <[email protected]>

This is to allow to keep the licence and readme files in the patches directory. Signed-off-by: Nikita Kalyazin <[email protected]>

Explicitly mention that Linux patches are distributed under the GPL-2.0 licence. Signed-off-by: Nikita Kalyazin <[email protected]>

Include the following patch series rebased on top of the v7 of "KVM: Mapping guest_memfd backed memory at the host for software protected VMs" (https://<lor>/kvm/[email protected]/, replace "<lor>" with "lore.kernel.org" in this and the following links): - v4 "Direct Map Removal for guest_memfd" (https://<lor>/kvm/[email protected]/), with fixups - v2 "KVM: Introduce KVM Userfault" (https://<lor>/kvm/[email protected]/) - v3 "KVM: guest_memfd: use write for population" (https://<lor>/kvm/[email protected]/) - v3 "KVM: guest_memfd: support for uffd minor" (https://<lor>/kvm/[email protected]/), with fixups After this change all patches are represented as plain text files, meaning no patches are required to be fetched via a lore link. Signed-off-by: Nikita Kalyazin <[email protected]>

This is to keep Linux patches separate in case we need to store some other patches at some point. Signed-off-by: Nikita Kalyazin <[email protected]>

Also strip doc and test patches committed by mistake last time. Signed-off-by: Nikita Kalyazin <[email protected]>

Aadditionally parametrize some of our throughput performance tests (network, block and vsock) by memory config, so that they run with secret freedom (and hence bounce buffering) enabled. Also add it to the boottime test, because bouncing can impact the time taken to read the rootfs. Skip them on m6g.metal because secret freedom does not work here for architectural reasons (and our patches do not take this into account, so trying to use secret freedom here would result in host kernel panics). Signed-off-by: Patrick Roy <[email protected]>

Add a test that we can boot VMs and initrds with secret freedom enabled. Signed-off-by: Patrick Roy <[email protected]>

Since we load the kernel using bounce buffers now, it will give us false-positives. Signed-off-by: Patrick Roy <[email protected]>

JackThomson2 and others added 15 commits April 25, 2025 14:19

ci: Create script for installing custom kernel

d43d3bd

Creating a script to build and install a modified kernel with patches applied. Signed-off-by: Jack Thomson <[email protected]>

test: Add test for kernel build

130080c

Adding a new integration test to assert that the kernel build script will succeed. Signed-off-by: Jack Thomson <[email protected]>

tests: Mark kernels newer than 6.12 as next

421da22

To make it easier to track the upstream kernels which may change as we rebase, let's mark kernels newer than 6.12 as next for now to make dashboarding easier. Signed-off-by: Jack Thomson <[email protected]>

tests: Skip more huge page tests on ARM

d59cbb7

This test is failing for ARM instances in our performance pipeline, skipping this for now until we resolve the issue. Signed-off-by: Jack Thomson <[email protected]>

ci: Move away from using dir stacks

3129768

Addressing a comment to move away from dir stacks in our install scripts. We now store the start directly before we move the build directory and cd back to that explicitly. Signed-off-by: Jack Thomson <[email protected]>

test: Skip final huge page tests

cfbe7f2

We previously skipped a huge page test which stoppped the test timing out, but now we get stuck on another. Skipping this and the other huge page snapshot tests in the file. Signed-off-by: Jack Thomson <[email protected]>

tests(bk): Run the kernel build in our nightly PR

7e34e20

Run the kernel build as part of our nightly tests so we can monitor it's success. Signed-off-by: Jack Thomson <[email protected]>

ci: Update the script to support x86 on AL23

b410770

The output from the build in x86 is archived so updated the script to support installing either output type from the build Signed-off-by: Jack Thomson <[email protected]>

fix: test_hiding_kernel.py

dcf08bb

Add an 'apt update' before `apt install`. Otherwise, we might hold an old view of the package versions and installation might fail. Signed-off-by: Babis Chalios <[email protected]>

roypat force-pushed the secret-freedom-no-swiotlb branch 2 times, most recently from 0a1f922 to 01dfe08 Compare April 25, 2025 14:18

roypat force-pushed the secret-freedom-no-swiotlb branch 2 times, most recently from 2121123 to eaf63ee Compare April 25, 2025 14:59

roypat added 6 commits April 25, 2025 16:31

ci: dont fail downloading artifacts if no firecracker binaries exist

6da974c

If the CI artifacts dont contain old firecracker releases, still succeed at setting them up after downloading them. Signed-off-by: Patrick Roy <[email protected]>

add Vm::create_guest_memfd

24610fc

Add a utility function for creating a guest_memfd and wrapping it into a `File` object. Signed-off-by: Patrick Roy <[email protected]>

roypat force-pushed the secret-freedom-no-swiotlb branch 2 times, most recently from bdccfff to 40217b8 Compare April 25, 2025 15:57

roypat added 2 commits April 25, 2025 16:59

use bounce buffers for loading kernel if secret freedom is enabled

841ac04

If secret freedom is enabled, the guest kernel and potential initrd needs to be loaded via bounce buffer, as we cannot directly do `read` syscalls that target guest memory. Signed-off-by: Patrick Roy <[email protected]>

roypat and others added 14 commits April 25, 2025 16:59

use userspace bounce buffers if secret freedom is enabled

c9a9cfb

Needed because we cannot do I/O straight into secret hidden memory - the host kernel cannot access it. Signed-off-by: Patrick Roy <[email protected]>

switch to using kvm_userspace_region2

3c68e63

Fall back to kvm_user_memory_region in case the 2 version of the struct isnt supported. Signed-off-by: Patrick Roy <[email protected]>

tmp: call mmap outselves

0f49046

vm-memory has faulty validation logic that prevents us from mmap-ing guest_memfds, so just bypass that by calling mmap ourselves for the time being. See also rust-vmm/vm-memory#320 Signed-off-by: Patrick Roy <[email protected]>

fix: Stop the scan for vmlinux failing

8c6ccd0

Previously this would fail on x86 as we set -e. By setting the || true this means the script will continue. The grubby step next will fail if it failed to find the image. Signed-off-by: Jack Thomson <[email protected]>

chore(hiding_ci): skip non-patch files when applying

5df1386

This is to allow to keep the licence and readme files in the patches directory. Signed-off-by: Nikita Kalyazin <[email protected]>

doc(hiding_ci): add readme and GPL-2.0 text for Linux patches

b968ca5

Explicitly mention that Linux patches are distributed under the GPL-2.0 licence. Signed-off-by: Nikita Kalyazin <[email protected]>

chore(hiding_ci): rename patches dir to linux_patches

51593f3

This is to keep Linux patches separate in case we need to store some other patches at some point. Signed-off-by: Nikita Kalyazin <[email protected]>

chore(hiding_ci): fix up guest_memfd UFFD patches

deb856c

Also strip doc and test patches committed by mistake last time. Signed-off-by: Nikita Kalyazin <[email protected]>

roypat force-pushed the secret-freedom-no-swiotlb branch from 40217b8 to 6798bcc Compare April 25, 2025 16:01

roypat added 2 commits April 25, 2025 17:15

test: add functional tests for booting secret free VMs

92a2b78

Add a test that we can boot VMs and initrds with secret freedom enabled. Signed-off-by: Patrick Roy <[email protected]>

roypat force-pushed the secret-freedom-no-swiotlb branch from 6798bcc to 92a2b78 Compare April 25, 2025 16:15

test: disable memory monitor in boottime tests

dc53e8d

Since we load the kernel using bounce buffers now, it will give us false-positives. Signed-off-by: Patrick Roy <[email protected]>

roypat closed this Apr 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[[do not merge]] PR to check CI of rebase feature/secret-hiding branch #5171

[[do not merge]] PR to check CI of rebase feature/secret-hiding branch #5171

Uh oh!

roypat commented Apr 25, 2025

Uh oh!

codecov bot commented Apr 25, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[[do not merge]] PR to check CI of rebase feature/secret-hiding branch #5171

[[do not merge]] PR to check CI of rebase feature/secret-hiding branch #5171

Uh oh!

Conversation

roypat commented Apr 25, 2025

Uh oh!

codecov bot commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Apr 25, 2025 •

edited

Loading