all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Markus Frank <m.frank@proxmox.com>
To: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>,
	pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH qemu-server v4 3/4] config: add AMD SEV-SNP support.
Date: Tue, 18 Mar 2025 10:41:11 +0100	[thread overview]
Message-ID: <648bdcf5-3742-4e0f-898e-da11023c08c6@proxmox.com> (raw)
In-Reply-To: <20250311150456.4071232-4-philipp.giersfeld@canarybit.eu>


On  2025-03-11 16:04, 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 means that it retains no-debug and kernel-hashes
> options, but the no-key-sharing option is removed.
> 
> The default policy value is identical to QEMU’s, and the therefore
> required option has been added to configure SMT support.
> 
> The code was tested by running a VM without SEV, with SEV, SEV-ES,
> SEV-SNP. Each configuration was tested with and without an EFI disk
> attached. For SEV-enabled configurations it was also verified that the
> kernel actually used the respective feature.
> 
> Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
> Reviewed-by: Daniel Kral <d.kral@proxmox.com>
> Tested-by: Markus Frank <m.frank@proxmox.com>
I have not tested this version before (there have been some changes since v3/v2).
But now I have. So the "Tested-by" can stay there.
> ---
> 
>   changes since v3: https://lists.proxmox.com/pipermail/pve-devel/2025-February/068578.html
>   * Add and clarify logging statements and descriptions
>   * Add additional safeguards
> 
>   PVE/API2/Qemu.pm            |  7 +++-
>   PVE/QemuServer.pm           | 52 +++++++++++++++++++++--------
>   PVE/QemuServer/CPUConfig.pm | 66 ++++++++++++++++++++++++++++---------
>   3 files changed, 95 insertions(+), 30 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 295260e7..ef3aa8d6 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 and does not require an EFI disk\n"
> +			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..727aad6c 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -52,7 +52,7 @@ use PVE::QemuConfig;
>   use PVE::QemuServer::Helpers qw(config_aware_timeout min_version kvm_user_version windows_version);
>   use PVE::QemuServer::Cloudinit;
>   use PVE::QemuServer::CGroup;
> -use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object);
> +use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object get_amd_sev_type);
>   use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
>   use PVE::QemuServer::Machine;
>   use PVE::QemuServer::Memory qw(get_current_memory);
> @@ -88,6 +88,13 @@ my $OVMF = {
>   	    "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd",
>   	    "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
>   	],
> +	'4m-sev' => [
> +	    "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd",
> +	    "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd",
> +	],
> +	'4m-snp' => [
> +	    "$EDK2_FW_BASE/OVMF_CVM_4M.fd",
> +	],
>   	# FIXME: These are legacy 2MB-sized images that modern OVMF doesn't supports to build
>   	# anymore. how can we deperacate this sanely without breaking existing instances, or using
>   	# older backups and snapshot?
> @@ -3172,15 +3179,22 @@ sub vga_conf_has_spice {
>       return $1 || 1;
>   }
>   
> -sub get_ovmf_files($$$) {
> -    my ($arch, $efidisk, $smm) = @_;
> +sub get_ovmf_files($$$$) {
> +    my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
>   
>       my $types = $OVMF->{$arch}
>   	or die "no OVMF images known for architecture '$arch'\n";
>   
>       my $type = 'default';
>       if ($arch eq 'x86_64') {
> -	if (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
> +	if ($amd_sev_type && $amd_sev_type eq 'snp') {
> +	    $type = "4m-snp";
> +	    my ($ovmf) = $types->{$type}->@*;
> +	    die "EFI base image '$ovmf' not found\n" if ! -f $ovmf;
> +	    return ($ovmf);
> +	} elsif ($amd_sev_type) {
> +	    $type = "4m-sev";
> +	} elsif (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
>   	    $type = $smm ? "4m" : "4m-no-smm";
>   	    $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
>   	} else {
> @@ -3329,7 +3343,10 @@ 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);
> +    die "Attempting to configure SEV-SNP with flash devices instead of using `-bios`\n"
> +	if $amd_sev_type && $amd_sev_type eq 'snp';
> +    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,18 @@ 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);
> +	   print "Existing EFI disk will be ignored for SEV-SNP\n"
> +		if parse_drive('efidisk0', $conf->{efidisk0});
> +	   push $cmd->@*, '-bios', get_ovmf_files($arch, undef, 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 +8362,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 +8387,10 @@ sub update_tpmstate_size {
>       $conf->{tpmstate0} = print_drive($disk);
>   }
>   
> -sub create_efidisk($$$$$$$) {
> -    my ($storecfg, $storeid, $vmid, $fmt, $arch, $efidisk, $smm) = @_;
> +sub create_efidisk($$$$$$$$) {
> +    my ($storecfg, $storeid, $vmid, $fmt, $arch, $efidisk, $smm, $amd_sev_type) = @_;
>   
> -    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm);
> +    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm, $amd_sev_type);
>   
>       my $vars_size_b = -s $ovmf_vars;
>       my $vars_size = PVE::Tools::convert_size($vars_size_b, 'b' => 'kb');
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index ad0be16e..9ae1996b 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 (Ignored for SEV-SNP)",
>   	type => 'boolean',
>   	default => 0,
>   	optional => 1,
>       },
> +    'allow-smt' => {
> +	description => "Sets policy bit to allow Simultaneous Multi Threading (SMT) (Ignored unless for SEV-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 !defined($amd_sev_conf->{'allow-smt'}) || $amd_sev_conf->{'allow-smt'};
> +	$policy |= 1 << 19 if !$amd_sev_conf->{'no-debug'};
> +    }
> +
>   
>       $sev_mem_object .= ',policy='.sprintf("%#x", $policy);
>       $sev_mem_object .= ',kernel-hashes=on' if ($amd_sev_conf->{'kernel-hashes'});



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

  reply	other threads:[~2025-03-18  9:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-11 15:04 [pve-devel] [PATCH edk2-firmware/qemu-server/manager v3 0/4] AMD SEV-SNP Philipp Giersfeld
2025-03-11 15:04 ` [pve-devel] [PATCH edk2-firmware v4 1/4] Add OVMF targets for AMD SEV-ES and SEV-SNP Philipp Giersfeld
2025-03-18  9:40   ` Markus Frank
2025-03-11 15:04 ` [pve-devel] [PATCH qemu-server v4 2/4] Convert policy calculation Philipp Giersfeld
2025-03-11 15:04 ` [pve-devel] [PATCH qemu-server v4 3/4] config: add AMD SEV-SNP support Philipp Giersfeld
2025-03-18  9:41   ` Markus Frank [this message]
2025-03-11 15:04 ` [pve-devel] [PATCH pve-manager v4 4/4] Add configuration options for AMD SEV-SNP Philipp Giersfeld
2025-03-17 10:13   ` Markus Frank

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=648bdcf5-3742-4e0f-898e-da11023c08c6@proxmox.com \
    --to=m.frank@proxmox.com \
    --cc=philipp.giersfeld@canarybit.eu \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal