[pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP

[pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP

[Philipp Giersfeld]

This patch series adds support for AMD SEV-SNP. 
Where possible it maintains 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.



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

[pve-devel] [PATCH pve-edk2-firmware 1/4] Update edk2 to edkstable202411

[Philipp Giersfeld]

Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
---
 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

[pve-devel] [PATCH pve-edk2-firmware 2/4] Add OVMF targets for AMD SEV-ES and SEV-SNP

[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

Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
---
 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

[pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support.

[Philipp Giersfeld]

This patch is for enabling AMD SEV-SNP support.

Where applicable, it extends support for existing SEV(-ES) variables
to SEV-SNP.

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>
---
 PVE/API2/Qemu.pm            |  9 +++--
 PVE/QemuServer.pm           | 57 +++++++++++++++++++++++---------
 PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
 3 files changed, 99 insertions(+), 33 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 295260e7..35c76dc2 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);
-		    ($volid, $size) = PVE::QemuServer::create_efidisk(
-			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
+			my $amd_sev_type = PVE::QemuServer::CPUConfig::get_amd_sev_type($conf);
+			if($amd_sev_type && $amd_sev_type eq 'snp') {
+				die "SEV-SNP only uses consolidated read-only firmware";
+			} else {
+			    ($volid, $size) = PVE::QemuServer::create_efidisk(
+				$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..e46b3434 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,19 +3179,28 @@ 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') {
-	    $type = $smm ? "4m" : "4m-no-smm";
-	    $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
+	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"; 
 	} else {
-	    # TODO: log_warn about use of legacy images for x86_64 with Promxox VE 9
+	    if (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
+	        $type = $smm ? "4m" : "4m-no-smm";
+	        $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
+	    } else {
+	        # TODO: log_warn about use of legacy images for x86_64 with Promxox VE 9
+	    }
 	}
     }
 
@@ -3329,7 +3345,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 +3543,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 +8361,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 +8386,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 e65d8c26..dbc31462 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 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,16 @@ sub get_hw_capabilities {
     }
     return $hw_capabilities;
 }
+sub get_amd_sev_type {
+    my ($conf) = @_;
+
+    if ($conf->{'amd-sev'}) {
+        return PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{'amd-sev'})->{type};
+    }
+    else {
+        return undef;
+    }
+}
 
 sub get_amd_sev_object {
     my ($amd_sev, $bios) = @_;
@@ -836,22 +854,40 @@ 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'};
+    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 = 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';
+    	# disable migration with bit 3 nosend to prevent amd-sev-migration-attack
+    	$policy += 0b1000;
+    } 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 here:
+    	# https://libvirt.org/formatdomain.html#id121
+    	$policy = 0x20000;
+    	$policy += 0x80000 if $amd_sev_conf->{'no-debug'};
+    	$policy += 0x10000 if $amd_sev_conf->{'allow-smt'};
+    }
 
-    # 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';
-    # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
-    $policy += 0b1000;
 
     $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

[pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP

[Philipp Giersfeld]

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>
---
 www/manager6/qemu/Options.js |  1 +
 www/manager6/qemu/SevEdit.js | 40 ++++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 6 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..9605cf59 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',
+			snpEnabled: get => get('type') === 'snp',
 	},
     },
 
@@ -21,10 +22,14 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	if (!values.debug) {
 	    values["no-debug"] = 1;
 	}
