Am 24.02.25 um 13:37 schrieb Philipp Giersfeld:
> This patch is for enabling AMD SEV-SNP support.
>
> Where applicable, it extends support for existing SEV(-ES) variables
> to SEV-SNP. This means that it retains no-debug and kernel-hashes
> options, but the no-key-sharing option is removed.
>
> The default policy value is identical to QEMU’s, and the therefore
> required option has been added to configure SMT support.
>
> The code was tested by running a VM without SEV, with SEV, SEV-ES,
> SEV-SNP. Each configuration was tested with and without an EFI disk
> attached. For SEV-enabled configurations it was also verified that the
> kernel actually used the respective feature.
>
> Signed-off-by: Philipp Giersfeld <philipp.giersf...@canarybit.eu>
> Reviewed-by: Daniel Kral <d.k...@proxmox.com>
> Tested-by: Markus Frank <m.fr...@proxmox.com>
> ---
>
> no changes since last version
>
> PVE/API2/Qemu.pm | 7 +++-
> PVE/QemuServer.pm | 49 +++++++++++++++++++--------
> PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
> 3 files changed, 92 insertions(+), 30 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 295260e7..4f48cf85 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -548,8 +548,13 @@ my sub create_disks : prototype($$$$$$$$$$$) {
> my $volid;
> if ($ds eq 'efidisk0') {
> my $smm =
> PVE::QemuServer::Machine::machine_type_is_q35($conf);
> +
> + my $amd_sev_type =
> PVE::QemuServer::CPUConfig::get_amd_sev_type($conf);
> + die "SEV-SNP uses consolidated read-only firmware"
Missing newline at the end of error message. Perl will automatically
attach the filename and line number then, which is rather ugly here. I'd
also mention "and doesn't need an EFI disk" to clarify this a bit more
for users.
> + if $amd_sev_type && $amd_sev_type eq 'snp';
> +
> ($volid, $size) = PVE::QemuServer::create_efidisk(
> - $storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm);
> + $storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm,
> $amd_sev_type);
> } elsif ($ds eq 'tpmstate0') {
> # swtpm can only use raw volumes, and uses a fixed size
> $size =
> PVE::Tools::convert_size(PVE::QemuServer::Drive::TPMSTATE_DISK_SIZE, 'b' =>
> 'kb');
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 808c0e1c..51e551b8 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -52,7 +52,7 @@ use PVE::QemuConfig;
> use PVE::QemuServer::Helpers qw(config_aware_timeout min_version
> kvm_user_version windows_version);
> use PVE::QemuServer::Cloudinit;
> use PVE::QemuServer::CGroup;
> -use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options
> get_cpu_bitness is_native_arch get_amd_sev_object);
> +use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options
> get_cpu_bitness is_native_arch get_amd_sev_object get_amd_sev_type);
> use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format
> drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
> use PVE::QemuServer::Machine;
> use PVE::QemuServer::Memory qw(get_current_memory);
> @@ -88,6 +88,13 @@ my $OVMF = {
> "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd",
> "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
> ],
> + '4m-sev' => [
> + "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd",
> + "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd",
Style nit: trailing whitespace above
Is switching over for the std+ES SEV scenarios still
required/advantageous if we go for edk2-stable202502?
> + ],
> + '4m-snp' => [
> + "$EDK2_FW_BASE/OVMF_CVM_4M.fd",
> + ],
> # FIXME: These are legacy 2MB-sized images that modern OVMF doesn't
> supports to build
> # anymore. how can we deperacate this sanely without breaking existing
> instances, or using
> # older backups and snapshot?
> @@ -3172,15 +3179,22 @@ sub vga_conf_has_spice {
> return $1 || 1;
> }
>
> -sub get_ovmf_files($$$) {
> - my ($arch, $efidisk, $smm) = @_;
> +sub get_ovmf_files($$$$) {
> + my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
>
> my $types = $OVMF->{$arch}
> or die "no OVMF images known for architecture '$arch'\n";
>
> my $type = 'default';
> if ($arch eq 'x86_64') {
> - if (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
> + if ($amd_sev_type && $amd_sev_type eq 'snp') {
> + $type = "4m-snp";
> + my ($ovmf) = $types->{$type}->@*;
> + die "EFI base image '$ovmf' not found\n" if ! -f $ovmf;
> + return ($ovmf);
> + } elsif ($amd_sev_type) {
> + $type = "4m-sev";
> + } elsif (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
> $type = $smm ? "4m" : "4m-no-smm";
> $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
> } else {
> @@ -3329,7 +3343,8 @@ my sub print_ovmf_drive_commandlines {
>
> my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) :
> undef;
>
> - my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35);
> + my $amd_sev_type = get_amd_sev_type($conf);
I'd die here if the type is 'snp' just to be sure, so that we have a
clear error should that ever happen by accident.
> + my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35,
> $amd_sev_type);
>
> my $var_drive_str = "if=pflash,unit=1,id=drive-efidisk0";
> if ($d) {
> @@ -3526,10 +3541,17 @@ sub config_to_command {
> die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n"
> if !$forcecpu && get_cpu_bitness($conf->{cpu}, $arch) == 32;
>
> - my ($code_drive_str, $var_drive_str) =
> - print_ovmf_drive_commandlines($conf, $storecfg, $vmid, $arch, $q35,
> $version_guard);
> - push $cmd->@*, '-drive', $code_drive_str;
> - push $cmd->@*, '-drive', $var_drive_str;
> + my $amd_sev_type = get_amd_sev_type($conf);
> + if ($amd_sev_type && $amd_sev_type eq 'snp') {
> + my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
> + my $d = $conf->{efidisk0} ? parse_drive('efidisk0',
> $conf->{efidisk0}) : undef;
Since the EFI disk is not used in this case, we should always pass undef
to be explicit. We can print an informational message that it will be
ignored if present.
> + push $cmd->@*, '-bios', get_ovmf_files($arch, $d, undef,
> $amd_sev_type);
> + } else {
> + my ($code_drive_str, $var_drive_str) =
> print_ovmf_drive_commandlines(
> + $conf, $storecfg, $vmid, $arch, $q35, $version_guard);
> + push $cmd->@*, '-drive', $code_drive_str;
> + push $cmd->@*, '-drive', $var_drive_str;
> + }
> }
>
> if ($q35) { # tell QEMU to load q35 config early
---snip 8<---
> @@ -231,25 +232,32 @@ my $cpu_fmt = {
> my $sev_fmt = {
> type => {
> description => "Enable standard SEV with type='std' or enable"
> - ." experimental SEV-ES with the 'es' option.",
> + ." experimental SEV-ES with the 'es' option or enable "
> + ."experimental SEV-SNP with the 'snp' option.",
Style nit: it's preferred to use the space at the beginning of the next
line: https://pve.proxmox.com/wiki/Perl_Style_Guide#Wrapping_Strings
> type => 'string',
> default_key => 1,
> format_description => "sev-type",
> - enum => ['std', 'es'],
> + enum => ['std', 'es', 'snp'],
> maxLength => 3,
> },
> 'no-debug' => {
> - description => "Sets policy bit 0 to 1 to disallow debugging of guest",
> + description => "Sets policy bit to disallow debugging of guest",
> type => 'boolean',
> default => 0,
> optional => 1,
> },
> 'no-key-sharing' => {
> - description => "Sets policy bit 1 to 1 to disallow key sharing with
> other guests",
> + description => "Sets policy bit to disallow key sharing with other
> guests",
I'd also mention that it is ignored if using SNP
> type => 'boolean',
> default => 0,
> optional => 1,
> },
> + 'allow-smt' => {
> + description => "Sets policy bit to allow Simultaneous Multi Threading
> (SMT)",
I'd also mention that it is ignored if not using SNP
> + type => 'boolean',
> + default => 1,
> + optional => 1,
> + },
> "kernel-hashes" => {
> description => "Add kernel hashes to guest firmware for measured linux
> kernel launch",
> type => 'boolean',
> @@ -823,6 +831,13 @@ sub get_hw_capabilities {
> }
> return $hw_capabilities;
> }
> +sub get_amd_sev_type {
> + my ($conf) = @_;
> +
> + return undef if !$conf->{'amd-sev'};
> +
> + return PVE::JSONSchema::parse_property_string($sev_fmt,
> $conf->{'amd-sev'})->{type};
> +}
>
> sub get_amd_sev_object {
> my ($amd_sev, $bios) = @_;
> @@ -836,22 +851,41 @@ sub get_amd_sev_object {
> if ($amd_sev_conf->{type} eq 'es' && !$sev_hw_caps->{'sev-support-es'}) {
> die "Your CPU does not support AMD SEV-ES.\n";
> }
> + if ($amd_sev_conf->{type} eq 'snp' &&
> !$sev_hw_caps->{'sev-support-snp'}) {
> + die "Your CPU does not support AMD SEV-SNP.\n";
> + }
> if (!$bios || $bios ne 'ovmf') {
> die "To use AMD SEV, you need to change the BIOS to OVMF.\n";
> }
>
> - my $sev_mem_object = 'sev-guest,id=sev0';
> - $sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> - $sev_mem_object .=
> ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> -
> - # guest policy bit calculation as described here:
> - #
> https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
> - my $policy = 0;
> - $policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
> - $policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
> - $policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
> - # disable migration with bit 3 nosend to prevent amd-sev-migration-attack
> - $policy |= 1 << 3;
> + my $sev_mem_object = '';
> + my $policy;
> + if ($amd_sev_conf->{type} eq 'es' || $amd_sev_conf->{type} eq 'std') {
> + $sev_mem_object .= 'sev-guest,id=sev0';
> + $sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> + $sev_mem_object .=
> ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> +
> + # guest policy bit calculation as described here:
> + #
> https://documentation.suse.com/sles/15-SP5/html/SLES-amd-sev/article-amd-sev.html#table-guestpolicy
> + $policy = 0;
> + $policy |= 1 << 0 if $amd_sev_conf->{'no-debug'};
> + $policy |= 1 << 1 if $amd_sev_conf->{'no-key-sharing'};
> + $policy |= 1 << 2 if $amd_sev_conf->{type} eq 'es';
> + # disable migration with bit 3 nosend to prevent
> amd-sev-migration-attack
> + $policy |= 1 << 3;
> + } elsif ($amd_sev_conf->{type} eq 'snp') {
> + $sev_mem_object .= 'sev-snp-guest,id=sev0';
> + $sev_mem_object .= ',cbitpos='.$sev_hw_caps->{cbitpos};
> + $sev_mem_object .=
> ',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};
> +
> + # guest policy bit calculation as described in chapter 4.3:
> + # https://www.amd.com/system/files/TechDocs/56860.pdf
> + # Reserved bit must be one
> + $policy = 1 << 17;
> + $policy |= 1 << 16 if $amd_sev_conf->{'allow-smt'};
In the schema, the default is set to 1. Note that this is only
informational unfortunately, i.e. parse_property_string() will not fill
in a default. You need to apply the default yourself here using
> if !defined($amd_sev_conf->{'allow-smt'}) || $amd_sev_conf->{'allow-smt'};
> + $policy |= 1 << 19 if !$amd_sev_conf->{'no-debug'};
Note that here, it already works automatically, because the default is 0.
> + }
> +
>
> $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
> $sev_mem_object .= ',kernel-hashes=on' if
> ($amd_sev_conf->{'kernel-hashes'});
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
