* [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP
@ 2025-02-07 8:51 Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH pve-edk2-firmware 1/4] Update edk2 to edkstable202411 Philipp Giersfeld
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Philipp Giersfeld @ 2025-02-07 8:51 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH pve-edk2-firmware 1/4] Update edk2 to edkstable202411
2025-02-07 8:51 [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP Philipp Giersfeld
@ 2025-02-07 8:51 ` Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH pve-edk2-firmware 2/4] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Philipp Giersfeld @ 2025-02-07 8:51 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH pve-edk2-firmware 2/4] Add OVMF targets for AMD SEV-ES and SEV-SNP
2025-02-07 8:51 [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH pve-edk2-firmware 1/4] Update edk2 to edkstable202411 Philipp Giersfeld
@ 2025-02-07 8:51 ` Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support Philipp Giersfeld
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Philipp Giersfeld @ 2025-02-07 8:51 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>
---
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] 8+ messages in thread
* [pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support.
2025-02-07 8:51 [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH pve-edk2-firmware 1/4] Update edk2 to edkstable202411 Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH pve-edk2-firmware 2/4] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
@ 2025-02-07 8:51 ` Philipp Giersfeld
2025-02-07 10:55 ` Daniel Kral
2025-02-07 8:51 ` [pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP Philipp Giersfeld
2025-02-13 15:08 ` [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] " Markus Frank
4 siblings, 1 reply; 8+ messages in thread
From: Philipp Giersfeld @ 2025-02-07 8:51 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.
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP
2025-02-07 8:51 [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP Philipp Giersfeld
` (2 preceding siblings ...)
2025-02-07 8:51 ` [pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support Philipp Giersfeld
@ 2025-02-07 8:51 ` Philipp Giersfeld
2025-02-07 10:56 ` Daniel Kral
2025-02-13 15:08 ` [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] " Markus Frank
4 siblings, 1 reply; 8+ messages in thread
From: Philipp Giersfeld @ 2025-02-07 8:51 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>
---
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support.
2025-02-07 8:51 ` [pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support Philipp Giersfeld
@ 2025-02-07 10:55 ` Daniel Kral
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Kral @ 2025-02-07 10:55 UTC (permalink / raw)
To: Proxmox VE development discussion, Philipp Giersfeld
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP
2025-02-07 8:51 ` [pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP Philipp Giersfeld
@ 2025-02-07 10:56 ` Daniel Kral
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Kral @ 2025-02-07 10:56 UTC (permalink / raw)
To: Proxmox VE development discussion, Philipp Giersfeld
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP
2025-02-07 8:51 [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP Philipp Giersfeld
` (3 preceding siblings ...)
2025-02-07 8:51 ` [pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP Philipp Giersfeld
@ 2025-02-13 15:08 ` Markus Frank
4 siblings, 0 replies; 8+ messages in thread
From: Markus Frank @ 2025-02-13 15:08 UTC (permalink / raw)
To: Proxmox VE development discussion, Philipp Giersfeld
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-02-13 15:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-07 8:51 [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] AMD SEV-SNP Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH pve-edk2-firmware 1/4] Update edk2 to edkstable202411 Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH pve-edk2-firmware 2/4] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
2025-02-07 8:51 ` [pve-devel] [PATCH qemu-server 3/4] config: add AMD SEV-SNP support Philipp Giersfeld
2025-02-07 10:55 ` Daniel Kral
2025-02-07 8:51 ` [pve-devel] [PATCH pve-manager 4/4] Add configuration options for AMD SEV-SNP Philipp Giersfeld
2025-02-07 10:56 ` Daniel Kral
2025-02-13 15:08 ` [pve-devel] [PATCH edk2-firmware/qemu-server/manager 0/4] " Markus Frank
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