public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP
@ 2025-02-24 12:37 Philipp Giersfeld
  2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 1/5] Update edk2 to edkstable202411 Philipp Giersfeld
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Philipp Giersfeld @ 2025-02-24 12:37 UTC (permalink / raw)
  To: pve-devel

This patch series adds support for AMD SEV-SNP. 
Where possible it mimics the existing support for AMD SEV(-ES). 

Running SEV-SNP VMs requires a more recent version of edk2
and OVMF firmware image. Contrary to other setups, SEV-SNP does not support loading the firmware via pflash. Instead, the firmware image is loaded  via the -bios option.

---

 changes since v2: https://lists.proxmox.com/pipermail/pve-devel/2025-February/068418.html
 * Only set no-key-sharing if SNP is not used
 * Fix formatting



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH edk2-firmware v3 1/5] Update edk2 to edkstable202411
  2025-02-24 12:37 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP Philipp Giersfeld
@ 2025-02-24 12:37 ` Philipp Giersfeld
  2025-03-05 16:51   ` [pve-devel] applied: " Thomas Lamprecht
  2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Philipp Giersfeld @ 2025-02-24 12:37 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
Tested-by: Markus Frank <m.frank@proxmox.com>
---

 no changes since last version

 debian/binary-check.remove | 4 ++--
 debian/rules               | 6 +++---
 edk2                       | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/debian/binary-check.remove b/debian/binary-check.remove
index 7651301..e5f352a 100644
--- a/debian/binary-check.remove
+++ b/debian/binary-check.remove
@@ -1,5 +1,5 @@
-ArmPkg/Library/GccLto/liblto-aarch64.a
-ArmPkg/Library/GccLto/liblto-arm.a
+BaseTools/Bin/GccLto/liblto-aarch64.a
+BaseTools/Bin/GccLto/liblto-arm.a
 BaseTools/Source/Python/Eot/EfiCompressor.pyd
 BaseTools/Source/Python/Eot/LzmaCompressor.pyd
 IntelFsp2Pkg/FspSecCore/Vtf0/Bin/ResetVec.ia32.raw
diff --git a/debian/rules b/debian/rules
index 7f92c16..2e9365b 100755
--- a/debian/rules
+++ b/debian/rules
@@ -180,10 +180,10 @@ debian/oem-string-%: debian/PkKek-1-%.pem
 		--no-default \
 		-C `< debian/oem-string-snakeoil` -o $@
 
-ArmPkg/Library/GccLto/liblto-aarch64.a:	ArmPkg/Library/GccLto/liblto-aarch64.s
+BaseTools/Bin/GccLto/liblto-aarch64.a:	BaseTools/Bin/GccLto/liblto-aarch64.s
 	$($(EDK2_TOOLCHAIN)_AARCH64_PREFIX)gcc -c -fpic $< -o $@
 
-ArmPkg/Library/GccLto/liblto-arm.a: ArmPkg/Library/GccLto/liblto-arm.s
+BaseTools/Bin/GccLto/liblto-arm.a: BaseTools/Bin/GccLto/liblto-arm.s
 	$($(EDK2_TOOLCHAIN)_ARM_PREFIX)gcc -c -fpic $< -o $@
 
 build-qemu-efi: debian/setup-build-stamp
@@ -202,7 +202,7 @@ build-qemu-efi: debian/setup-build-stamp
 	truncate -s 64M $(QEMU_EFI_BUILD_DIR)/FV/$(FW_NAME)_VARS.fd
 
 build-qemu-efi-aarch64: $(AAVMF_BINARIES) $(AAVMF_IMAGES) $(AAVMF_PREENROLLED_VARS)
-$(AAVMF_BINARIES) $(AAVMF_IMAGES): ArmPkg/Library/GccLto/liblto-aarch64.a
+$(AAVMF_BINARIES) $(AAVMF_IMAGES): BaseTools/Bin/GccLto/liblto-aarch64.a
 	$(MAKE) -f debian/rules build-qemu-efi EDK2_ARCH_DIR=AArch64 EDK2_HOST_ARCH=AARCH64 FW_NAME=AAVMF
 
 build-qemu-efi-riscv64:  $(RISCV64_IMAGES)
diff --git a/edk2 b/edk2
index 819cfc6..0f3867f 160000
--- a/edk2
+++ b/edk2
@@ -1 +1 @@
-Subproject commit 819cfc6b42a68790a23509e4fcc58ceb70e1965e
+Subproject commit 0f3867fa6ef0553e26c42f7d71ff6bdb98429742
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP
  2025-02-24 12:37 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP Philipp Giersfeld
  2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 1/5] Update edk2 to edkstable202411 Philipp Giersfeld
@ 2025-02-24 12:37 ` Philipp Giersfeld
  2025-03-05 14:18   ` Fiona Ebner
  2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 3/5] Convert policy calculation Philipp Giersfeld
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Philipp Giersfeld @ 2025-02-24 12:37 UTC (permalink / raw)
  To: pve-devel

AMD SEV-SNP boots with a single volatile firmware image OVMF.fd via the
-bios option.

Currently, an SEV-enabled VM will not boot with an OVMF
firmware that was compiled with `SECURE_BOOT_ENABLE` [1].

Furthermore, during testing, SEV-enabled amchines did not boot with
`SMM_REQUIRE`.

Therefore, introduce a new target build-ovmf-cvm that builds OVMF
firmware suitable for AMD SEV.

[1] https://github.com/tianocore/edk2/pull/6285

Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
Tested-by: Markus Frank <m.frank@proxmox.com>
---

 no changes since last version

 debian/pve-edk2-firmware-ovmf.install |  3 +++
 debian/rules                          | 35 +++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/debian/pve-edk2-firmware-ovmf.install b/debian/pve-edk2-firmware-ovmf.install
index f4c0602..a51846e 100644
--- a/debian/pve-edk2-firmware-ovmf.install
+++ b/debian/pve-edk2-firmware-ovmf.install
@@ -1,5 +1,8 @@
 debian/ovmf-install/OVMF_CODE*.fd	/usr/share/pve-edk2-firmware
 debian/ovmf-install/OVMF_VARS*.fd	/usr/share/pve-edk2-firmware
+debian/ovmf-cvm-install/OVMF_CVM_CODE*.fd	/usr/share/pve-edk2-firmware
+debian/ovmf-cvm-install/OVMF_CVM_VARS*.fd	/usr/share/pve-edk2-firmware
+debian/ovmf-cvm-install/OVMF_CVM_4M.fd	/usr/share/pve-edk2-firmware
 debian/ovmf32-install/OVMF32_CODE*.fd		/usr/share/pve-edk2-firmware
 debian/ovmf32-install/OVMF32_VARS*.fd		/usr/share/pve-edk2-firmware
 debian/PkKek-1-snakeoil.*			/usr/share/pve-edk2-firmware
diff --git a/debian/rules b/debian/rules
index 2e9365b..3959500 100755
--- a/debian/rules
+++ b/debian/rules
@@ -28,22 +28,24 @@ PCD_FLAGS += --pcd PcdFirmwareReleaseDateString=L"$(PCD_RELEASE_DATE)\\0"
 COMMON_FLAGS  = -DNETWORK_HTTP_BOOT_ENABLE=TRUE
 COMMON_FLAGS += -DNETWORK_IP6_ENABLE=TRUE
 COMMON_FLAGS += -DNETWORK_TLS_ENABLE
-COMMON_FLAGS += -DSECURE_BOOT_ENABLE=TRUE
 COMMON_FLAGS += -DPVSCSI_ENABLE=TRUE
 COMMON_FLAGS += $(PCD_FLAGS)
 OVMF_COMMON_FLAGS  = $(COMMON_FLAGS)
 OVMF_COMMON_FLAGS += -DTPM2_ENABLE=TRUE
-OVMF_4M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_4MB
+OVMF_4M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_4MB -DSECURE_BOOT_ENABLE=TRUE
 OVMF_4M_SMM_FLAGS = $(OVMF_4M_FLAGS) -DSMM_REQUIRE=TRUE
-OVMF32_4M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_4MB
+OVMF32_4M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_4MB -DSECURE_BOOT_ENABLE=TRUE
 OVMF32_4M_SMM_FLAGS =  $(OVMF32_4M_FLAGS) -DSMM_REQUIRE=TRUE
+OVMF_CVM_4M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_4MB
 
 AAVMF_FLAGS  = $(COMMON_FLAGS)
