tee/snp: Update SEV-SNP APIs, use KVM guest_memfd #296

tylerfanelli · 2025-04-02T05:09:40Z

This series updates the SEV-SNP APIs (sev library dependency to 6.0.0) and adds KVM guest_memfd support for SEV-SNP guests. This is required for running SEV-SNP encrypted guests on upstream kernels.

Tested on linux 6.14.0-0.rc3

cc @slp @jakecorrenti @MatiasVara

src/libkrun/src/lib.rs

src/vmm/src/linux/vstate.rs

jakecorrenti · 2025-04-03T12:56:39Z

Re-ran the Integration Tests but LGTM otherwise

tylerfanelli · 2025-04-03T19:14:02Z

@jakecorrenti @slp @MatiasVara Can you (re-)review? I'm not sure why the integration test (starting a VM with 1 vCPU and 1GiB of memory) fails. I'm able to boot a (non-confidential) VM with as little as 64MiB of memory.

MatiasVara · 2025-04-03T20:03:10Z

@jakecorrenti @slp @MatiasVara Can you (re-)review? I'm not sure why the integration test (starting a VM with 1 vCPU and 1GiB of memory) fails. I'm able to boot a (non-confidential) VM with as little as 64MiB of memory.

It is in my TODO list. I just had no time yet :'(

MatiasVara · 2025-04-04T16:24:32Z

I'm trying to understand why the rebase wasn’t done on my branch, which already included the guestmemfd (see 15ac9fc) and KVM_EXIT_MEMORY_FAUL (see 5c3f283) changes since last year—correct me if I’m wrong. To speed things up, I could create a new PR with just those two changes, so you would only need to rebase on top of that. I’d like to avoid any duplication of effort.

tylerfanelli · 2025-04-04T16:39:29Z

@MatiasVara I could do that if you prefer. The reason is because I needed to handle KVM_EXIT_HYPERCALL first, as on SEV-SNP that is what is encountered first. Therefore, I thought it was just be worthwhile to add KVM_EXIT_MEMORY_FAULT later.

I also did things a bit differently with not requiring guest_memfd with non-TEE workloads and managing of guest_memfds, associating with a RangeMap rather than a standard Vec.

EDIT: I am going to rebase on top of your commits and re-submit. Marking this as a draft until that is done.

tylerfanelli · 2025-04-06T17:40:56Z

@MatiasVara PTAL. I've rebased on your branch.

tylerfanelli · 2025-04-07T19:38:33Z

@mtjhrc Not sure why the integration test is failing, I'm able to run it locally just fine:

$ make test

.. snip ..

[configure-vm-1cpu-256MiB]: OK
[configure-vm-2cpu-1GiB]: OK
[vsock-guest-connect]: OK
[tsi-tcp-guest-connect]: OK
[tsi-tcp-guest-listen]: OK

OK (PASSED 5/5)

Currently investigating.

In the legacy AMD SEV implementation, the sev_secure_virt_attest function performed pre-boot attestation for a VM. This implementation was removed, and SEV-SNP uses post-boot attestation. As such, the SEV-SNP implementation only measures each region for guests, and does not attest anything, making the function name a bit misleading. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

kvm_bindings does not yet expose the KVM_X86_SNP_VM value. Hard code the value until it is available in kvm_bindings. Taken from linux/arch/x86/include/uapi/asm/kvm.h Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

Updated SEV-SNP support in the Linux kernel requires an update of the sev library version along with some modifications to the APIs provided by the library. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

When tee feature is enabled, use create_guest_memfd() and set_user_memory_region2(). The created regions are marked as private by using set_memory_attributes() thus imitating QEMU behavior. Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>

guest_memfd is required by SEV-SNP guests, and will also be used for other TEE architectures such as TDX and CCA. Probe if guest_memfd will be used on the system (non-TEE workloads do not require guest_memfd), and create/map them if so. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

SEV-SNP guests use KVM_EXIT_HYPERCALL to signal to the hypervisor that they would like some memory shared or private. Enable the KVM capability to allow the guests to use KVM_EXIT_HYPERCALL. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

tylerfanelli · 2025-04-17T14:29:27Z

@slp @MatiasVara @jakecorrenti Can you re-review? it seems the integration tests failure will need to be handled in a separate PR as its failing on non-TEE tests as well.

jakecorrenti

Commits 6833eb7 and 6be8e02 could be squashed, as well as the last two commits.

Also... Are we positive that nothing in this PR is causing the CI failure? I've posted 2 PRs myself after this one and haven't run into the issue with Integration tests. I only ask because I'd like to avoid failing CI on main if possible

src/vmm/src/linux/vstate.rs

src/libkrun/src/lib.rs

src/vmm/src/linux/vstate.rs

src/libkrun/src/lib.rs

tylerfanelli · 2025-04-18T03:22:20Z

Commits 6833eb7 and 6be8e02 could be squashed, as well as the last two commits.

I don't believe they should, as they are from two different authors.

Also... Are we positive that nothing in this PR is causing the CI failure? I've posted 2 PRs myself after this one and haven't run into the issue with Integration tests. I only ask because I'd like to avoid failing CI on main if possible

The integration test fails on non-TEE tests. Perhaps its the thread sleep in KVM_RUN. Will configure that to only be present with the tee feature and test.

jakecorrenti · 2025-04-18T12:52:09Z

Commits 6833eb7 and 6be8e02 could be squashed, as well as the last two commits.

I don't believe they should, as they are from two different authors.

You can add Co-authored by: <signature> in the git commit to show you and someone else should get credit for the squashed commit. IMO it would result in a cleaner git history, but I'm not going to block you on that.

Also... Are we positive that nothing in this PR is causing the CI failure? I've posted 2 PRs myself after this one and haven't run into the issue with Integration tests. I only ask because I'd like to avoid failing CI on main if possible

The integration test fails on non-TEE tests. Perhaps its the thread sleep in KVM_RUN. Will configure that to only be present with the tee feature and test.

MatiasVara · 2025-04-18T18:12:16Z

@slp @MatiasVara @jakecorrenti Can you re-review? it seems the integration tests failure will need to be handled in a separate PR as its failing on non-TEE tests as well.

Apologies, I did not have time to see this. I will try next week.

src/vmm/src/linux/vstate.rs

SEV-SNP guests use KVM_EXIT_HYPERCALL exits to signal to the hypervisor it would like some pages set to private or shared. Implements a handler that manages guest memory and can set regions to private or shared. vCPUs can send "memory properties" messages to the handler indicating: - Guest GPA - Size of memory region - Whether the region should be set to private or shared The handler will read these messages and configure the memory regions accordingly. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

The KVM_EXIT_MEMORY_FAULT vmexit is triggered when guest wants to switch a region of memory from private to shared and viceversa. To support this when tee is enabled, add an extra thread named sender_io that gets the parameters from the vcpu thread and triggers the set_memory_properties(). The vcpu fd is owned only by this thread. Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

jakecorrenti · 2025-04-23T21:04:53Z

Ideally I would like a review from @MatiasVara as well since this impacts him, but if that can't happen that's ok too

MatiasVara · 2025-04-25T19:15:39Z

Commits 6833eb7 and 6be8e02 could be squashed, as well as the last two commits.

I don't believe they should, as they are from two different authors.

You can add Co-authored by: <signature> in the git commit to show you and someone else should get credit for the squashed commit. IMO it would result in a cleaner git history, but I'm not going to block you on that.

Also... Are we positive that nothing in this PR is causing the CI failure? I've posted 2 PRs myself after this one and haven't run into the issue with Integration tests. I only ask because I'd like to avoid failing CI on main if possible

The integration test fails on non-TEE tests. Perhaps its the thread sleep in KVM_RUN. Will configure that to only be present with the tee feature and test.

I agree with Jake that those commits could be squashed, including 2f25d7d527ffda24a44291977f8b9da4fd68b035 and 81f46532f13fb44bbcac7c5a5606c2e676b5e0cc. I understand this is a result of the rebasing. My comment here was meant to point out that these changes could have been addressed directly during the review of #211. It's something we should keep in mind for next time. The PR LGTM.

tylerfanelli · 2025-04-28T02:07:47Z

Commits 6833eb7 and 6be8e02 could be squashed, as well as the last two commits.

I don't believe they should, as they are from two different authors.

You can add Co-authored by: <signature> in the git commit to show you and someone else should get credit for the squashed commit. IMO it would result in a cleaner git history, but I'm not going to block you on that.

Also... Are we positive that nothing in this PR is causing the CI failure? I've posted 2 PRs myself after this one and haven't run into the issue with Integration tests. I only ask because I'd like to avoid failing CI on main if possible

The integration test fails on non-TEE tests. Perhaps its the thread sleep in KVM_RUN. Will configure that to only be present with the tee feature and test.

I agree with Jake that those commits could be squashed, including 2f25d7d527ffda24a44291977f8b9da4fd68b035 and 81f46532f13fb44bbcac7c5a5606c2e676b5e0cc. I understand this is a result of the rebasing.

I've generally avoided squashing commits that were from two separate authors. Does the Co-authored-by: line preserve authorship in git, or does authorship switch to the author of the squashed commit?

My comment here was meant to point out that these changes could have been addressed directly during the review of #211. It's something we should keep in mind for next time. The PR LGTM.

I see. It was a bit of a conundrum because those changes weren't able to be merged yet, so I thought I'd address the changes in my PR. Under typical circumstances I completely agree.

Bit of a strange circumstance here, but I will definitely keep your comments in mind in the future.

tylerfanelli requested review from jakecorrenti, MatiasVara, mtjhrc and slp as code owners April 2, 2025 05:09

tylerfanelli force-pushed the snp-update branch from 2907fca to 9aee52f Compare April 2, 2025 05:31

