Re: [PATCH RFC v3 00/49] Add AMD Secure Nested Paging (SEV-SNP) support
> On 20 Mar 2024, at 14:08, Michael Roth wrote: > > These patches implement SEV-SNP base support along with CPUID enforcement > support for QEMU, and are also available at: > > https://github.com/amdese/qemu/commits/snp-v3-rfc > > they are based on top of the following patchset from Paolo: > > "[PATCH 0/7] target/i386: VM type infrastructure and KVM_SEV_INIT2 support" > https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg04663.html Can you please also CC me on future revisions of this patchset? Thanks! > > > Patch Layout > > > 01-05: Various changes needed to handle new header files in kvm-next tree > and some hacks to get a functional header sync in place for building > this series. > 06-18: These are patches directly plucked from Xiaoyao's TDX v5 patchset[1] > that implement common dependencies between SNP/TDX like base > guest_memfd, KVM_EXIT_MEMORY_FAULT handling (with a small FIXUP), and > mechanisms to disable SMM. We would've also needed some of the basic > infrastructure for handling specifying VM types for KVM_CREATE, but > much of that is now part of the sevinit2 series this patchset is based > on. Ideally all these patches, once stable, could be maintained in a > common tree so that future SNP/TDX patchsets can be more easily > iterated on/reviewed. > 19-20: Patches introduced by this series that are possible candidate for a > common tree. > shared/private pages when things like VFIO are in use. > 21-32: Introduction of sev-snp-guest object and various configuration > requirements for SNP. > 33-36: Handling for various KVM_EXIT_VMGEXIT events that are handled in > userspace. > 37-49: Support for creating a cryptographic "launch" context and populating > various OVMF metadata pages, BIOS regions, and vCPU/VMSA pages with > the initial encrypted/measured/validated launch data prior to > launching the SNP guest. > > > Testing > --- > > This series has been tested against the following host kernel tree, which > is a snapshot of the latest WIP SNP hypervisor tree at the time of this > posting. It will likely not be kept up to date afterward, so please keep an > eye upstream or official AMDESE github if you are looking for the latest > some time after this posting: > > https://github.com/mdroth/linux/commits/snp-host-v12-wip40/ > > A patched OVMF is also needed due to upstream KVM no longer supporting MMIO > ranges that are mapped as private. It is recommended you build the AmdSevX64 > variant as it provides the kernel-hashing support present in this series: > > https://github.com/mdroth/edk2/commits/apic-mmio-fix1c/ > > A basic command-line invocation for SNP would be: > > qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2 > -machine q35,confidential-guest-support=sev0,memory-backend=ram1 > -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false > -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth= > -bios /home/mroth/ovmf/OVMF_CODE-upstream-20240228-apicfix-1c-AmdSevX64.fd > > With kernel-hashing and certificate data supplied: > > qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2 > -machine q35,confidential-guest-support=sev0,memory-backend=ram1 > -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false > -object > sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth=,certs-path=/home/mroth/cert.blob,kernel-hashes=on > -bios /home/mroth/ovmf/OVMF_CODE-upstream-20240228-apicfix-1c-AmdSevX64.fd > -kernel /boot/vmlinuz-6.8.0-snp-host-v12-wip40+ > -initrd /boot/initrd.img-6.8.0-snp-host-v12-wip40+ > -append "root=UUID=d72a6d1c-06cf-4b79-af43-f1bac4f620f9 ro > console=ttyS0,115200n8" > > Any comments/feedback would be very much appreciated. > > [1] https://github.com/amdese/linux >https://github.com/amdese/amdsev/tree/snp-latest > > Changes since rfc2: > > - reworked on top of guest_memfd support > - added handling for various KVM_EXIT_VMGEXIT events > - various changes/considerations for PCI passthrough support > - general bugfixes/hardening/cleanups > - qapi cmdline doc fixes/rework (Dov, Markus) > - switch to qbase64_decode, more error-checking for cmdline opts (Dov) > - unset id_block_en for 0 input (Dov) > - use error_setg in snp init (Dov) > - report more info in trace_kvm_sev_init (Dov) > - rework bounds-checking for kvm_cpuid_info, rework existing checks for > readability, add additional checks (Dov) > - fixups for validated_ranges handling (Dov) > - rename 'policy' field to 'snp-policy' in query-sev when sev-type is SNP > > Changes since rfc1: > > - rebased onto latest master > - drop SNP config file in favor of a new 'sev-snp-guest' object where all > SNP-related params are passed as strings/integers via command-line > - report specific error if BIOS reports invalid address/len for > reserved/pre-validated regions (Connor) > - use Range helpers
Re: [PATCH RFC v3 00/49] Add AMD Secure Nested Paging (SEV-SNP) support
On Wed, Mar 20, 2024 at 03:38:56AM -0500, Michael Roth wrote: > > Testing > --- > > This series has been tested against the following host kernel tree, which > is a snapshot of the latest WIP SNP hypervisor tree at the time of this > posting. It will likely not be kept up to date afterward, so please keep an > eye upstream or official AMDESE github if you are looking for the latest > some time after this posting: > > https://github.com/mdroth/linux/commits/snp-host-v12-wip40/ I just noticed I had a necessary local change that wasn't included in the initial push of this branch. I've updated the branch now, but just wanted to post a heads-up in case anyone was having issues. -Mike
Re: [PATCH RFC v3 00/49] Add AMD Secure Nested Paging (SEV-SNP) support
On 3/21/2024 1:08 AM, Paolo Bonzini wrote: On Wed, Mar 20, 2024 at 10:59 AM Paolo Bonzini wrote: I will now focus on reviewing patches 6-20. This way we can prepare a common tree for SEV_INIT2/SNP/TDX, for both vendors to build upon. Ok, the attachment is the delta that I have. The only major change is requiring discard (thus effectively blocking VFIO support for SEV-SNP/TDX, at least for now). I will push it shortly to the same sevinit2 branch, and will post the patches sometime soon. Xiaoyao, you can use that branch too (it's on https://gitlab.com/bonzini/qemu) as the basis for your TDX work. Sure, it's really a good news for us. BTW, there are some minor comments on guest_memfd patches of my v5 post[*]. Could you please resolve them it your branch? [*] https://lore.kernel.org/qemu-devel/20240229063726.610065-1-xiaoyao...@intel.com/ Paolo
Re: [PATCH RFC v3 00/49] Add AMD Secure Nested Paging (SEV-SNP) support
On Wed, Mar 20, 2024 at 10:59 AM Paolo Bonzini wrote:
> I will now focus on reviewing patches 6-20. This way we can prepare a
> common tree for SEV_INIT2/SNP/TDX, for both vendors to build upon.
Ok, the attachment is the delta that I have. The only major change is
requiring discard (thus effectively blocking VFIO support for
SEV-SNP/TDX, at least for now).
I will push it shortly to the same sevinit2 branch, and will post the
patches sometime soon.
Xiaoyao, you can use that branch too (it's on
https://gitlab.com/bonzini/qemu) as the basis for your TDX work.
Paolo
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index bf0ae0c8adb..428468950d9 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -285,19 +285,8 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
{
KVMState *s = kvm_state;
struct kvm_userspace_memory_region2 mem;
-static int cap_user_memory2 = -1;
int ret;
-if (cap_user_memory2 == -1) {
-cap_user_memory2 = kvm_check_extension(s, KVM_CAP_USER_MEMORY2);
-}
-
-if (!cap_user_memory2 && slot->guest_memfd >= 0) {
-error_report("%s, KVM doesn't support KVM_CAP_USER_MEMORY2,"
- " which is required by guest memfd!", __func__);
-exit(1);
-}
-
mem.slot = slot->slot | (kml->as_id << 16);
mem.guest_phys_addr = slot->start_addr;
mem.userspace_addr = (unsigned long)slot->ram;
@@ -310,7 +299,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
* value. This is needed based on KVM commit 75d61fbc. */
mem.memory_size = 0;
-if (cap_user_memory2) {
+if (kvm_guest_memfd_supported) {
ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION2, );
} else {
ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, );
@@ -320,7 +309,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
}
}
mem.memory_size = slot->memory_size;
-if (cap_user_memory2) {
+if (kvm_guest_memfd_supported) {
ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION2, );
} else {
ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, );
@@ -332,7 +321,7 @@ err:
mem.userspace_addr, mem.guest_memfd,
mem.guest_memfd_offset, ret);
if (ret < 0) {
-if (cap_user_memory2) {
+if (kvm_guest_memfd_supported) {
error_report("%s: KVM_SET_USER_MEMORY_REGION2 failed, slot=%d,"
" start=0x%" PRIx64 ", size=0x%" PRIx64 ","
" flags=0x%" PRIx32 ", guest_memfd=%" PRId32 ","
@@ -502,6 +491,7 @@ static int kvm_mem_flags(MemoryRegion *mr)
flags |= KVM_MEM_READONLY;
}
if (memory_region_has_guest_memfd(mr)) {
+assert(kvm_guest_memfd_supported);
flags |= KVM_MEM_GUEST_MEMFD;
}
return flags;
@@ -1310,18 +1300,7 @@ static int kvm_set_memory_attributes(hwaddr start, hwaddr size, uint64_t attr)
struct kvm_memory_attributes attrs;
int r;
-if (kvm_supported_memory_attributes == 0) {
-error_report("No memory attribute supported by KVM\n");
-return -EINVAL;
-}
-
-if ((attr & kvm_supported_memory_attributes) != attr) {
-error_report("memory attribute 0x%lx not supported by KVM,"
- " supported bits are 0x%lx\n",
- attr, kvm_supported_memory_attributes);
-return -EINVAL;
-}
-
+assert((attr & kvm_supported_memory_attributes) == attr);
attrs.attributes = attr;
attrs.address = start;
attrs.size = size;
@@ -2488,11 +2467,14 @@ static int kvm_init(MachineState *ms)
}
s->as = g_new0(struct KVMAs, s->nr_as);
-kvm_guest_memfd_supported = kvm_check_extension(s, KVM_CAP_GUEST_MEMFD);
-
ret = kvm_check_extension(s, KVM_CAP_MEMORY_ATTRIBUTES);
kvm_supported_memory_attributes = ret > 0 ? ret : 0;
+kvm_guest_memfd_supported =
+kvm_check_extension(s, KVM_CAP_GUEST_MEMFD) &&
+kvm_check_extension(s, KVM_CAP_USER_MEMORY2) &&
+(kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE);
+
if (object_property_find(OBJECT(current_machine), "kvm-type")) {
g_autofree char *kvm_type = object_property_get_str(OBJECT(current_machine),
"kvm-type",
@@ -2962,14 +2944,10 @@ int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
*/
return 0;
} else {
-ret = ram_block_discard_is_disabled()
- ? ram_block_discard_range(rb, offset, size)
- : 0;
+ret = ram_block_discard_range(rb, offset, size);
}
} else {
-ret = ram_block_discard_is_disabled()
- ? ram_block_discard_guest_memfd_range(rb, offset,
Re: [PATCH RFC v3 00/49] Add AMD Secure Nested Paging (SEV-SNP) support
On 3/20/24 09:38, Michael Roth wrote: These patches implement SEV-SNP base support along with CPUID enforcement support for QEMU, and are also available at: https://github.com/amdese/qemu/commits/snp-v3-rfc they are based on top of the following patchset from Paolo: "[PATCH 0/7] target/i386: VM type infrastructure and KVM_SEV_INIT2 support" https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg04663.html Patch Layout 01-05: Various changes needed to handle new header files in kvm-next tree and some hacks to get a functional header sync in place for building this series. 06-18: These are patches directly plucked from Xiaoyao's TDX v5 patchset[1] that implement common dependencies between SNP/TDX like base guest_memfd, KVM_EXIT_MEMORY_FAULT handling (with a small FIXUP), and mechanisms to disable SMM. We would've also needed some of the basic infrastructure for handling specifying VM types for KVM_CREATE, but much of that is now part of the sevinit2 series this patchset is based on. Ideally all these patches, once stable, could be maintained in a common tree so that future SNP/TDX patchsets can be more easily iterated on/reviewed. 19-20: Patches introduced by this series that are possible candidate for a common tree. shared/private pages when things like VFIO are in use. 21-32: Introduction of sev-snp-guest object and various configuration requirements for SNP. 33-36: Handling for various KVM_EXIT_VMGEXIT events that are handled in userspace. 37-49: Support for creating a cryptographic "launch" context and populating various OVMF metadata pages, BIOS regions, and vCPU/VMSA pages with the initial encrypted/measured/validated launch data prior to launching the SNP guest. I reviewed the non-SEV bits of patches 21-46 and it looks nicely self-contained. That's pretty much expected but still good news. I didn't look closely at the SEV-SNP code for obvious reasons (it's only been one hour :)), except for the object-oriented aesthetics which I have remarked upon. However, they seem to be in good shape. I will now focus on reviewing patches 6-20. This way we can prepare a common tree for SEV_INIT2/SNP/TDX, for both vendors to build upon. Thanks for posting this, and thanks to the Intel people too for the previous work on the guest_memfd parts! Paolo
[PATCH RFC v3 00/49] Add AMD Secure Nested Paging (SEV-SNP) support
These patches implement SEV-SNP base support along with CPUID enforcement support for QEMU, and are also available at: https://github.com/amdese/qemu/commits/snp-v3-rfc they are based on top of the following patchset from Paolo: "[PATCH 0/7] target/i386: VM type infrastructure and KVM_SEV_INIT2 support" https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg04663.html Patch Layout 01-05: Various changes needed to handle new header files in kvm-next tree and some hacks to get a functional header sync in place for building this series. 06-18: These are patches directly plucked from Xiaoyao's TDX v5 patchset[1] that implement common dependencies between SNP/TDX like base guest_memfd, KVM_EXIT_MEMORY_FAULT handling (with a small FIXUP), and mechanisms to disable SMM. We would've also needed some of the basic infrastructure for handling specifying VM types for KVM_CREATE, but much of that is now part of the sevinit2 series this patchset is based on. Ideally all these patches, once stable, could be maintained in a common tree so that future SNP/TDX patchsets can be more easily iterated on/reviewed. 19-20: Patches introduced by this series that are possible candidate for a common tree. shared/private pages when things like VFIO are in use. 21-32: Introduction of sev-snp-guest object and various configuration requirements for SNP. 33-36: Handling for various KVM_EXIT_VMGEXIT events that are handled in userspace. 37-49: Support for creating a cryptographic "launch" context and populating various OVMF metadata pages, BIOS regions, and vCPU/VMSA pages with the initial encrypted/measured/validated launch data prior to launching the SNP guest. Testing --- This series has been tested against the following host kernel tree, which is a snapshot of the latest WIP SNP hypervisor tree at the time of this posting. It will likely not be kept up to date afterward, so please keep an eye upstream or official AMDESE github if you are looking for the latest some time after this posting: https://github.com/mdroth/linux/commits/snp-host-v12-wip40/ A patched OVMF is also needed due to upstream KVM no longer supporting MMIO ranges that are mapped as private. It is recommended you build the AmdSevX64 variant as it provides the kernel-hashing support present in this series: https://github.com/mdroth/edk2/commits/apic-mmio-fix1c/ A basic command-line invocation for SNP would be: qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2 -machine q35,confidential-guest-support=sev0,memory-backend=ram1 -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth= -bios /home/mroth/ovmf/OVMF_CODE-upstream-20240228-apicfix-1c-AmdSevX64.fd With kernel-hashing and certificate data supplied: qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2 -machine q35,confidential-guest-support=sev0,memory-backend=ram1 -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth=,certs-path=/home/mroth/cert.blob,kernel-hashes=on -bios /home/mroth/ovmf/OVMF_CODE-upstream-20240228-apicfix-1c-AmdSevX64.fd -kernel /boot/vmlinuz-6.8.0-snp-host-v12-wip40+ -initrd /boot/initrd.img-6.8.0-snp-host-v12-wip40+ -append "root=UUID=d72a6d1c-06cf-4b79-af43-f1bac4f620f9 ro console=ttyS0,115200n8" Any comments/feedback would be very much appreciated. [1] https://github.com/amdese/linux https://github.com/amdese/amdsev/tree/snp-latest Changes since rfc2: - reworked on top of guest_memfd support - added handling for various KVM_EXIT_VMGEXIT events - various changes/considerations for PCI passthrough support - general bugfixes/hardening/cleanups - qapi cmdline doc fixes/rework (Dov, Markus) - switch to qbase64_decode, more error-checking for cmdline opts (Dov) - unset id_block_en for 0 input (Dov) - use error_setg in snp init (Dov) - report more info in trace_kvm_sev_init (Dov) - rework bounds-checking for kvm_cpuid_info, rework existing checks for readability, add additional checks (Dov) - fixups for validated_ranges handling (Dov) - rename 'policy' field to 'snp-policy' in query-sev when sev-type is SNP Changes since rfc1: - rebased onto latest master - drop SNP config file in favor of a new 'sev-snp-guest' object where all SNP-related params are passed as strings/integers via command-line - report specific error if BIOS reports invalid address/len for reserved/pre-validated regions (Connor) - use Range helpers for handling validated region overlaps (Dave) - simplify error handling in sev_snp_launch_start, and report the correct return code when handling LAUNCH_START failures (Dov) - add SEV-SNP bit to CPUID 0x801f when SNP enabled - updated query-sev to handle differences between SEV and SEV-SNP -