+AAVMF_FLAGS += -DSECURE_BOOT_ENABLE=TRUE
 AAVMF_FLAGS += -DTPM2_ENABLE=TRUE
 AAVMF_FLAGS += -DTPM2_CONFIG_ENABLE=TRUE
 AAVMF_FLAGS += -DCAVIUM_ERRATUM_27456=TRUE
 
 RISCV64_FLAGS = $(COMMON_FLAGS)
+RISCV64_FLAGS += -DSECURE_BOOT_ENABLE=TRUE
 
 # Clear variables used internally by the edk2 build system
 undefine WORKSPACE
@@ -56,7 +58,7 @@ undefine CONF_PATH
 %:
 	dh $@
 
-override_dh_auto_build: build-qemu-efi-aarch64 build-ovmf build-ovmf32 build-qemu-efi-riscv64
+override_dh_auto_build: build-qemu-efi-aarch64 build-ovmf build-ovmf32 build-ovmf-cvm build-qemu-efi-riscv64
 
 debian/setup-build-stamp:
 	cp -a debian/Logo.bmp MdeModulePkg/Logo/Logo.bmp
@@ -79,6 +81,12 @@ OVMF32_SHELL = $(OVMF32_BUILD_DIR)/IA32/Shell.efi
 OVMF32_BINARIES = $(OVMF32_SHELL)
 OVMF32_IMAGES  := $(addprefix $(OVMF32_INSTALL_DIR)/,OVMF32_CODE_4M.secboot.fd OVMF32_VARS_4M.fd)
 
+OVMF_CVM_INSTALL_DIR = debian/ovmf-cvm-install
+OVMF_CVM_BUILD_DIR = Build/OvmfX64/$(BUILD_TYPE)_$(EDK2_TOOLCHAIN)
+OVMF_CVM_SHELL = $(OVMF_CVM_BUILD_DIR)/X64/Shell.efi
+OVMF_CVM_BINARIES = $(OVMF_CVM_SHELL)
+OVMF_CVM_IMAGES  := $(addprefix $(OVMF_CVM_INSTALL_DIR)/,OVMF_CVM_CODE_4M.fd OVMF_CVM_VARS_4M.fd)
+
 QEMU_EFI_BUILD_DIR = Build/ArmVirtQemu-$(EDK2_HOST_ARCH)/$(BUILD_TYPE)_$(EDK2_TOOLCHAIN)
 AAVMF_BUILD_DIR = Build/ArmVirtQemu-AARCH64/$(BUILD_TYPE)_$(EDK2_TOOLCHAIN)
 AAVMF_ENROLL    = $(AAVMF_BUILD_DIR)/AARCH64/EnrollDefaultKeys.efi
@@ -106,6 +114,23 @@ $(OVMF32_BINARIES) $(OVMF32_IMAGES): debian/setup-build-stamp
 	cp $(OVMF32_BUILD_DIR)/FV/OVMF_VARS.fd \
 		$(OVMF32_INSTALL_DIR)/OVMF32_VARS_4M.fd
 
+build-ovmf-cvm: $(OVMF_CVM_BINARIES) $(OVMF_CVM_IMAGES)
+$(OVMF_CVM_BINARIES) $(OVMF_CVM_IMAGES): debian/setup-build-stamp
+	rm -rf $(OVMF_CVM_INSTALL_DIR)
+	mkdir $(OVMF_CVM_INSTALL_DIR)
+	set -e; . ./edksetup.sh; \
+		build -a X64 \
+			-t $(EDK2_TOOLCHAIN) \
+			-p OvmfPkg/OvmfPkgX64.dsc \
+			$(OVMF_CVM_4M_FLAGS) -b $(BUILD_TYPE)
+			#-b $(BUILD_TYPE)
+	cp $(OVMF_CVM_BUILD_DIR)/FV/OVMF_CODE.fd \
+		$(OVMF_CVM_INSTALL_DIR)/OVMF_CVM_CODE_4M.fd
+	cp $(OVMF_CVM_BUILD_DIR)/FV/OVMF_VARS.fd \
+		$(OVMF_CVM_INSTALL_DIR)/OVMF_CVM_VARS_4M.fd
+	cp $(OVMF_CVM_BUILD_DIR)/FV/OVMF.fd \
+		$(OVMF_CVM_INSTALL_DIR)/OVMF_CVM_4M.fd
+
 build-ovmf: $(OVMF_BINARIES) $(OVMF_IMAGES) $(OVMF_PREENROLLED_VARS)
 $(OVMF_BINARIES) $(OVMF_IMAGES): debian/setup-build-stamp
 	rm -rf $(OVMF_INSTALL_DIR)
@@ -250,4 +275,4 @@ get-orig-source:
 		edk2-$(DEB_VERSION_UPSTREAM)
 	rm -rf edk2.tmp edk2-$(DEB_VERSION_UPSTREAM)
 
-.PHONY: build-ovmf build-ovmf32 build-qemu-efi build-qemu-efi-aarch64 build-qemu-efi-riscv64
+.PHONY: build-ovmf build-ovmf32 build-ovmf-cvm build-qemu-efi build-qemu-efi-aarch64 build-qemu-efi-riscv64
\ No newline at end of file
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH qemu-server v3 3/5] Convert policy calculation
  2025-02-24 12:37 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP Philipp Giersfeld
  2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 1/5] Update edk2 to edkstable202411 Philipp Giersfeld
  2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
@ 2025-02-24 12:37 ` Philipp Giersfeld
  2025-03-05 15:35   ` Fiona Ebner
  2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support Philipp Giersfeld
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Philipp Giersfeld @ 2025-02-24 12:37 UTC (permalink / raw)
  To: pve-devel