tylerfanelli changed the title ~~tee/snp:Update SEV-SNP APIs, use KVM guest_memfd~~ tee/snp: Update SEV-SNP APIs, use KVM guest_memfd Apr 2, 2025

jakecorrenti reviewed Apr 2, 2025

View reviewed changes

tylerfanelli commented Apr 2, 2025

View reviewed changes

src/vmm/src/linux/vstate.rs Outdated Show resolved Hide resolved

tylerfanelli force-pushed the snp-update branch from 9aee52f to a8f39e1 Compare April 3, 2025 02:39

tylerfanelli force-pushed the snp-update branch 2 times, most recently from 4a3fdde to 6861441 Compare April 3, 2025 17:43

tylerfanelli force-pushed the snp-update branch from 6861441 to 818122d Compare April 3, 2025 20:41

tylerfanelli marked this pull request as draft April 4, 2025 16:56

tylerfanelli force-pushed the snp-update branch 3 times, most recently from 8b56864 to 2baf291 Compare April 6, 2025 03:26

tylerfanelli marked this pull request as ready for review April 6, 2025 03:28

tylerfanelli force-pushed the snp-update branch from 2baf291 to 262b34c Compare April 7, 2025 16:03

tylerfanelli force-pushed the snp-update branch from 262b34c to d368877 Compare April 8, 2025 14:43

mtjhrc mentioned this pull request Apr 16, 2025

Integration tests failures #314

Open

tylerfanelli force-pushed the snp-update branch from d368877 to 9d96dd5 Compare April 16, 2025 17:27

tylerfanelli added 2 commits April 16, 2025 13:31

amd-sev: Create VM with KVM_X86_SNP_VM type

9d16f2a

kvm_bindings does not yet expose the KVM_X86_SNP_VM value. Hard code the value until it is available in kvm_bindings. Taken from linux/arch/x86/include/uapi/asm/kvm.h Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

tylerfanelli and others added 4 commits April 16, 2025 13:31

amd-sev: Update sev library to 6.0.0

aeee48b

Updated SEV-SNP support in the Linux kernel requires an update of the sev library version along with some modifications to the APIs provided by the library. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

tylerfanelli force-pushed the snp-update branch 2 times, most recently from 5d29996 to 1bc3f71 Compare April 17, 2025 14:27

jakecorrenti reviewed Apr 17, 2025

View reviewed changes

src/vmm/src/linux/vstate.rs Outdated Show resolved Hide resolved

src/libkrun/src/lib.rs Show resolved Hide resolved

src/vmm/src/linux/vstate.rs Show resolved Hide resolved

src/vmm/src/linux/vstate.rs Outdated Show resolved Hide resolved

src/libkrun/src/lib.rs Show resolved Hide resolved

tylerfanelli force-pushed the snp-update branch 2 times, most recently from e425c47 to ecf414d Compare April 21, 2025 02:37

slp reviewed Apr 21, 2025

View reviewed changes

src/vmm/src/linux/vstate.rs Outdated Show resolved Hide resolved

tylerfanelli force-pushed the snp-update branch from ecf414d to cc727a1 Compare April 21, 2025 15:58

tylerfanelli and others added 3 commits April 21, 2025 21:28

vmm: Coalesce around pm_sender

81f4653

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>

tylerfanelli force-pushed the snp-update branch from cc727a1 to 81f4653 Compare April 22, 2025 02:05

jakecorrenti approved these changes Apr 23, 2025

View reviewed changes

MatiasVara approved these changes Apr 25, 2025

View reviewed changes

tylerfanelli merged commit d8a1bc9 into containers:main Apr 28, 2025
6 checks passed

tylerfanelli deleted the snp-update branch April 28, 2025 02:00

tee/snp: Update SEV-SNP APIs, use KVM guest_memfd #296

tee/snp: Update SEV-SNP APIs, use KVM guest_memfd #296

Uh oh!

Conversation

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakecorrenti commented Apr 3, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

MatiasVara commented Apr 4, 2025

Uh oh!

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

Uh oh!

Uh oh!

tylerfanelli commented Apr 6, 2025

Uh oh!

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

Uh oh!

Uh oh!

tylerfanelli commented Apr 17, 2025

Uh oh!

jakecorrenti left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

MatiasVara commented Apr 18, 2025

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

MatiasVara commented Apr 25, 2025

Uh oh!

Uh oh!

tylerfanelli commented Apr 28, 2025

Uh oh!

Uh oh!

tylerfanelli commented Apr 2, 2025 •

edited

Loading

tylerfanelli commented Apr 3, 2025 •

edited

Loading

MatiasVara commented Apr 3, 2025 •

edited

Loading

tylerfanelli commented Apr 4, 2025 •

edited

Loading

tylerfanelli commented Apr 7, 2025 •

edited

Loading

jakecorrenti left a comment •

edited

Loading

tylerfanelli commented Apr 18, 2025 •

edited

Loading

jakecorrenti commented Apr 18, 2025 •

edited

Loading

jakecorrenti commented Apr 23, 2025 •

edited

Loading