+	if (values.smt) {
+		values["allow-smt"] = 1;
+	}
 	if (!values["key-sharing"]) {
 	    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,20 @@ 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 variable store is loaded as pflash.'),
+	bind: {
+	    hidden: '{!snpEnabled}',
+	},
+    }],
 
     advancedItems: [
 	{
@@ -66,8 +83,8 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	    name: 'debug',
 	    value: 1,
 	    bind: {
-		hidden: '{!sevEnabled}',
-		disabled: '{!sevEnabled}',
+		hidden: '{!sevEnabled && !snpEnabled}',
+		disabled: '{!sevEnabled && !snpEnabled}',
 	    },
 	},
 	{
@@ -81,6 +98,17 @@ Ext.define('PVE.qemu.SevInputPanel', {
 		disabled: '{!sevEnabled}',
 	    },
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Allow SMT'),
+	    labelWidth: 150,
+	    name: 'smt',
+	    value: 1,
+	    bind: {
+		hidden: '{!snpEnabled}',
+		disabled: '{!snpEnabled}',
+	    },
+	},
 	{
 	    xtype: 'proxmoxcheckbox',
 	    fieldLabel: gettext('Enable Kernel Hashes'),
@@ -88,8 +116,8 @@ Ext.define('PVE.qemu.SevInputPanel', {
 	    name: 'kernel-hashes',
 	    deleteDefaultValue: false,
 	    bind: {
-		hidden: '{!sevEnabled}',
-		disabled: '{!sevEnabled}',
+		hidden: '{!sevEnabled && !snpEnabled}',
+		disabled: '{!sevEnabled && !snpEnabled}',
 	    },
 	},
     ],
-- 
2.39.5


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

Re: [pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support.

[Daniel Kral]

Hi,

thanks for wanting to contribute and sending a patch for AMD SEV-SNP! We 
really appreciate it!

Unfortunately I don't have the hardware to test this and neither have 
enough knowledge about the specific details for this, but I added some 
higher-level inlined comments as I wanted to take a closer look at this 
anyway. Hope those help the process.

On another note, if you haven't already, please make sure that you have 
a signed CLA sent to office@proxmox.com (see 
https://pve.proxmox.com/wiki/Developer_Documentation for more information).

All else, I'd be happy to see this applied!

     Daniel

On 2/7/25 09:51, Philipp Giersfeld wrote:
> This patch is for enabling AMD SEV-SNP support.
> 
> Where applicable, it extends support for existing SEV(-ES) variables
> to SEV-SNP.

This could be more specific and tell which parameters are not applicable 
to SEV-SNP anymore (i.e. no-key-sharing).

> 
> 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>
> ---
>   PVE/API2/Qemu.pm            |  9 +++--
>   PVE/QemuServer.pm           | 57 +++++++++++++++++++++++---------
>   PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
>   3 files changed, 99 insertions(+), 33 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 295260e7..35c76dc2 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);
> -		    ($volid, $size) = PVE::QemuServer::create_efidisk(
> -			$storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
> +			my $amd_sev_type = PVE::QemuServer::CPUConfig::get_amd_sev_type($conf);
> +			if($amd_sev_type && $amd_sev_type eq 'snp') {
> +				die "SEV-SNP only uses consolidated read-only firmware";

it could be just my reader, but it seems like there are three tabs 
instead of two tabs + 4 spaces to align it with my $smm.

also nit: this special case could be handled with a die post-if, i.e.

```
die "SEV-SNP only uses consolidated read-only firmware"
	if $amd_sev_type && $amd_sev_type eq 'snp';
```

as there's plenty of indentation in this subroutine already.

> +			} else {
> +			    ($volid, $size) = PVE::QemuServer::create_efidisk(
> +				$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..e46b3434 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,19 +3179,28 @@ 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') {
> -	    $type = $smm ? "4m" : "4m-no-smm";
> -	    $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
> +	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";
>   	} else {
> -	    # TODO: log_warn about use of legacy images for x86_64 with Promxox VE 9
> +	    if (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
> +	        $type = $smm ? "4m" : "4m-no-smm";
> +	        $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
> +	    } else {
> +	        # TODO: log_warn about use of legacy images for x86_64 with Promxox VE 9
> +	    }

nit: this branch could also be folded into an elsif

>   	}
>       }
>   
> @@ -3329,7 +3345,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 +3543,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 +8361,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 +8386,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 e65d8c26..dbc31462 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.",

`'snp'` instead of `'snp`

>   	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",

Good catch to also adapt these!

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

This would certainly benefit for a clarification that SMT = Simultaneous 
Multi Threading.

> +	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,16 @@ sub get_hw_capabilities {
>       }
>       return $hw_capabilities;
>   }
> +sub get_amd_sev_type {
> +    my ($conf) = @_;
> +
> +    if ($conf->{'amd-sev'}) {
> +        return PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{'amd-sev'})->{type};
> +    }
> +    else {
> +        return undef;
> +    }

nit: could be a `return undef if !$conf->{'amd-sev'};`
> +}
>   
>   sub get_amd_sev_object {
>       my ($amd_sev, $bios) = @_;
> @@ -836,22 +854,40 @@ 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'};
> +    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 = 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';
> +    	# disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> +    	$policy += 0b1000;

Since the AMD SEV-SNP code below uses hex numbers since they use the 
higher number bits 15, 16 and 19, I think it would be more consistent to 
convert the above binary numbers (in its own patch before this!) to either

1. hexadecimal numbers too,
2. or maybe even more readable: use shift operators for all of these, as 
the official manuals specify them in bits. Also ORing those is more 
standard for me than adding binary numbers, but that's just a nit, e.g.

```
$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 here:
> +    	# https://libvirt.org/formatdomain.html#id121

This should be https://libvirt.org/formatdomain.html#launch-security, 
and nit: could also directly point to the official AMD specification at 
https://www.amd.com/system/files/TechDocs/56860.pdf and then chapter 4.3 
Guest Policy settings

> +    	$policy = 0x20000;
> +    	$policy += 0x80000 if $amd_sev_conf->{'no-debug'};

I think this is wrong, as the AMD SEV-SNP specification in 4.3 Guest 
Policy states:

Bit 19 (Debug): 0 - Debugging is disallowed, 1 - Debugging is allowed

This is probably an oversight, since the AMD **SEV** spec states the 
opposite for the debug bit 0.

> +    	$policy += 0x10000 if $amd_sev_conf->{'allow-smt'};

And if you consider the shift operation + ORing then the above becomes:

```
# 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'};
```

> +    }
>   
> -    # 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';
> -    # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> -    $policy += 0b1000;
>   
>       $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
>       $sev_mem_object .= ',kernel-hashes=on' if ($amd_sev_conf->{'kernel-hashes'});

With those comments fixed, consider this:

Reviewed-by: Daniel Kral <d.kral@proxmox.com>


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

Re: [pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP

[Daniel Kral]

On 2/7/25 09:51, Philipp Giersfeld wrote:
> 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>
> ---
>   www/manager6/qemu/Options.js |  1 +
>   www/manager6/qemu/SevEdit.js | 40 ++++++++++++++++++++++++++++++------
>   2 files changed, 35 insertions(+), 6 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..9605cf59 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',

Looks like most changes here and below use tabs instead of 4 spaces. See 
[0] for more information on the indentation for our JavaScript projects.

Also relevant here: I think it'd be better to add all three SEV types to 
`sevEnabled` here, so we do not need to modify any `!sevEnabled` to 
`!sevEnabled && !snpEnabled` below.
The `snpEnabled` can stay as it is here, as it's relevant for the pflash 
warning and the "Allow SMT" checkbox ofc.

> +			snpEnabled: get => get('type') === 'snp',
>   	},
>       },
>   
> @@ -21,10 +22,14 @@ Ext.define('PVE.qemu.SevInputPanel', {
>   	if (!values.debug) {
>   	    values["no-debug"] = 1;
>   	}
> +	if (values.smt) {
> +		values["allow-smt"] = 1;
> +	}
>   	if (!values["key-sharing"]) {
>   	    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,20 @@ 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 variable store is loaded as pflash.'),
> +	bind: {
> +	    hidden: '{!snpEnabled}',
> +	},
> +    }],
>   
>       advancedItems: [
>   	{
> @@ -66,8 +83,8 @@ Ext.define('PVE.qemu.SevInputPanel', {
>   	    name: 'debug',
>   	    value: 1,
>   	    bind: {
> -		hidden: '{!sevEnabled}',
> -		disabled: '{!sevEnabled}',
> +		hidden: '{!sevEnabled && !snpEnabled}',
> +		disabled: '{!sevEnabled && !snpEnabled}',
>   	    },
>   	},
>   	{
> @@ -81,6 +98,17 @@ Ext.define('PVE.qemu.SevInputPanel', {
>   		disabled: '{!sevEnabled}',

The "Allow Key-Sharing" option should be disabled AFAICS for SEV-SNP 
being set, no?

>   	    },
>   	},
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    fieldLabel: gettext('Allow SMT'),
> +	    labelWidth: 150,
> +	    name: 'smt',
> +	    value: 1,
> +	    bind: {
> +		hidden: '{!snpEnabled}',
> +		disabled: '{!snpEnabled}',
> +	    },
> +	},
>   	{
>   	    xtype: 'proxmoxcheckbox',
>   	    fieldLabel: gettext('Enable Kernel Hashes'),
> @@ -88,8 +116,8 @@ Ext.define('PVE.qemu.SevInputPanel', {
>   	    name: 'kernel-hashes',
>   	    deleteDefaultValue: false,
>   	    bind: {
> -		hidden: '{!sevEnabled}',
> -		disabled: '{!sevEnabled}',
> +		hidden: '{!sevEnabled && !snpEnabled}',
> +		disabled: '{!sevEnabled && !snpEnabled}',
>   	    },
>   	},
>       ],

[0] https://pve.proxmox.com/wiki/Javascript_Style_Guide#Indentation

With those comments fixed, consider this:

Reviewed-by: Daniel Kral <d.kral@proxmox.com>


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

Re: [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP

[Markus Frank]

Thanks.

I have tested AMD SEV SNP and it works fine with your patch series.

Tested-by: Markus Frank <m.frank@proxmox.com>

One thing I noticed:
I would add a note in the WebUI that you need kernel >= 6.11 installed on the pve host to enable SEV-SNP as long as >=6.11 is not the default kernel.

On  2025-02-07 09:51, Philipp Giersfeld wrote:
> This patch series adds support for AMD SEV-SNP.
> Where possible it maintains 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.
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 



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