Convert policy calcucalation to use shift operators and OR operation
instead of binary numbers and addition.

Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
Reviewed-by: Daniel Kral <d.kral@proxmox.com>
Tested-by: Markus Frank <m.frank@proxmox.com>
---

 no changes since last version
 
 PVE/QemuServer/CPUConfig.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index e65d8c26..ad0be16e 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -846,12 +846,12 @@ sub get_amd_sev_object {
 
     # guest policy bit calculation as described here:
     # https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
-    my $policy = 0b0000;
-    $policy += 0b0001 if $amd_sev_conf->{'no-debug'};
-    $policy += 0b0010 if $amd_sev_conf->{'no-key-sharing'};
-    $policy += 0b0100 if $amd_sev_conf->{type} eq 'es';
+    my $policy = 0;
+    $policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
+    $policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
+    $policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
     # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
-    $policy += 0b1000;
+    $policy |= 1 << 3;
 
     $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
     $sev_mem_object .= ',kernel-hashes=on' if ($amd_sev_conf->{'kernel-hashes'});
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support.
  2025-02-24 12:37 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP Philipp Giersfeld
                   ` (2 preceding siblings ...)
  2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 3/5] Convert policy calculation Philipp Giersfeld
@ 2025-02-24 12:37 ` Philipp Giersfeld
  2025-03-05 15:35   ` Fiona Ebner
  2025-02-24 12:37 ` [pve-devel] [PATCH pve-manager v3 5/5] Add configuration options for AMD SEV-SNP Philipp Giersfeld
  2025-03-05  8:31 ` [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] " Philipp Giersfeld
  5 siblings, 1 reply; 15+ messages in thread
From: Philipp Giersfeld @ 2025-02-24 12:37 UTC (permalink / raw)
  To: pve-devel

This patch is for enabling AMD SEV-SNP support.

Where applicable, it extends support for existing SEV(-ES) variables
to SEV-SNP. This means that it retains no-debug and kernel-hashes
options, but the no-key-sharing option is removed.

The default policy value is identical to QEMU’s, and the therefore
required option has been added to configure SMT support.

The code was tested by running a VM without SEV, with SEV, SEV-ES,
SEV-SNP. Each configuration was tested with and without an EFI disk
attached. For SEV-enabled configurations it was also verified that the
kernel actually used the respective feature.

Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
Reviewed-by: Daniel Kral <d.kral@proxmox.com>
Tested-by: Markus Frank <m.frank@proxmox.com>
---

 no changes since last version

 PVE/API2/Qemu.pm            |  7 +++-
 PVE/QemuServer.pm           | 49 +++++++++++++++++++--------
 PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
 3 files changed, 92 insertions(+), 30 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 295260e7..4f48cf85 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -548,8 +548,13 @@ my sub create_disks : prototype($$$$$$$$$$$) {
 		my $volid;
 		if ($ds eq 'efidisk0') {
 		    my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
+
+		    my $amd_sev_type = PVE::QemuServer::CPUConfig::get_amd_sev_type($conf);
+		    die "SEV-SNP uses consolidated read-only firmware"
+			if $amd_sev_type && $amd_sev_type eq 'snp';
+
 		    ($volid, $size) = PVE::QemuServer::create_efidisk(
-			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
+			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm, $amd_sev_type);
 		} elsif ($ds eq 'tpmstate0') {
 		    # swtpm can only use raw volumes, and uses a fixed size
 		    $size = PVE::Tools::convert_size(PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE, 'b' => 'kb');
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 808c0e1c..51e551b8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -52,7 +52,7 @@ use PVE::QemuConfig;
 use PVE::QemuServer::Helpers qw(config_aware_timeout min_version kvm_user_version windows_version);
 use PVE::QemuServer::Cloudinit;
 use PVE::QemuServer::CGroup;
-use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object);
+use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object get_amd_sev_type);
 use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
@@ -88,6 +88,13 @@ my $OVMF = {
 	    "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd",
 	    "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
 	],
+	'4m-sev' => [
+	    "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd",
+	    "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd",
+	],
+	'4m-snp' => [
+	    "$EDK2_FW_BASE/OVMF_CVM_4M.fd",
+	],
 	# FIXME: These are legacy 2MB-sized images that modern OVMF doesn't supports to build
 	# anymore. how can we deperacate this sanely without breaking existing instances, or using
 	# older backups and snapshot?
@@ -3172,15 +3179,22 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-sub get_ovmf_files($$$) {
-    my ($arch, $efidisk, $smm) = @_;
+sub get_ovmf_files($$$$) {
+    my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
 
     my $types = $OVMF->{$arch}
 	or die "no OVMF images known for architecture '$arch'\n";
 
     my $type = 'default';
     if ($arch eq 'x86_64') {
-	if (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
+	if ($amd_sev_type && $amd_sev_type eq 'snp') {
+	    $type = "4m-snp";
+	    my ($ovmf) = $types->{$type}->@*;
+	    die "EFI base image '$ovmf' not found\n" if ! -f $ovmf;
+	    return ($ovmf);
+	} elsif ($amd_sev_type) {
+	    $type = "4m-sev";
+	} elsif (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
 	    $type = $smm ? "4m" : "4m-no-smm";
 	    $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
 	} else {
@@ -3329,7 +3343,8 @@ my sub print_ovmf_drive_commandlines {
 
     my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
 
-    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35);
+    my $amd_sev_type = get_amd_sev_type($conf);
+    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35, $amd_sev_type);
 
     my $var_drive_str = "if=pflash,unit=1,id=drive-efidisk0";
     if ($d) {
@@ -3526,10 +3541,17 @@ sub config_to_command {
 	die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n"
 	    if !$forcecpu && get_cpu_bitness($conf->{cpu}, $arch) == 32;
 
-	my ($code_drive_str, $var_drive_str) =
-	    print_ovmf_drive_commandlines($conf, $storecfg, $vmid, $arch, $q35, $version_guard);
-	push $cmd->@*, '-drive', $code_drive_str;
-	push $cmd->@*, '-drive', $var_drive_str;
+	my $amd_sev_type = get_amd_sev_type($conf);
+	if ($amd_sev_type && $amd_sev_type eq 'snp') {
+	   my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
+	   my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
+	   push $cmd->@*, '-bios', get_ovmf_files($arch, $d, undef, $amd_sev_type);
+	} else {
+	    my ($code_drive_str, $var_drive_str) = print_ovmf_drive_commandlines(
+		$conf, $storecfg, $vmid, $arch, $q35, $version_guard);
+	    push $cmd->@*, '-drive', $code_drive_str;
+	    push $cmd->@*, '-drive', $var_drive_str;
+	}
     }
 
     if ($q35) { # tell QEMU to load q35 config early
@@ -8337,7 +8359,8 @@ sub get_efivars_size {
     my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
     $efidisk //= $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
     my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
-    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm);
+    my $amd_sev_type = get_amd_sev_type($conf);
+    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm, $amd_sev_type);
     return -s $ovmf_vars;
 }
 
@@ -8361,10 +8384,10 @@ sub update_tpmstate_size {
     $conf->{tpmstate0} = print_drive($disk);
 }
 
-sub create_efidisk($$$$$$$) {
-    my ($storecfg, $storeid, $vmid, $fmt, $arch, $efidisk, $smm) = @_;
+sub create_efidisk($$$$$$$$) {
+    my ($storecfg, $storeid, $vmid, $fmt, $arch, $efidisk, $smm, $amd_sev_type) = @_;
 
-    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm);
+    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm, $amd_sev_type);
 
     my $vars_size_b = -s $ovmf_vars;
     my $vars_size = PVE::Tools::convert_size($vars_size_b, 'b' => 'kb');
diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index ad0be16e..94a00a4e 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -18,6 +18,7 @@ get_cpu_options
 get_cpu_bitness
 is_native_arch
 get_amd_sev_object
+get_amd_sev_type
 );
 
 # under certain race-conditions, this module might be loaded before pve-cluster
@@ -231,25 +232,32 @@ my $cpu_fmt = {
 my $sev_fmt = {
     type => {
 	description => "Enable standard SEV with type='std' or enable"
-	    ." experimental SEV-ES with the 'es' option.",
+	    ." experimental SEV-ES with the 'es' option or enable "
+	    ."experimental SEV-SNP with the 'snp' option.",
 	type => 'string',
 	default_key => 1,
 	format_description => "sev-type",
-	enum => ['std', 'es'],
+	enum => ['std', 'es', 'snp'],
 	maxLength => 3,
     },
     'no-debug' => {
-	description => "Sets policy bit 0 to 1 to disallow debugging of guest",
+	description => "Sets policy bit to disallow debugging of guest",
 	type => 'boolean',
 	default => 0,
 	optional => 1,
     },
     'no-key-sharing' => {
-	description => "Sets policy bit 1 to 1 to disallow key sharing with other guests",
+	description => "Sets policy bit to disallow key sharing with other guests",
 	type => 'boolean',
 	default => 0,
 	optional => 1,
     },
+    'allow-smt' => {
+	description => "Sets policy bit to allow Simultaneous Multi Threading (SMT)",
+	type => 'boolean',
+	default => 1,
+	optional => 1,
+    },
     "kernel-hashes" => {
 	description => "Add kernel hashes to guest firmware for measured linux kernel launch",
 	type => 'boolean',
@@ -823,6 +831,13 @@ sub get_hw_capabilities {
     }
     return $hw_capabilities;
 }
+sub get_amd_sev_type {
+    my ($conf) = @_;
+
+    return undef if !$conf->{'amd-sev'};
+
+    return PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{'amd-sev'})->{type};
+}
 
 sub get_amd_sev_object {
     my ($amd_sev, $bios) = @_;
@@ -836,22 +851,41 @@ sub get_amd_sev_object {
     if ($amd_sev_conf->{type} eq 'es' && !$sev_hw_caps->{'sev-support-es'}) {
 	die "Your CPU does not support AMD SEV-ES.\n";
     }
+    if ($amd_sev_conf->{type} eq 'snp' && !$sev_hw_caps->{'sev-support-snp'}) {
+	die "Your CPU does not support AMD SEV-SNP.\n";
+    }
     if (!$bios || $bios ne 'ovmf') {
 	die "To use AMD SEV, you need to change the BIOS to OVMF.\n";
     }
 
-    my $sev_mem_object = 'sev-guest,id=sev0';
-    $sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
-    $sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
-
-    # guest policy bit calculation as described here:
-    # https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
-    my $policy = 0;
-    $policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
-    $policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
-    $policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
-    # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
-    $policy |= 1 << 3;
+    my $sev_mem_object = '';
+    my $policy;
+    if ($amd_sev_conf->{type} eq 'es' || $amd_sev_conf->{type} eq 'std') {
+	$sev_mem_object .= 'sev-guest,id=sev0';
+	$sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
+	$sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
+
+	# guest policy bit calculation as described here:
+	# https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
+	$policy = 0;
+	$policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
+	$policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
+	$policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
+	# disable migration with bit 3 nosend to prevent amd-sev-migration-attack
+	$policy |= 1 << 3;
+    } elsif ($amd_sev_conf->{type} eq 'snp') {
+	$sev_mem_object .= 'sev-snp-guest,id=sev0';
+	$sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
+	$sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
+
+	# guest policy bit calculation as described in chapter 4.3:
+	# https://www.amd.com/system/files/TechDocs/56860.pdf
+	# Reserved bit must be one
+	$policy = 1 << 17;
+	$policy |= 1 << 16 if $amd_sev_conf->{'allow-smt'};
+	$policy |= 1 << 19 if !$amd_sev_conf->{'no-debug'};
+    }
+
 
     $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
     $sev_mem_object .= ',kernel-hashes=on' if ($amd_sev_conf->{'kernel-hashes'});
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] [PATCH pve-manager v3 5/5] Add configuration options for AMD SEV-SNP
  2025-02-24 12:37 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP Philipp Giersfeld
                   ` (3 preceding siblings ...)
  2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support Philipp Giersfeld
@ 2025-02-24 12:37 ` Philipp Giersfeld
  2025-03-05  8:31 ` [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] " Philipp Giersfeld
  5 siblings, 0 replies; 15+ messages in thread
From: Philipp Giersfeld @ 2025-02-24 12:37 UTC (permalink / raw)
  To: pve-devel

Expand input panel with AMD SEV-SNP selection, and relevant optional
parameters similar to existing options for AMD SEV(-ES).

Further, upon selecting AMD SEV-SNP, issue a warning that EFI disks are
not included when using SEV-SNP.

Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
Reviewed-by: Daniel Kral <d.kral@proxmox.com>
---

 changes since v2: https://lists.proxmox.com/pipermail/pve-devel/2025-February/068418.html
 * Only set no-key-sharing if SNP is not used
 * Fix formatting

 www/manager6/qemu/Options.js |  1 +
 www/manager6/qemu/SevEdit.js | 46 ++++++++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index cbe9e52b..49a921cd 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -346,6 +346,7 @@ Ext.define('PVE.qemu.Options', {
 		    let amd_sev = PVE.Parser.parsePropertyString(value, "type");
 		    if (amd_sev.type === 'std') return 'AMD SEV (' + value + ')';
 		    if (amd_sev.type === 'es') return 'AMD SEV-ES (' + value + ')';
+		    if (amd_sev.type === 'snp') return 'AMD SEV-SNP (' + value + ')';
 		    return value;
 		},
 	    },
diff --git a/www/manager6/qemu/SevEdit.js b/www/manager6/qemu/SevEdit.js
index a2080f2d..3e0d0cbb 100644
--- a/www/manager6/qemu/SevEdit.js
+++ b/www/manager6/qemu/SevEdit.js
@@ -9,7 +9,8 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	    type: '__default__',
 	},
 	formulas: {
-	    sevEnabled: get => get('type') !== '__default__',
+	    sevEnabled: get => get('type') === 'std' || get('type') === 'es' || get('type') === 'snp',
+	    snpEnabled: get => get('type') === 'snp',
 	},
     },
 
@@ -21,10 +22,14 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	if (!values.debug) {
 	    values["no-debug"] = 1;
 	}
-	if (!values["key-sharing"]) {
+	if (values.smt) {
+	    values["allow-smt"] = 1;
+	}
+	if (!values["key-sharing"] && values.type !== 'snp') {
 	    values["no-key-sharing"] = 1;
 	}
 	delete values.debug;
+	delete values.smt;
 	delete values["key-sharing"];
 	let ret = {};
 	ret['amd-sev'] = PVE.Parser.printPropertyString(values, 'type');
@@ -36,13 +41,16 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	if (PVE.Parser.parseBoolean(values["no-debug"])) {
 	    values.debug = 0;
 	}
+	if (PVE.Parser.parseBoolean(values["allow-smt"])) {
+	    values.smt = 1;
+	}
 	if (PVE.Parser.parseBoolean(values["no-key-sharing"])) {
 	    values["key-sharing"] = 0;
 	}
 	this.callParent(arguments);
     },
 
-    items: {
+	items: [{
 	xtype: 'proxmoxKVComboBox',
 	fieldLabel: gettext('AMD SEV Type'),
 	labelWidth: 150,
@@ -52,11 +60,28 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	    ['__default__', Proxmox.Utils.defaultText + ' (' + Proxmox.Utils.disabledText + ')'],
 	    ['std', 'AMD SEV'],
 	    ['es', 'AMD SEV-ES (highly experimental)'],
+	    ['snp', 'AMD SEV-SNP (highly experimental)'],
 	],
 	bind: {
 	    value: '{type}',
 	},
     },
+    {
+	xtype: 'displayfield',
+	userCls: 'pmx-hint',
+	value: gettext('WARNING: When using SEV-SNP no EFI disk is loaded as pflash.'),
+	bind: {
+	    hidden: '{!snpEnabled}',
+	},
+    },
+    {
+	xtype: 'displayfield',
+	userCls: 'pmx-hint',
+	value: gettext('Note: SEV-SNP requires host kernel version 6.11 or higher.'),
+	bind: {
+	    hidden: '{!snpEnabled}',
+	},
+    }],
 
     advancedItems: [
 	{
@@ -77,8 +102,19 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	    name: 'key-sharing',
 	    value: 1,
 	    bind: {
-		hidden: '{!sevEnabled}',
-		disabled: '{!sevEnabled}',
+		hidden: '{!sevEnabled || snpEnabled}',
+		disabled: '{!sevEnabled || snpEnabled}',
+	    },
+	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Allow SMT'),
+	    labelWidth: 150,
+	    name: 'smt',
+	    value: 1,
+	    bind: {
+		hidden: '{!snpEnabled}',
+		disabled: '{!snpEnabled}',
 	    },
 	},
 	{
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP
  2025-02-24 12:37 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP Philipp Giersfeld
                   ` (4 preceding siblings ...)
  2025-02-24 12:37 ` [pve-devel] [PATCH pve-manager v3 5/5] Add configuration options for AMD SEV-SNP Philipp Giersfeld
@ 2025-03-05  8:31 ` Philipp Giersfeld
  5 siblings, 0 replies; 15+ messages in thread
From: Philipp Giersfeld @ 2025-03-05  8:31 UTC (permalink / raw)
  To: Proxmox VE development discussion

On 25/02/24 01:37PM, Philipp Giersfeld wrote:
> This patch series adds support for AMD SEV-SNP. 
> Where possible it mimics the existing support for AMD SEV(-ES). 
> 
> Running SEV-SNP VMs requires a more recent version of edk2
> and OVMF firmware image. Contrary to other setups, SEV-SNP does not support loading the firmware via pflash. Instead, the firmware image is loaded  via the -bios option.
> 
> ---
> 
>  changes since v2: https://lists.proxmox.com/pipermail/pve-devel/2025-February/068418.html
>  * Only set no-key-sharing if SNP is not used
>  * Fix formatting
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
ping

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP
  2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
@ 2025-03-05 14:18   ` Fiona Ebner
  2025-03-05 15:35     ` Thomas Lamprecht
  2025-03-11 12:31     ` Philipp Giersfeld
  0 siblings, 2 replies; 15+ messages in thread
From: Fiona Ebner @ 2025-03-05 14:18 UTC (permalink / raw)
  To: Proxmox VE development discussion, Philipp Giersfeld

Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
> AMD SEV-SNP boots with a single volatile firmware image OVMF.fd via the
> -bios option.
> 
> Currently, an SEV-enabled VM will not boot with an OVMF
> firmware that was compiled with `SECURE_BOOT_ENABLE` [1].
> 
> Furthermore, during testing, SEV-enabled amchines did not boot with
> `SMM_REQUIRE`.
> 
> Therefore, introduce a new target build-ovmf-cvm that builds OVMF
> firmware suitable for AMD SEV.
> 
> [1] https://github.com/tianocore/edk2/pull/6285
> 

This has been merged in edk2-stable202502, which is already out now. I'd
prefer going directly for that tag. Can we avoid splitting out the
SMM_REQUIRE flag then?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH qemu-server v3 3/5] Convert policy calculation
  2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 3/5] Convert policy calculation Philipp Giersfeld
@ 2025-03-05 15:35   ` Fiona Ebner
  0 siblings, 0 replies; 15+ messages in thread
From: Fiona Ebner @ 2025-03-05 15:35 UTC (permalink / raw)
  To: Proxmox VE development discussion, Philipp Giersfeld

Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
> Convert policy calcucalation to use shift operators and OR operation

Nit: there is a typo here: calcucalation

> instead of binary numbers and addition.
> 
> Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
> Reviewed-by: Daniel Kral <d.kral@proxmox.com>
> Tested-by: Markus Frank <m.frank@proxmox.com>

With the typo above fixed:

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

> ---
> 
>  no changes since last version
>  
>  PVE/QemuServer/CPUConfig.pm | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index e65d8c26..ad0be16e 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -846,12 +846,12 @@ sub get_amd_sev_object {
>  
>      # guest policy bit calculation as described here:
>      # https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
> -    my $policy = 0b0000;
> -    $policy += 0b0001 if $amd_sev_conf->{'no-debug'};
> -    $policy += 0b0010 if $amd_sev_conf->{'no-key-sharing'};
> -    $policy += 0b0100 if $amd_sev_conf->{type} eq 'es';
> +    my $policy = 0;
> +    $policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
> +    $policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
> +    $policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
>      # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> -    $policy += 0b1000;
> +    $policy |= 1 << 3;
>  
>      $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
>      $sev_mem_object .= ',kernel-hashes=on' if ($amd_sev_conf->{'kernel-hashes'});

While at it, we could also go for using constants, i.e. something like:
$policy |= AMD_SEV_POLICY_NO_DEBUG if $amd_sev_conf->{'no-debug'};
and for the one in the next patch, AMD_SEV_SNP_POLICY_NO_DEBUG.

But not a blocker from my side, if you don't want to go for it.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support.
  2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support Philipp Giersfeld
@ 2025-03-05 15:35   ` Fiona Ebner
  2025-03-11 13:56     ` Philipp Giersfeld
  0 siblings, 1 reply; 15+ messages in thread
From: Fiona Ebner @ 2025-03-05 15:35 UTC (permalink / raw)
  To: Proxmox VE development discussion, Philipp Giersfeld

Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
> This patch is for enabling AMD SEV-SNP support.
> 
> Where applicable, it extends support for existing SEV(-ES) variables
> to SEV-SNP. This means that it retains no-debug and kernel-hashes
> options, but the no-key-sharing option is removed.
> 
> The default policy value is identical to QEMU’s, and the therefore
> required option has been added to configure SMT support.
> 
> The code was tested by running a VM without SEV, with SEV, SEV-ES,
> SEV-SNP. Each configuration was tested with and without an EFI disk
> attached. For SEV-enabled configurations it was also verified that the
> kernel actually used the respective feature.
> 
> Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
> Reviewed-by: Daniel Kral <d.kral@proxmox.com>
> Tested-by: Markus Frank <m.frank@proxmox.com>
> ---
> 
>  no changes since last version
> 
>  PVE/API2/Qemu.pm            |  7 +++-
>  PVE/QemuServer.pm           | 49 +++++++++++++++++++--------
>  PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
>  3 files changed, 92 insertions(+), 30 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 295260e7..4f48cf85 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -548,8 +548,13 @@ my sub create_disks : prototype($$$$$$$$$$$) {
>  		my $volid;
>  		if ($ds eq 'efidisk0') {
>  		    my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
> +
> +		    my $amd_sev_type = PVE::QemuServer::CPUConfig::get_amd_sev_type($conf);
> +		    die "SEV-SNP uses consolidated read-only firmware"

Missing newline at the end of error message. Perl will automatically
attach the filename and line number then, which is rather ugly here. I'd
also mention "and doesn't need an EFI disk" to clarify this a bit more
for users.

> +			if $amd_sev_type && $amd_sev_type eq 'snp';
> +
>  		    ($volid, $size) = PVE::QemuServer::create_efidisk(
> -			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
> +			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm, $amd_sev_type);
>  		} elsif ($ds eq 'tpmstate0') {
>  		    # swtpm can only use raw volumes, and uses a fixed size
>  		    $size = PVE::Tools::convert_size(PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE, 'b' => 'kb');
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 808c0e1c..51e551b8 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -52,7 +52,7 @@ use PVE::QemuConfig;
>  use PVE::QemuServer::Helpers qw(config_aware_timeout min_version kvm_user_version windows_version);
>  use PVE::QemuServer::Cloudinit;
>  use PVE::QemuServer::CGroup;
> -use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object);
> +use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object get_amd_sev_type);
>  use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
>  use PVE::QemuServer::Machine;
>  use PVE::QemuServer::Memory qw(get_current_memory);
> @@ -88,6 +88,13 @@ my $OVMF = {
>  	    "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd",
>  	    "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
>  	],
> +	'4m-sev' => [
> +	    "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd",
> +	    "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd", 

Style nit: trailing whitespace above

Is switching over for the std+ES SEV scenarios still
required/advantageous if we go for edk2-stable202502?

> +	],
> +	'4m-snp' => [
> +	    "$EDK2_FW_BASE/OVMF_CVM_4M.fd",
> +	],
>  	# FIXME: These are legacy 2MB-sized images that modern OVMF doesn't supports to build
>  	# anymore. how can we deperacate this sanely without breaking existing instances, or using
>  	# older backups and snapshot?
> @@ -3172,15 +3179,22 @@ sub vga_conf_has_spice {
>      return $1 || 1;
>  }
>  
> -sub get_ovmf_files($$$) {
> -    my ($arch, $efidisk, $smm) = @_;
> +sub get_ovmf_files($$$$) {
> +    my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
>  
>      my $types = $OVMF->{$arch}
>  	or die "no OVMF images known for architecture '$arch'\n";
>  
>      my $type = 'default';
>      if ($arch eq 'x86_64') {
> -	if (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
> +	if ($amd_sev_type && $amd_sev_type eq 'snp') {
> +	    $type = "4m-snp";
> +	    my ($ovmf) = $types->{$type}->@*;
> +	    die "EFI base image '$ovmf' not found\n" if ! -f $ovmf;
> +	    return ($ovmf);
> +	} elsif ($amd_sev_type) {
> +	    $type = "4m-sev";
> +	} elsif (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
>  	    $type = $smm ? "4m" : "4m-no-smm";
>  	    $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
>  	} else {
> @@ -3329,7 +3343,8 @@ my sub print_ovmf_drive_commandlines {
>  
>      my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
>  
> -    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35);
> +    my $amd_sev_type = get_amd_sev_type($conf);

I'd die here if the type is 'snp' just to be sure, so that we have a
clear error should that ever happen by accident.

> +    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35, $amd_sev_type);
>  
>      my $var_drive_str = "if=pflash,unit=1,id=drive-efidisk0";
>      if ($d) {
> @@ -3526,10 +3541,17 @@ sub config_to_command {
>  	die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n"
>  	    if !$forcecpu && get_cpu_bitness($conf->{cpu}, $arch) == 32;
>  
> -	my ($code_drive_str, $var_drive_str) =
> -	    print_ovmf_drive_commandlines($conf, $storecfg, $vmid, $arch, $q35, $version_guard);
> -	push $cmd->@*, '-drive', $code_drive_str;
> -	push $cmd->@*, '-drive', $var_drive_str;
> +	my $amd_sev_type = get_amd_sev_type($conf);
> +	if ($amd_sev_type && $amd_sev_type eq 'snp') {
> +	   my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
> +	   my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;

Since the EFI disk is not used in this case, we should always pass undef
to be explicit. We can print an informational message that it will be
ignored if present.

> +	   push $cmd->@*, '-bios', get_ovmf_files($arch, $d, undef, $amd_sev_type);
> +	} else {
> +	    my ($code_drive_str, $var_drive_str) = print_ovmf_drive_commandlines(
> +		$conf, $storecfg, $vmid, $arch, $q35, $version_guard);
> +	    push $cmd->@*, '-drive', $code_drive_str;
> +	    push $cmd->@*, '-drive', $var_drive_str;
> +	}
>      }
>  
>      if ($q35) { # tell QEMU to load q35 config early

---snip 8<---

> @@ -231,25 +232,32 @@ my $cpu_fmt = {
>  my $sev_fmt = {
>      type => {
>  	description => "Enable standard SEV with type='std' or enable"
> -	    ." experimental SEV-ES with the 'es' option.",
> +	    ." experimental SEV-ES with the 'es' option or enable "
> +	    ."experimental SEV-SNP with the 'snp' option.",

Style nit: it's preferred to use the space at the beginning of the next
line: https://pve.proxmox.com/wiki/Perl_Style_Guide#Wrapping_Strings

>  	type => 'string',
>  	default_key => 1,
>  	format_description => "sev-type",
> -	enum => ['std', 'es'],
> +	enum => ['std', 'es', 'snp'],
>  	maxLength => 3,
>      },
>      'no-debug' => {
> -	description => "Sets policy bit 0 to 1 to disallow debugging of guest",
> +	description => "Sets policy bit to disallow debugging of guest",
>  	type => 'boolean',
>  	default => 0,
>  	optional => 1,
>      },
>      'no-key-sharing' => {
> -	description => "Sets policy bit 1 to 1 to disallow key sharing with other guests",
> +	description => "Sets policy bit to disallow key sharing with other guests",

I'd also mention that it is ignored if using SNP

>  	type => 'boolean',
>  	default => 0,
>  	optional => 1,
>      },
> +    'allow-smt' => {
> +	description => "Sets policy bit to allow Simultaneous Multi Threading (SMT)",

I'd also mention that it is ignored if not using SNP

> +	type => 'boolean',
> +	default => 1,
> +	optional => 1,
> +    },
>      "kernel-hashes" => {
>  	description => "Add kernel hashes to guest firmware for measured linux kernel launch",
>  	type => 'boolean',
> @@ -823,6 +831,13 @@ sub get_hw_capabilities {
>      }
>      return $hw_capabilities;
>  }
> +sub get_amd_sev_type {
> +    my ($conf) = @_;
> +
> +    return undef if !$conf->{'amd-sev'};
> +
> +    return PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{'amd-sev'})->{type};
> +}
>  
>  sub get_amd_sev_object {
>      my ($amd_sev, $bios) = @_;
> @@ -836,22 +851,41 @@ sub get_amd_sev_object {
>      if ($amd_sev_conf->{type} eq 'es' && !$sev_hw_caps->{'sev-support-es'}) {
>  	die "Your CPU does not support AMD SEV-ES.\n";
>      }
> +    if ($amd_sev_conf->{type} eq 'snp' && !$sev_hw_caps->{'sev-support-snp'}) {
> +	die "Your CPU does not support AMD SEV-SNP.\n";
> +    }
>      if (!$bios || $bios ne 'ovmf') {
>  	die "To use AMD SEV, you need to change the BIOS to OVMF.\n";
>      }
>  
> -    my $sev_mem_object = 'sev-guest,id=sev0';
> -    $sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> -    $sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> -
> -    # guest policy bit calculation as described here:
> -    # https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
> -    my $policy = 0;
> -    $policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
> -    $policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
> -    $policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
> -    # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> -    $policy |= 1 << 3;
> +    my $sev_mem_object = '';
> +    my $policy;
> +    if ($amd_sev_conf->{type} eq 'es' || $amd_sev_conf->{type} eq 'std') {
> +	$sev_mem_object .= 'sev-guest,id=sev0';
> +	$sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> +	$sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> +
> +	# guest policy bit calculation as described here:
> +	# https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
> +	$policy = 0;
> +	$policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
> +	$policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
> +	$policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
> +	# disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> +	$policy |= 1 << 3;
> +    } elsif ($amd_sev_conf->{type} eq 'snp') {
> +	$sev_mem_object .= 'sev-snp-guest,id=sev0';
> +	$sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> +	$sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> +
> +	# guest policy bit calculation as described in chapter 4.3:
> +	# https://www.amd.com/system/files/TechDocs/56860.pdf
> +	# Reserved bit must be one
> +	$policy = 1 << 17;
> +	$policy |= 1 << 16 if $amd_sev_conf->{'allow-smt'};

In the schema, the default is set to 1. Note that this is only
informational unfortunately, i.e. parse_property_string() will not fill
in a default. You need to apply the default yourself here using

> if !defined($amd_sev_conf->{'allow-smt'}) || $amd_sev_conf->{'allow-smt'};


> +	$policy |= 1 << 19 if !$amd_sev_conf->{'no-debug'};

Note that here, it already works automatically, because the default is 0.

> +    }
> +
>  
>      $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
>      $sev_mem_object .= ',kernel-hashes=on' if ($amd_sev_conf->{'kernel-hashes'});



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP
  2025-03-05 14:18   ` Fiona Ebner
@ 2025-03-05 15:35     ` Thomas Lamprecht
  2025-03-11 12:31     ` Philipp Giersfeld
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2025-03-05 15:35 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner, Philipp Giersfeld

Am 05.03.25 um 15:18 schrieb Fiona Ebner:
> Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
>> AMD SEV-SNP boots with a single volatile firmware image OVMF.fd via the
>> -bios option.
>>
>> Currently, an SEV-enabled VM will not boot with an OVMF
>> firmware that was compiled with `SECURE_BOOT_ENABLE` [1].
>>
>> Furthermore, during testing, SEV-enabled amchines did not boot with
>> `SMM_REQUIRE`.
>>
>> Therefore, introduce a new target build-ovmf-cvm that builds OVMF
>> firmware suitable for AMD SEV.
>>
>> [1] https://github.com/tianocore/edk2/pull/6285
>>
> 
> This has been merged in edk2-stable202502, which is already out now. I'd
> prefer going directly for that tag. Can we avoid splitting out the
> SMM_REQUIRE flag then?
>

An edk2 update is due since quite a bit already, so I'd apply your first
patch and would rebase on latest edk2-stable202502 and release that; we
can still do another bump if something else is needed while getting
feedback about the new version independent of SEV related changes already.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pve-devel] applied: [PATCH edk2-firmware v3 1/5] Update edk2 to edkstable202411
  2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 1/5] Update edk2 to edkstable202411 Philipp Giersfeld
@ 2025-03-05 16:51   ` Thomas Lamprecht
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2025-03-05 16:51 UTC (permalink / raw)
  To: Proxmox VE development discussion, Philipp Giersfeld

Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
> Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
> Tested-by: Markus Frank <m.frank@proxmox.com>
> ---
> 
>  no changes since last version
> 
>  debian/binary-check.remove | 4 ++--
>  debian/rules               | 6 +++---
>  edk2                       | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
>

applied this one, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP
  2025-03-05 14:18   ` Fiona Ebner
  2025-03-05 15:35     ` Thomas Lamprecht
@ 2025-03-11 12:31     ` Philipp Giersfeld
  1 sibling, 0 replies; 15+ messages in thread
From: Philipp Giersfeld @ 2025-03-11 12:31 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: Proxmox VE development discussion

On 25/03/05 03:18PM, Fiona Ebner wrote:
> Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
> > AMD SEV-SNP boots with a single volatile firmware image OVMF.fd via the
> > -bios option.
> > 
> > Currently, an SEV-enabled VM will not boot with an OVMF
> > firmware that was compiled with `SECURE_BOOT_ENABLE` [1].
> > 
> > Furthermore, during testing, SEV-enabled amchines did not boot with
> > `SMM_REQUIRE`.
> > 
> > Therefore, introduce a new target build-ovmf-cvm that builds OVMF
> > firmware suitable for AMD SEV.
> > 
> > [1] https://github.com/tianocore/edk2/pull/6285
> > 
> 
> This has been merged in edk2-stable202502, which is already out now. I'd
> prefer going directly for that tag. Can we avoid splitting out the
> SMM_REQUIRE flag then?
> 
(Assuming you mean the SECURE_BOOT flag)
Yes, I also prefer going directly for edk2-stable202502. I already tested it
briefly and will prepare an updated version of the patch.

Splitting out SMM cannot be avoided since SEV-ES and SEV-SNP do not support it [1,2].

[1] https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html
[2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support.
  2025-03-05 15:35   ` Fiona Ebner
@ 2025-03-11 13:56     ` Philipp Giersfeld
  2025-03-11 14:22       ` Fiona Ebner
  0 siblings, 1 reply; 15+ messages in thread
From: Philipp Giersfeld @ 2025-03-11 13:56 UTC (permalink / raw)
  To: Fiona Ebner; +Cc: Proxmox VE development discussion

On 25/03/05 04:35PM, Fiona Ebner wrote:
> Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
> > This patch is for enabling AMD SEV-SNP support.
> > 
> > Where applicable, it extends support for existing SEV(-ES) variables
> > to SEV-SNP. This means that it retains no-debug and kernel-hashes
> > options, but the no-key-sharing option is removed.
> > 
> > The default policy value is identical to QEMU’s, and the therefore
> > required option has been added to configure SMT support.
> > 
> > The code was tested by running a VM without SEV, with SEV, SEV-ES,
> > SEV-SNP. Each configuration was tested with and without an EFI disk
> > attached. For SEV-enabled configurations it was also verified that the
> > kernel actually used the respective feature.
> > 
> > Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
> > Reviewed-by: Daniel Kral <d.kral@proxmox.com>
> > Tested-by: Markus Frank <m.frank@proxmox.com>
> > ---
> > 
> >  no changes since last version
> > 
> >  PVE/API2/Qemu.pm            |  7 +++-
> >  PVE/QemuServer.pm           | 49 +++++++++++++++++++--------
> >  PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
> >  3 files changed, 92 insertions(+), 30 deletions(-)
> > 
> > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> > index 295260e7..4f48cf85 100644
> > --- a/PVE/API2/Qemu.pm
> > +++ b/PVE/API2/Qemu.pm
> > @@ -548,8 +548,13 @@ my sub create_disks : prototype($$$$$$$$$$$) {
> >  		my $volid;
> >  		if ($ds eq 'efidisk0') {
> >  		    my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
> > +
> > +		    my $amd_sev_type = PVE::QemuServer::CPUConfig::get_amd_sev_type($conf);
> > +		    die "SEV-SNP uses consolidated read-only firmware"
> 
> Missing newline at the end of error message. Perl will automatically
> attach the filename and line number then, which is rather ugly here. I'd
> also mention "and doesn't need an EFI disk" to clarify this a bit more
> for users.

Does "SEV-SNP uses consolidated read-only firmware instead of a seperate
non-volatile variable store\n" work?

> 
> > +			if $amd_sev_type && $amd_sev_type eq 'snp';
> > +
> >  		    ($volid, $size) = PVE::QemuServer::create_efidisk(
> > -			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
> > +			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm, $amd_sev_type);
> >  		} elsif ($ds eq 'tpmstate0') {
> >  		    # swtpm can only use raw volumes, and uses a fixed size
> >  		    $size = PVE::Tools::convert_size(PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE, 'b' => 'kb');
> > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> > index 808c0e1c..51e551b8 100644
> > --- a/PVE/QemuServer.pm
> > +++ b/PVE/QemuServer.pm
> > @@ -52,7 +52,7 @@ use PVE::QemuConfig;
> >  use PVE::QemuServer::Helpers qw(config_aware_timeout min_version kvm_user_version windows_version);
> >  use PVE::QemuServer::Cloudinit;
> >  use PVE::QemuServer::CGroup;
> > -use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object);
> > +use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object get_amd_sev_type);
> >  use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
> >  use PVE::QemuServer::Machine;
> >  use PVE::QemuServer::Memory qw(get_current_memory);
> > @@ -88,6 +88,13 @@ my $OVMF = {
> >  	    "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd",
> >  	    "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
> >  	],
> > +	'4m-sev' => [
> > +	    "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd",
> > +	    "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd", 
> 
> Style nit: trailing whitespace above
> 
> Is switching over for the std+ES SEV scenarios still
> required/advantageous if we go for edk2-stable202502?

From my testing this is still the case.
IIUC, this is because the existing firmware is built targeting
OmvfPkg/OvmfPkgI32X64.dsc which does not support SEV.

> 
> > +	],
> > +	'4m-snp' => [
> > +	    "$EDK2_FW_BASE/OVMF_CVM_4M.fd",
> > +	],
> >  	# FIXME: These are legacy 2MB-sized images that modern OVMF doesn't supports to build
> >  	# anymore. how can we deperacate this sanely without breaking existing instances, or using
> >  	# older backups and snapshot?
> > @@ -3172,15 +3179,22 @@ sub vga_conf_has_spice {
> >      return $1 || 1;
> >  }
> >  
> > -sub get_ovmf_files($$$) {
> > -    my ($arch, $efidisk, $smm) = @_;
> > +sub get_ovmf_files($$$$) {
> > +    my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
> >  
> >      my $types = $OVMF->{$arch}
> >  	or die "no OVMF images known for architecture '$arch'\n";
> >  
> >      my $type = 'default';
> >      if ($arch eq 'x86_64') {
> > -	if (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
> > +	if ($amd_sev_type && $amd_sev_type eq 'snp') {
> > +	    $type = "4m-snp";
> > +	    my ($ovmf) = $types->{$type}->@*;
> > +	    die "EFI base image '$ovmf' not found\n" if ! -f $ovmf;
> > +	    return ($ovmf);
> > +	} elsif ($amd_sev_type) {
> > +	    $type = "4m-sev";
> > +	} elsif (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
> >  	    $type = $smm ? "4m" : "4m-no-smm";
> >  	    $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
> >  	} else {
> > @@ -3329,7 +3343,8 @@ my sub print_ovmf_drive_commandlines {
> >  
> >      my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
> >  
> > -    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35);
> > +    my $amd_sev_type = get_amd_sev_type($conf);
> 
> I'd die here if the type is 'snp' just to be sure, so that we have a
> clear error should that ever happen by accident.

Ack

> 
> > +    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35, $amd_sev_type);
> >  
> >      my $var_drive_str = "if=pflash,unit=1,id=drive-efidisk0";
> >      if ($d) {
> > @@ -3526,10 +3541,17 @@ sub config_to_command {
> >  	die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n"
> >  	    if !$forcecpu && get_cpu_bitness($conf->{cpu}, $arch) == 32;
> >  
> > -	my ($code_drive_str, $var_drive_str) =
> > -	    print_ovmf_drive_commandlines($conf, $storecfg, $vmid, $arch, $q35, $version_guard);
> > -	push $cmd->@*, '-drive', $code_drive_str;
> > -	push $cmd->@*, '-drive', $var_drive_str;
> > +	my $amd_sev_type = get_amd_sev_type($conf);
> > +	if ($amd_sev_type && $amd_sev_type eq 'snp') {
> > +	   my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
> > +	   my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
> 
> Since the EFI disk is not used in this case, we should always pass undef
> to be explicit. We can print an informational message that it will be
> ignored if present.
> 
> > +	   push $cmd->@*, '-bios', get_ovmf_files($arch, $d, undef, $amd_sev_type);
> > +	} else {
> > +	    my ($code_drive_str, $var_drive_str) = print_ovmf_drive_commandlines(
> > +		$conf, $storecfg, $vmid, $arch, $q35, $version_guard);
> > +	    push $cmd->@*, '-drive', $code_drive_str;
> > +	    push $cmd->@*, '-drive', $var_drive_str;
> > +	}
> >      }
> >  
> >      if ($q35) { # tell QEMU to load q35 config early
> 
> ---snip 8<---
> 
> > @@ -231,25 +232,32 @@ my $cpu_fmt = {
> >  my $sev_fmt = {
> >      type => {
> >  	description => "Enable standard SEV with type='std' or enable"
> > -	    ." experimental SEV-ES with the 'es' option.",
> > +	    ." experimental SEV-ES with the 'es' option or enable "
> > +	    ."experimental SEV-SNP with the 'snp' option.",
> 
> Style nit: it's preferred to use the space at the beginning of the next
> line: https://pve.proxmox.com/wiki/Perl_Style_Guide#Wrapping_Strings
> 
Ack.
> >  	type => 'string',
> >  	default_key => 1,
> >  	format_description => "sev-type",
> > -	enum => ['std', 'es'],
> > +	enum => ['std', 'es', 'snp'],
> >  	maxLength => 3,
> >      },
> >      'no-debug' => {
> > -	description => "Sets policy bit 0 to 1 to disallow debugging of guest",
> > +	description => "Sets policy bit to disallow debugging of guest",
> >  	type => 'boolean',
> >  	default => 0,
> >  	optional => 1,
> >      },
> >      'no-key-sharing' => {
> > -	description => "Sets policy bit 1 to 1 to disallow key sharing with other guests",
> > +	description => "Sets policy bit to disallow key sharing with other guests",
> 
> I'd also mention that it is ignored if using SNP
> 
Ack
> >  	type => 'boolean',
> >  	default => 0,
> >  	optional => 1,
> >      },
> > +    'allow-smt' => {
> > +	description => "Sets policy bit to allow Simultaneous Multi Threading (SMT)",
> 
> I'd also mention that it is ignored if not using SNP
> 
Ack
> > +	type => 'boolean',
> > +	default => 1,
> > +	optional => 1,
> > +    },
> >      "kernel-hashes" => {
> >  	description => "Add kernel hashes to guest firmware for measured linux kernel launch",
> >  	type => 'boolean',
> > @@ -823,6 +831,13 @@ sub get_hw_capabilities {
> >      }
> >      return $hw_capabilities;
> >  }
> > +sub get_amd_sev_type {
> > +    my ($conf) = @_;
> > +
> > +    return undef if !$conf->{'amd-sev'};
> > +
> > +    return PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{'amd-sev'})->{type};
> > +}
> >  
> >  sub get_amd_sev_object {
> >      my ($amd_sev, $bios) = @_;
> > @@ -836,22 +851,41 @@ sub get_amd_sev_object {
> >      if ($amd_sev_conf->{type} eq 'es' && !$sev_hw_caps->{'sev-support-es'}) {
> >  	die "Your CPU does not support AMD SEV-ES.\n";
> >      }
> > +    if ($amd_sev_conf->{type} eq 'snp' && !$sev_hw_caps->{'sev-support-snp'}) {
> > +	die "Your CPU does not support AMD SEV-SNP.\n";
> > +    }
> >      if (!$bios || $bios ne 'ovmf') {
> >  	die "To use AMD SEV, you need to change the BIOS to OVMF.\n";
> >      }
> >  
> > -    my $sev_mem_object = 'sev-guest,id=sev0';
> > -    $sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> > -    $sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> > -
> > -    # guest policy bit calculation as described here:
> > -    # https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
> > -    my $policy = 0;
> > -    $policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
> > -    $policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
> > -    $policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
> > -    # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> > -    $policy |= 1 << 3;
> > +    my $sev_mem_object = '';
> > +    my $policy;
> > +    if ($amd_sev_conf->{type} eq 'es' || $amd_sev_conf->{type} eq 'std') {
> > +	$sev_mem_object .= 'sev-guest,id=sev0';
> > +	$sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> > +	$sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> > +
> > +	# guest policy bit calculation as described here:
> > +	# https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
> > +	$policy = 0;
> > +	$policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
> > +	$policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
> > +	$policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
> > +	# disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> > +	$policy |= 1 << 3;
> > +    } elsif ($amd_sev_conf->{type} eq 'snp') {
> > +	$sev_mem_object .= 'sev-snp-guest,id=sev0';
> > +	$sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> > +	$sev_mem_object .= ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> > +
> > +	# guest policy bit calculation as described in chapter 4.3:
> > +	# https://www.amd.com/system/files/TechDocs/56860.pdf
> > +	# Reserved bit must be one
> > +	$policy = 1 << 17;
> > +	$policy |= 1 << 16 if $amd_sev_conf->{'allow-smt'};
> 
> In the schema, the default is set to 1. Note that this is only
> informational unfortunately, i.e. parse_property_string() will not fill
> in a default. You need to apply the default yourself here using
> 
> > if !defined($amd_sev_conf->{'allow-smt'}) || $amd_sev_conf->{'allow-smt'};
> 
Ack
> 
> > +	$policy |= 1 << 19 if !$amd_sev_conf->{'no-debug'};
> 
> Note that here, it already works automatically, because the default is 0.
> 
> > +    }
> > +
> >  
> >      $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
> >      $sev_mem_object .= ',kernel-hashes=on' if ($amd_sev_conf->{'kernel-hashes'});
> 
> 

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support.
  2025-03-11 13:56     ` Philipp Giersfeld
@ 2025-03-11 14:22       ` Fiona Ebner
  0 siblings, 0 replies; 15+ messages in thread
From: Fiona Ebner @ 2025-03-11 14:22 UTC (permalink / raw)
  To: Philipp Giersfeld; +Cc: Proxmox VE development discussion

Am 11.03.25 um 14:56 schrieb Philipp Giersfeld:
> On 25/03/05 04:35PM, Fiona Ebner wrote:
>> Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
>>> This patch is for enabling AMD SEV-SNP support.
>>>
>>> Where applicable, it extends support for existing SEV(-ES) variables
>>> to SEV-SNP. This means that it retains no-debug and kernel-hashes
>>> options, but the no-key-sharing option is removed.
>>>
>>> The default policy value is identical to QEMU’s, and the therefore
>>> required option has been added to configure SMT support.
>>>
>>> The code was tested by running a VM without SEV, with SEV, SEV-ES,
>>> SEV-SNP. Each configuration was tested with and without an EFI disk
>>> attached. For SEV-enabled configurations it was also verified that the
>>> kernel actually used the respective feature.
>>>
>>> Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
>>> Reviewed-by: Daniel Kral <d.kral@proxmox.com>
>>> Tested-by: Markus Frank <m.frank@proxmox.com>
>>> ---
>>>
>>>  no changes since last version
>>>
>>>  PVE/API2/Qemu.pm            |  7 +++-
>>>  PVE/QemuServer.pm           | 49 +++++++++++++++++++--------
>>>  PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
>>>  3 files changed, 92 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>>> index 295260e7..4f48cf85 100644
>>> --- a/PVE/API2/Qemu.pm
>>> +++ b/PVE/API2/Qemu.pm
>>> @@ -548,8 +548,13 @@ my sub create_disks : prototype($$$$$$$$$$$) {
>>>  		my $volid;
>>>  		if ($ds eq 'efidisk0') {
>>>  		    my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
>>> +
>>> +		    my $amd_sev_type = PVE::QemuServer::CPUConfig::get_amd_sev_type($conf);
>>> +		    die "SEV-SNP uses consolidated read-only firmware"
>>
>> Missing newline at the end of error message. Perl will automatically
>> attach the filename and line number then, which is rather ugly here. I'd
>> also mention "and doesn't need an EFI disk" to clarify this a bit more
>> for users.
> 
> Does "SEV-SNP uses consolidated read-only firmware instead of a seperate
> non-volatile variable store\n" work?

I'd rather mention the "EFI disk" directly. Like that it will be clear
to users, since they know the "EFI disk" from the UI/configuration.

> 
>>
>>> +			if $amd_sev_type && $amd_sev_type eq 'snp';
>>> +
>>>  		    ($volid, $size) = PVE::QemuServer::create_efidisk(
>>> -			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
>>> +			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm, $amd_sev_type);
>>>  		} elsif ($ds eq 'tpmstate0') {
>>>  		    # swtpm can only use raw volumes, and uses a fixed size
>>>  		    $size = PVE::Tools::convert_size(PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE, 'b' => 'kb');
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index 808c0e1c..51e551b8 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -52,7 +52,7 @@ use PVE::QemuConfig;
>>>  use PVE::QemuServer::Helpers qw(config_aware_timeout min_version kvm_user_version windows_version);
>>>  use PVE::QemuServer::Cloudinit;
>>>  use PVE::QemuServer::CGroup;
>>> -use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object);
>>> +use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object get_amd_sev_type);
>>>  use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
>>>  use PVE::QemuServer::Machine;
>>>  use PVE::QemuServer::Memory qw(get_current_memory);
>>> @@ -88,6 +88,13 @@ my $OVMF = {
>>>  	    "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd",
>>>  	    "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
>>>  	],
>>> +	'4m-sev' => [
>>> +	    "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd",
>>> +	    "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd", 
>>
>> Style nit: trailing whitespace above
>>
>> Is switching over for the std+ES SEV scenarios still
>> required/advantageous if we go for edk2-stable202502?
> 
> From my testing this is still the case.
> IIUC, this is because the existing firmware is built targeting
> OmvfPkg/OvmfPkgI32X64.dsc which does not support SEV.

Okay, thank you for testing!

Best Regards,
Fiona


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-03-11 14:23 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-24 12:37 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] AMD SEV-SNP Philipp Giersfeld
2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 1/5] Update edk2 to edkstable202411 Philipp Giersfeld
2025-03-05 16:51   ` [pve-devel] applied: " Thomas Lamprecht
2025-02-24 12:37 ` [pve-devel] [PATCH edk2-firmware v3 2/5] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
2025-03-05 14:18   ` Fiona Ebner
2025-03-05 15:35     ` Thomas Lamprecht
2025-03-11 12:31     ` Philipp Giersfeld
2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 3/5] Convert policy calculation Philipp Giersfeld
2025-03-05 15:35   ` Fiona Ebner
2025-02-24 12:37 ` [pve-devel] [PATCH qemu-server v3 4/5] config: add AMD SEV-SNP support Philipp Giersfeld
2025-03-05 15:35   ` Fiona Ebner
2025-03-11 13:56     ` Philipp Giersfeld
2025-03-11 14:22       ` Fiona Ebner
2025-02-24 12:37 ` [pve-devel] [PATCH pve-manager v3 5/5] Add configuration options for AMD SEV-SNP Philipp Giersfeld
2025-03-05  8:31 ` [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/5] " Philipp Giersfeld

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal