[pve-devel] [PATCH qemu-server/docs/manager v11 0/5] AMD SEV

[pve-devel] [PATCH qemu-server/docs/manager v11 0/5] AMD SEV

[Markus Frank]

Patch series to enable AMD Secure Encrypted Virtualization (SEV)
https://www.amd.com/en/developer/sev.html

changes v11:
* removed systemd service and added run_command in qemu-server instead
* moved SEV related code to CPUConfig
* instead of the no-debug and no-key-sharing checkboxes there are no 
 "allow debug/key-sharing" checkboxes in the WebUI with true beeing 
 the default.
* add check_non_migratable_resources function in qemu-server

changes v10:
* removed include of sys/types.h in C Program
* also die if the BIOS is not set, since the default is SeaBIOS
* added pve-manager patch

apply/compile order:
1. qemu-server: add C program to get hardware capabilities from CPUID
2. qemu-server: config: add AMD SEV support
3. qemu-server: migration: add check_non_migratable_resources function
4. pve-docs: add AMD SEV documentation
5. pve-manager: ui: add AMD SEV configuration to Options


qemu-server:

Markus Frank (3):
  add C program to get hardware capabilities from CPUID
  config: add AMD SEV support
  migration: add check_non_migratable_resources function

 Makefile                                      |  1 +
 PVE/API2/Qemu.pm                              |  5 +-
 PVE/QemuMigrate.pm                            |  2 +-
 PVE/QemuServer.pm                             | 36 +++++++-
 PVE/QemuServer/CPUConfig.pm                   | 83 ++++++++++++++++++-
 query-machine-capabilities/Makefile           | 18 ++++
 .../query-machine-capabilities.c              | 79 ++++++++++++++++++
 7 files changed, 219 insertions(+), 5 deletions(-)
 create mode 100644 query-machine-capabilities/Makefile
 create mode 100644 query-machine-capabilities/query-machine-capabilities.c


docs:

Markus Frank (1):
  add AMD SEV documentation

 qm.adoc | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)


manager:

Markus Frank (1):
  ui: add AMD SEV configuration to Options

 www/manager6/Makefile        |   1 +
 www/manager6/qemu/Options.js |  11 ++++
 www/manager6/qemu/SevEdit.js | 121 +++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 www/manager6/qemu/SevEdit.js

-- 
2.39.2



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

[pve-devel] [PATCH qemu-server v11 1/5] add C program to get hardware capabilities from CPUID

[Markus Frank]

Implement a C program that extracts AMD SEV hardware information such
as reduced-phys-bios and cbitpos from CPUID at boot time, looks if
SEV, SEV-ES & SEV-SNP are enabled, and outputs these details as JSON
to /run/qemu-server/host-hw-capabilities.json

This programm can also be used to read and save other hardware
information at boot time.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Tested-by: Filip Schauer <f.schauer@proxmox.com>
---
changes v11:
* removed systemd service

 Makefile                                      |  1 +
 query-machine-capabilities/Makefile           | 18 +++++
 .../query-machine-capabilities.c              | 79 +++++++++++++++++++
 3 files changed, 98 insertions(+)
 create mode 100644 query-machine-capabilities/Makefile
 create mode 100644 query-machine-capabilities/query-machine-capabilities.c

diff --git a/Makefile b/Makefile
index 133468d..ed67fe0 100644
--- a/Makefile
+++ b/Makefile
@@ -65,6 +65,7 @@ install: $(PKGSOURCES)
 	install -m 0644 -D bootsplash.jpg $(DESTDIR)/usr/share/$(PACKAGE)
 	$(MAKE) -C PVE install
 	$(MAKE) -C qmeventd install
+	$(MAKE) -C query-machine-capabilities install
 	$(MAKE) -C qemu-configs install
 	$(MAKE) -C vm-network-scripts install
 	install -m 0755 qm $(DESTDIR)$(SBINDIR)
diff --git a/query-machine-capabilities/Makefile b/query-machine-capabilities/Makefile
new file mode 100644
index 0000000..469c69f
--- /dev/null
+++ b/query-machine-capabilities/Makefile
@@ -0,0 +1,18 @@
+DESTDIR=
+PREFIX=/usr
+BINDIR=${PREFIX}/libexec/qemu-server
+
+CC ?= gcc
+CFLAGS += -O2 -fanalyzer -Werror -Wall -Wextra -Wpedantic -Wtype-limits -Wl,-z,relro -std=gnu11
+
+query-machine-capabilities: query-machine-capabilities.c
+	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS)
+
+.PHONY: install
+install: query-machine-capabilities
+	install -d ${DESTDIR}/${BINDIR}
+	install -m 0755 query-machine-capabilities ${DESTDIR}${BINDIR}
+
+.PHONY: clean
+clean:
+	rm -f query-machine-capabilities
diff --git a/query-machine-capabilities/query-machine-capabilities.c b/query-machine-capabilities/query-machine-capabilities.c
new file mode 100644
index 0000000..7640e38
--- /dev/null
+++ b/query-machine-capabilities/query-machine-capabilities.c
@@ -0,0 +1,79 @@
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <string.h>
+
+int main() {
+    uint32_t eax, ebx, ecx, edx;
+
+    // query Encrypted Memory Capabilities, see:
+    // https://en.wikipedia.org/wiki/CPUID#EAX=8000001Fh:_Encrypted_Memory_Capabilities
+    uint32_t query_function = 0x8000001F;
+    asm volatile("cpuid"
+	 : "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx)
+	 : "0"(query_function)
+    );
+
+    bool sev_support = (eax & (1<<1)) != 0;
+    bool sev_es_support = (eax & (1<<3)) != 0;
+    bool sev_snp_support = (eax & (1<<4)) != 0;
+
+    uint8_t cbitpos = ebx & 0x3f;
+    uint8_t reduced_phys_bits = (ebx >> 6) & 0x3f;
+
+    const char *path = "/run/qemu-server/";
+    // Check that the directory exists and create it if it does not.
+    struct stat statbuf;
+    int ret = stat(path, &statbuf);
+    if (ret == 0) {
+	if (!S_ISDIR(statbuf.st_mode)) {
+	    printf("Path %s is not a directory.\n", path);
+	    return 1;
+	}
+    } else if (errno == ENOENT) {
+	if (mkdir(path, 0755) != 0) {
+	    printf("Error creating directory %s: %s\n", path, strerror(errno));
+	    return 1;
+	}
+    } else {
+	printf("Error checking path %s: %s\n", path, strerror(errno));
+	return 1;
+    }
+
+    FILE *file;
+    const char *filename = "/run/qemu-server/host-hw-capabilities.json";
+    file = fopen(filename, "w");
+    if (file == NULL) {
+	perror("Error opening file");
+	return 1;
+    }
+
+    ret = fprintf(file,
+	"{"
+	" \"amd-sev\": {"
+	" \"cbitpos\": %u,"
+	" \"reduced-phys-bits\": %u,"
+	" \"sev-support\": %s,"
+	" \"sev-support-es\": %s,"
+	" \"sev-support-snp\": %s"
+	" }"
+	" }\n",
+	cbitpos,
+	reduced_phys_bits,
+	sev_support ? "true" : "false",
+	sev_es_support ? "true" : "false",
+	sev_snp_support ? "true" : "false"
+    );
+    if (ret == -1) {
+	printf("Error writing to file %s: %s\n", path, strerror(errno));
+    }
+
+    ret = fclose(file);
+    if (ret == -1) {
+	printf("Error closing file %s: %s\n", path, strerror(errno));
+    }
+
+    return 0;
+}
-- 
2.39.2



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

[pve-devel] [PATCH qemu-server v11 2/5] config: add AMD SEV support

[Markus Frank]

This patch is for enabling AMD SEV (Secure Encrypted Virtualization)
support in QEMU.

VM-Config-Examples:
amd_sev: type=std,no-debug=1,no-key-sharing=1
amd_sev: es,no-debug=1,kernel-hashes=1

kernel-hashes, reduced-phys-bits & cbitpos correspond to the variables
with the same name in QEMU.

kernel-hashes=1 adds kernel hashes to enable measured linux kernel
launch since it is per default off for backward compatibility.

reduced-phys-bios and cbitpos are system specific and are read out by
the query-machine-capabilities c program and saved to the
/run/qemu-server/host-hw-capabilities.json file. This file is parsed
and than used by qemu-server to correctly start a AMD SEV VM.

type=std stands for standard sev to differentiate it from sev-es (es)
or sev-snp (snp) when support is upstream.

QEMU's sev-guest policy gets calculated with the parameters no-debug
& no-key-sharing. These parameters correspond to policy-bits 0 & 1.
If type is 'es' than policy-bit 2 gets set to 1 to activate SEV-ES.
Policy bit 3 (nosend) is always set to 1, because migration features
for sev are not upstream yet and are attackable.

SEV-ES is highly experimental since it could not be tested.

see coherent doc patch

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
changes v11:
* add run_command() for the query-machine-capabilities program
* moved SEV related code to CPUConfig

 PVE/QemuServer.pm           | 13 +++++-
 PVE/QemuServer/CPUConfig.pm | 83 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 5df0c96..31a7ee9 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -53,7 +53,7 @@ use PVE::QemuConfig;
 use PVE::QemuServer::Helpers qw(config_aware_timeout min_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);
+use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object);
 use PVE::QemuServer::Drive qw(is_valid_drivename 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);
@@ -358,6 +358,12 @@ my $confdesc = {
 	description => "Memory properties.",
 	format => $PVE::QemuServer::Memory::memory_fmt
     },
+    amd_sev => {
+	description => "Secure Encrypted Virtualization (SEV) features by AMD CPUs",
+	optional => 1,
+	format => 'pve-qemu-sev-fmt',
+	type => 'string',
+    },
     balloon => {
 	optional => 1,
 	type => 'integer',
@@ -4094,6 +4100,11 @@ sub config_to_command {
 	}
     }
 
+    if ($conf->{amd_sev}) {
+	push @$devices, '-object', get_amd_sev_object($conf);
+	push @$machineFlags, 'confidential-guest-support=sev0';
+    }
+
     push @$cmd, @$devices;
     push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags);
     push @$cmd, '-machine', join(',', @$machineFlags) if scalar(@$machineFlags);
diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index 33f7524..2542aa2 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -3,9 +3,10 @@ package PVE::QemuServer::CPUConfig;
 use strict;
 use warnings;
 
+use JSON;
 use PVE::JSONSchema;
 use PVE::Cluster qw(cfs_register_file cfs_read_file);
-use PVE::Tools qw(get_host_arch);
+use PVE::Tools qw(run_command get_host_arch);
 use PVE::QemuServer::Helpers qw(min_version);
 
 use base qw(PVE::SectionConfig Exporter);
@@ -15,6 +16,7 @@ print_cpu_device
 get_cpu_options
 get_cpu_bitness
 is_native_arch
+get_amd_sev_object
 );
 
 # under certain race-conditions, this module might be loaded before pve-cluster
@@ -225,6 +227,37 @@ my $cpu_fmt = {
     },
 };
 
+my $sev_fmt = {
+    type => {
+	description => "Enable standard SEV with type='std' or enable"
+	    ." experimental SEV-ES with the 'es' option.",
+	type => 'string',
+	default_key => 1,
+	format_description => "sev-type",
+	enum => ['std', 'es'],
+	maxLength => 3,
+    },
+    'no-debug' => {
+	description => "Sets policy bit 0 to 1 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",
+	type => 'boolean',
+	default => 0,
+	optional => 1,
+    },
+    "kernel-hashes" => {
+	description => "Add kernel hashes to guest firmware for measured linux kernel launch",
+	type => 'boolean',
+	default => 0,
+	optional => 1,
+    },
+};
+PVE::JSONSchema::register_format('pve-qemu-sev-fmt', $sev_fmt);
+
 PVE::JSONSchema::register_format('pve-phys-bits', \&parse_phys_bits);
 sub parse_phys_bits {
     my ($str, $noerr) = @_;
@@ -773,6 +806,54 @@ sub get_cpu_bitness {
     die "unsupported architecture '$arch'\n";
 }
 
+sub get_hw_capabilities {
+    # Get reduced-phys-bits & cbitpos from host-hw-capabilities.json
+    my $filename = '/run/qemu-server/host-hw-capabilities.json';
+    if (! -e $filename) {
+       run_command("/usr/libexec/qemu-server/query-machine-capabilities");
+    }
+    my $json_text = PVE::Tools::file_get_contents($filename);
+    ($json_text) = $json_text =~ /(.*)/; # untaint json text
+    my $hw_capabilities = decode_json($json_text);
+    return $hw_capabilities;
+}
+
+sub get_amd_sev_object {
+    my ($conf) = @_;
+
+    if (!$conf->{bios} || ($conf->{bios} && $conf->{bios} ne 'ovmf')) {
+	die "To use SEV, you need to change the BIOS to OVMF.\n";
+    }
+    die "To use SEV, you need to add an efidisk.\n" if (!$conf->{efidisk0});
+
+    my $amd_sev_conf = PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{amd_sev});
+    my $sev_hw_caps = get_hw_capabilities()->{'amd-sev'};
+
+    if (!$sev_hw_caps->{'sev-support'}) {
+	die "Your CPU does not support AMD SEV!\n";
+    }
+    if ($amd_sev_conf->{type} eq 'es' && !$sev_hw_caps->{'sev-support-es'}) {
+	die "Your CPU does not support AMD SEV-ES!\n";
+    }
+
+    my $sev_mem_object = 'sev-guest,id=sev0'
+    .',cbitpos='.$sev_hw_caps->{cbitpos}
+    .',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 = 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'});
+    return $sev_mem_object;
+}
+
 __PACKAGE__->register();
 __PACKAGE__->init();
 
-- 
2.39.2



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

[pve-devel] [PATCH qemu-server v11 3/5] migration: add check_non_migratable_resources function

[Markus Frank]

The function checks for resources that cannot be migrated, snapshoted,
or suspended.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
changes v11:
* this patch is new to v11

 PVE/API2/Qemu.pm   |  5 ++++-
 PVE/QemuMigrate.pm |  2 +-
 PVE/QemuServer.pm  | 23 ++++++++++++++++++++++-
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2a1d4d7..8f36cf8 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4504,7 +4504,7 @@ __PACKAGE__->register_method({
 	$res->{running} = PVE::QemuServer::check_running($vmid) ? 1:0;
 
 	my ($local_resources, $mapped_resources, $missing_mappings_by_node) =
-	    PVE::QemuServer::check_local_resources($vmconf, 1);
+	    PVE::QemuServer::check_local_resources($vmconf, $res->{running}, 1);
 	delete $missing_mappings_by_node->{$localnode};
 
 	my $vga = PVE::QemuServer::parse_vga($vmconf->{vga});
@@ -5192,6 +5192,9 @@ __PACKAGE__->register_method({
 	die "unable to use snapshot name 'pending' (reserved name)\n"
 	    if lc($snapname) eq 'pending';
 
+	my $vmconf = PVE::QemuConfig->load_config($vmid);
+	PVE::QemuServer::check_non_migratable_resources($vmconf, $param->{vmstate}, 0);
+
 	my $realcmd = sub {
 	    PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: $snapname");
 	    PVE::QemuConfig->snapshot_create($vmid, $snapname, $param->{vmstate},
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 33d5b2d..27043c1 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -232,7 +232,7 @@ sub prepare {
 	$self->{vm_was_paused} = 1 if PVE::QemuServer::vm_is_paused($vmid, 0);
     }
 
-    my ($loc_res, $mapped_res, $missing_mappings_by_node) = PVE::QemuServer::check_local_resources($conf, 1);
+    my ($loc_res, $mapped_res, $missing_mappings_by_node) = PVE::QemuServer::check_local_resources($conf, $running, 1);
     my $blocking_resources = [];
     for my $res ($loc_res->@*) {
 	if (!grep($res, $mapped_res->@*)) {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 31a7ee9..9f342bf 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2563,13 +2563,32 @@ sub config_list {
     return $res;
 }
 
+sub check_non_migratable_resources {
+    my ($conf, $state, $noerr) = @_;
+
+    my @non_migr_res = ();
+    if ($state && $conf->{amd_sev}) {
+	push @non_migr_res, "amd_sev";
+    }
+
+    if (scalar @non_migr_res && !$noerr) {
+	die "Cannot live-migrate, snapshot (with RAM), or hibernate a VM with:"
+	    ." @non_migr_res\n";
+    }
+
+    return @non_migr_res;
+}
+
 # test if VM uses local resources (to prevent migration)
 sub check_local_resources {
-    my ($conf, $noerr) = @_;
+    my ($conf, $state, $noerr) = @_;
 
     my @loc_res = ();
     my $mapped_res = [];
 
+    my @non_migr_res = check_non_migratable_resources($conf, $state, $noerr);
+    push(@loc_res, @non_migr_res);
+
     my $nodelist = PVE::Cluster::get_nodelist();
     my $pci_map = PVE::Mapping::PCI::config();
     my $usb_map = PVE::Mapping::USB::config();
@@ -6362,6 +6381,8 @@ sub vm_suspend {
 	die "cannot suspend to disk during backup\n"
 	    if $is_backing_up && $includestate;
 
+	check_non_migratable_resources($conf, $includestate, 0);
+
 	if ($includestate) {
 	    $conf->{lock} = 'suspending';
 	    my $date = strftime("%Y-%m-%d", localtime(time()));
-- 
2.39.2



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

[pve-devel] [PATCH docs v11 4/5] add AMD SEV documentation

[Markus Frank]

add documentation for the "[PATCH qemu-server] config: QEMU AMD SEV
enable" patch.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 qm.adoc | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 42c26db..2001bd4 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -715,6 +715,109 @@ systems.
 When allocating RAM to your VMs, a good rule of thumb is always to leave 1GB
 of RAM available to the host.
 
+[[qm_memory_encryption]]
+Memory Encryption
+~~~~~~~~~~~~~~~~~
+
+[[qm_memory_encryption_sev]]
+AMD SEV
+^^^^^^^
+
+SEV (Secure Encrypted Virtualization) enables memory encryption per VM using
+AES-128 encryption and the AMD Secure Processor.
+
+SEV-ES (Secure Encrypted Virtualization-Encrypted State) in addition encrypts
+all CPU register contents when a VM stops running, to prevent leakage of
+information to the hypervisor. This feature is very experimental.
+
+*Host Requirements:*
+
+* AMD EPYC CPU
+* SEV-ES is only supported on AMD EPYC 7xx2 and newer
+* configure AMD memory encryption in the BIOS settings of the host machine
+* add "kvm_amd.sev=1" to kernel parameters if not enabled by default
+* add "mem_encrypt=on" to kernel parameters if you want to encrypt memory on the
+host (SME) see https://www.kernel.org/doc/Documentation/x86/amd-memory-encryption.txt
+* maybe increase SWIOTLB see https://github.com/AMDESE/AMDSEV#faq-4
+
+To check if SEV is enabled on the host search for `sev` in dmesg and print out
+the SEV kernel parameter of kvm_amd:
+
+----
+# dmesg | grep -i sev
+[...] ccp 0000:45:00.1: sev enabled
+[...] ccp 0000:45:00.1: SEV API: <buildversion>
+[...] SEV supported: <number> ASIDs
+[...] SEV-ES supported: <number> ASIDs
+# cat /sys/module/kvm_amd/parameters/sev
+Y
+----
+
+*Guest Requirements:*
+
+* edk2-OVMF
+* advisable to use Q35
+* The guest operating system must contain SEV-support.
+
+*Limitations:*
+
+* Because the memory is encrypted the memory usage on host is always wrong.
+* Operations that involve saving or restoring memory like snapshots
+& live migration do not work yet or are attackable.
+https://github.com/PSPReverse/amd-sev-migration-attack
+* PCI passthrough is not supported.
+* SEV-ES is very experimental.
+* QEMU & AMD-SEV documentation is very limited.
+
+Example Configuration:
+
+----
+# qm set <vmid> -amd_sev type=std,no-debug=1,no-key-sharing=1,kernel-hashes=1
+----
+
+The *type* defines the encryption technology ("type=" is not necessary).
+Available options are std & es.
+
+The QEMU *policy* parameter gets calculated with the *no-debug* and
+*no-key-sharing* parameters. These parameters correspond to policy-bit 0 and 1.
+If *type* is *es* the policy-bit 2 is set to 1 so that SEV-ES is enabled.
+Policy-bit 3 (nosend) is always set to 1 to prevent migration-attacks. For more
+information on how to calculate the policy see:
+https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf[AMD SEV API Specification Chapter 3]
+
+The *kernel-hashes* is per default off for backward compatibility with older
+OVMF images and guests that do not measure the kernel/initrd.
+See https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg02598.html
+
+*Check if SEV is working on the guest*
+
+Method 1 - dmesg:
+
+Output should look like this.
+
+----
+# dmesg | grep -i sev
+AMD Memory Encryption Features active: SEV
+----
+
+Method 2 - MSR 0xc0010131 (MSR_AMD64_SEV):
+
+Output should be 1.
+
+----
+# apt install msr-tools
+# modprobe msr
+# rdmsr -a 0xc0010131
+1
+----
+
+Links:
+
+* https://developer.amd.com/sev/
+* https://github.com/AMDESE/AMDSEV
+* https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html
+* https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
+* https://documentation.suse.com/sles/15-SP1/html/SLES-amd-sev/index.html
 
 [[qm_network_device]]
 Network Device
-- 
2.39.2



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

[pve-devel] [PATCH manager v11 5/5] ui: add AMD SEV configuration to Options

[Markus Frank]

By adding a new input panel with an AMD SEV technology selection combo
box and checkboxes for the optional parameters in an advanced section,
the user can configure the amd_sev option via the WebUI's Options tab.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
changes v11:
* instead of the no-debug and no-key-sharing checkboxes there are now
 "allow debug/key-sharing" checkboxes with true beeing the default.

 www/manager6/Makefile        |   1 +
 www/manager6/qemu/Options.js |  11 ++++
 www/manager6/qemu/SevEdit.js | 121 +++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 www/manager6/qemu/SevEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 2c3a822b..801683a3 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -264,6 +264,7 @@ JSSRC= 							\
 	qemu/SSHKey.js					\
 	qemu/ScsiHwEdit.js				\
 	qemu/SerialEdit.js				\
+	qemu/SevEdit.js					\
 	qemu/Smbios1Edit.js				\
 	qemu/SystemEdit.js				\
 	qemu/USBEdit.js					\
diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index 7b112400..6907699c 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -338,6 +338,17 @@ Ext.define('PVE.qemu.Options', {
 		    },
 		} : undefined,
 	    },
+	    amd_sev: {
+		header: gettext('AMD SEV'),
+		editor: caps.vms['VM.Config.HWType'] ? 'PVE.qemu.SevEdit' : undefined,
+		defaultValue: Proxmox.Utils.defaultText + ' (' + Proxmox.Utils.disabledText + ')',
+		renderer: function(value, metaData, record, ri, ci, store, pending) {
+		    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 + ')';
+		    return value;
+		},
+	    },
 	    hookscript: {
 		header: gettext('Hookscript'),
 	    },
diff --git a/www/manager6/qemu/SevEdit.js b/www/manager6/qemu/SevEdit.js
new file mode 100644
index 00000000..db7ff3a8
--- /dev/null
+++ b/www/manager6/qemu/SevEdit.js
@@ -0,0 +1,121 @@
+Ext.define('PVE.qemu.SevInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pveSevInputPanel',
+    onlineHelp: 'qm_memory_encryption',
+
+    viewModel: {
+	data: {
+	    type: '__default__',
+	},
+	formulas: {
+	    sevEnabled: get => get('type') !== '__default__',
+	},
+    },
+
+    onGetValues: function(values) {
+	if (values.delete === 'type') {
+	    values.delete = 'amd_sev';
+	    return values;
+	}
+	if (!values.debug) {
+	    values["no-debug"] = 1;
+	}
+	if (!values["key-sharing"]) {
+	    values["no-key-sharing"] = 1;
+	}
+	delete values.debug;
+	delete values["key-sharing"];
+	let ret = {};
+	ret.amd_sev = PVE.Parser.printPropertyString(values, 'type');
+	return ret;
+    },
+
+
+    setValues: function(values) {
+	if (PVE.Parser.parseBoolean(values["no-debug"])) {
+	    values.debug = 0;
+	}
+	if (PVE.Parser.parseBoolean(values["no-key-sharing"])) {
+	    values["key-sharing"] = 0;
+	}
+	this.callParent(arguments);
+    },
+
+    items: {
+	xtype: 'proxmoxKVComboBox',
+	fieldLabel: gettext('AMD Secure Encrypted Virtualization (SEV)'),
+	labelWidth: 150,
+	name: 'type',
+	value: '__default__',
+	comboItems: [
+	    ['__default__', Proxmox.Utils.defaultText + ' (' + Proxmox.Utils.disabledText + ')'],
+	    ['std', 'AMD SEV'],
+	    ['es', 'AMD SEV-ES (highly experimental)'],
+	],
+	bind: {
+	    value: '{type}',
+	},
+    },
+
+    advancedItems: [
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Allow Debugging'),
+	    labelWidth: 150,
+	    name: 'debug',
+	    value: 1,
+	    bind: {
+		hidden: '{!sevEnabled}',
+		disabled: '{!sevEnabled}',
+	    },
+	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Allow Key-Sharing'),
+	    labelWidth: 150,
+	    name: 'key-sharing',
+	    value: 1,
+	    bind: {
+		hidden: '{!sevEnabled}',
+		disabled: '{!sevEnabled}',
+	    },
+	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Enable Kernel Hashes'),
+	    labelWidth: 150,
+	    name: 'kernel-hashes',
+	    deleteDefaultValue: false,
+	    bind: {
+		hidden: '{!sevEnabled}',
+		disabled: '{!sevEnabled}',
+	    },
+	},
+    ],
+});
+
+Ext.define('PVE.qemu.SevEdit', {
+    extend: 'Proxmox.window.Edit',
+
+    subject: gettext('SEV'),
+
+    items: {
+	xtype: 'pveSevInputPanel',
+    },
+
+    width: 400,
+
+    initComponent: function() {
+	let me = this;
+
+	me.callParent();
+
+	me.load({
+	    success: function(response) {
+		let conf = response.result.data;
+		let amd_sev = conf.amd_sev || '__default__';
+		me.setValues(PVE.Parser.parsePropertyString(amd_sev, 'type'));
+	    },
+	});
+    },
+});
-- 
2.39.2



_______________________________________________
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/docs/manager v11 0/5] AMD SEV

[Markus Frank]

Ping. Patches still apply.

On  2024-05-29 14:23, Markus Frank wrote:
> Patch series to enable AMD Secure Encrypted Virtualization (SEV)
> https://www.amd.com/en/developer/sev.html
> 
> changes v11:
> * removed systemd service and added run_command in qemu-server instead
> * moved SEV related code to CPUConfig
> * instead of the no-debug and no-key-sharing checkboxes there are no
>   "allow debug/key-sharing" checkboxes in the WebUI with true beeing
>   the default.
> * add check_non_migratable_resources function in qemu-server
> 
> changes v10:
> * removed include of sys/types.h in C Program
> * also die if the BIOS is not set, since the default is SeaBIOS
> * added pve-manager patch
> 
> apply/compile order:
> 1. qemu-server: add C program to get hardware capabilities from CPUID
> 2. qemu-server: config: add AMD SEV support
> 3. qemu-server: migration: add check_non_migratable_resources function
> 4. pve-docs: add AMD SEV documentation
> 5. pve-manager: ui: add AMD SEV configuration to Options
> 
> 
> qemu-server:
> 
> Markus Frank (3):
>    add C program to get hardware capabilities from CPUID
>    config: add AMD SEV support
>    migration: add check_non_migratable_resources function
> 
>   Makefile                                      |  1 +
>   PVE/API2/Qemu.pm                              |  5 +-
>   PVE/QemuMigrate.pm                            |  2 +-
>   PVE/QemuServer.pm                             | 36 +++++++-
>   PVE/QemuServer/CPUConfig.pm                   | 83 ++++++++++++++++++-
>   query-machine-capabilities/Makefile           | 18 ++++
>   .../query-machine-capabilities.c              | 79 ++++++++++++++++++
>   7 files changed, 219 insertions(+), 5 deletions(-)
>   create mode 100644 query-machine-capabilities/Makefile
>   create mode 100644 query-machine-capabilities/query-machine-capabilities.c
> 
> 
> docs:
> 
> Markus Frank (1):
>    add AMD SEV documentation
> 
>   qm.adoc | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 103 insertions(+)
> 
> 
> manager:
> 
> Markus Frank (1):
>    ui: add AMD SEV configuration to Options
> 
>   www/manager6/Makefile        |   1 +
>   www/manager6/qemu/Options.js |  11 ++++
>   www/manager6/qemu/SevEdit.js | 121 +++++++++++++++++++++++++++++++++++
>   3 files changed, 133 insertions(+)
>   create mode 100644 www/manager6/qemu/SevEdit.js
> 


_______________________________________________
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 v11 1/5] add C program to get hardware capabilities from CPUID

[Fiona Ebner]

Am 29.05.24 um 14:23 schrieb Markus Frank:
> Implement a C program that extracts AMD SEV hardware information such
> as reduced-phys-bios and cbitpos from CPUID at boot time, looks if
> SEV, SEV-ES & SEV-SNP are enabled, and outputs these details as JSON
> to /run/qemu-server/host-hw-capabilities.json
> 
> This programm can also be used to read and save other hardware

Typo: should be "program"

> information at boot time.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Tested-by: Filip Schauer <f.schauer@proxmox.com>

Since there were changes, the Tested-by trailer should be removed.

With the suggested changes below:

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

> ---
> changes v11:
> * removed systemd service

* improved error handling

> +    ret = fprintf(file,
> +	"{"
> +	" \"amd-sev\": {"
> +	" \"cbitpos\": %u,"
> +	" \"reduced-phys-bits\": %u,"
> +	" \"sev-support\": %s,"
> +	" \"sev-support-es\": %s,"
> +	" \"sev-support-snp\": %s"
> +	" }"
> +	" }\n",
> +	cbitpos,
> +	reduced_phys_bits,
> +	sev_support ? "true" : "false",
> +	sev_es_support ? "true" : "false",
> +	sev_snp_support ? "true" : "false"
> +    );
> +    if (ret == -1) {

"man 3 fprintf" states:

> If an output error is encountered, a negative value is returned."

and while a quick glance at the source code shows that -1 is used in
many places, I'm not sure that's true for all error paths. Let's check
for < 0 instead.

> +	printf("Error writing to file %s: %s\n", path, strerror(errno));
> +    }
> +
> +    ret = fclose(file);

Nit:

"man 3 fclose" states:

> Upon successful completion, 0 is returned. Otherwise, EOF is returned
> and errno is set to indicate the error.

While EOF is defined to be -1 in stdio.h, it's better to use the
constant explicitly (or could also use ret != 0).

> +    if (ret == -1) {
> +	printf("Error closing file %s: %s\n", path, strerror(errno));
> +    }
> +
> +    return 0;
> +}


_______________________________________________
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 v11 2/5] config: add AMD SEV support

[Fiona Ebner]

Am 29.05.24 um 14:23 schrieb Markus Frank:
> Signed-off-by: Markus Frank <m.frank@proxmox.com>

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

with some (style) nits that can still be addressed:

> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 33f7524..2542aa2 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -3,9 +3,10 @@ package PVE::QemuServer::CPUConfig;
>  use strict;
>  use warnings;
>  
> +use JSON;

Style nit: please add a blank line between non-PVE and PVE modules

> @@ -773,6 +806,54 @@ sub get_cpu_bitness {
>      die "unsupported architecture '$arch'\n";
>  }
>  
> +sub get_hw_capabilities {
> +    # Get reduced-phys-bits & cbitpos from host-hw-capabilities.json
> +    my $filename = '/run/qemu-server/host-hw-capabilities.json';
> +    if (! -e $filename) {
> +       run_command("/usr/libexec/qemu-server/query-machine-capabilities");
> +    }
> +    my $json_text = PVE::Tools::file_get_contents($filename);
> +    ($json_text) = $json_text =~ /(.*)/; # untaint json text
> +    my $hw_capabilities = decode_json($json_text);

Nit: could use eval and add context to errors from decode_json()

> +    return $hw_capabilities;
> +}
> +
> +sub get_amd_sev_object {
> +    my ($conf) = @_;

Nit: could also pass the three properties from the config it actually
needs instead of passing the whole config for a more explicit function
signature.

> +
> +    if (!$conf->{bios} || ($conf->{bios} && $conf->{bios} ne 'ovmf')) {
> +	die "To use SEV, you need to change the BIOS to OVMF.\n";
> +    }
> +    die "To use SEV, you need to add an efidisk.\n" if (!$conf->{efidisk0});

I'd move the checks to below the ones for the hardware capability.
Otherwise a user might see the error about BIOS first, change it, try
again and then fail with the error about lacking hardware support. I'd
also use "AMD SEV" instead of just "SEV".

What happens if there is no EFI disk (i.e. the temporary efivars disk is
used)?

Style nit: superfluous parentheses in post-if

> +
> +    my $amd_sev_conf = PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{amd_sev});
> +    my $sev_hw_caps = get_hw_capabilities()->{'amd-sev'};
> +
> +    if (!$sev_hw_caps->{'sev-support'}) {
> +	die "Your CPU does not support AMD SEV!\n";> +    }
> +    if ($amd_sev_conf->{type} eq 'es' && !$sev_hw_caps->{'sev-support-es'}) {
> +	die "Your CPU does not support AMD SEV-ES!\n";
> +    }

No need for the exclamation marks for those two error messages IMHO.

> +
> +    my $sev_mem_object = 'sev-guest,id=sev0'
> +    .',cbitpos='.$sev_hw_caps->{cbitpos}
> +    .',reduced-phys-bits='.$sev_hw_caps->{'reduced-phys-bits'};

Style nit: usually this is written as

my $a = "foo";
$a .= "bar";

in our code base.

> +
> +    # 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');

Style nit: superfluous parentheses in post-if, also below

> +    # 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'});
> +    return $sev_mem_object;
> +}
> +
>  __PACKAGE__->register();
>  __PACKAGE__->init();
>  


_______________________________________________
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 v11 3/5] migration: add check_non_migratable_resources function

[Fiona Ebner]

Am 29.05.24 um 14:23 schrieb Markus Frank:
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 2a1d4d7..8f36cf8 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -4504,7 +4504,7 @@ __PACKAGE__->register_method({
>  	$res->{running} = PVE::QemuServer::check_running($vmid) ? 1:0;
>  
>  	my ($local_resources, $mapped_resources, $missing_mappings_by_node) =
> -	    PVE::QemuServer::check_local_resources($vmconf, 1);
> +	    PVE::QemuServer::check_local_resources($vmconf, $res->{running}, 1);
>  	delete $missing_mappings_by_node->{$localnode};
>  
>  	my $vga = PVE::QemuServer::parse_vga($vmconf->{vga});
> @@ -5192,6 +5192,9 @@ __PACKAGE__->register_method({
>  	die "unable to use snapshot name 'pending' (reserved name)\n"
>  	    if lc($snapname) eq 'pending';
>  
> +	my $vmconf = PVE::QemuConfig->load_config($vmid);
> +	PVE::QemuServer::check_non_migratable_resources($vmconf, $param->{vmstate}, 0);

This can be fine as an early check. But there should be another check
after the 'snapshot' lock was set in the config (or right before while
still holding the config file lock). Otherwise, there is no guarantee
that the config is the same at the time of the check and during the
actual snapshot operation.

My suggestion is to introduce a new abstract method (named something
like __snapshot_assert_no_blockers()) in AbstractConfig that is called
during __snapshot_prepare() which QemuConfig can override. Like that we
can do the check before writing the modified config for the snapshot.

> +
>  	my $realcmd = sub {
>  	    PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: $snapname");
>  	    PVE::QemuConfig->snapshot_create($vmid, $snapname, $param->{vmstate},
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 31a7ee9..9f342bf 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2563,13 +2563,32 @@ sub config_list {
>      return $res;
>  }
>  
> +sub check_non_migratable_resources {
> +    my ($conf, $state, $noerr) = @_;
> +
> +    my @non_migr_res = ();

Style nit: I'd prefer writing out the name or use something like
"blockers" (inside the function where it's clear what is meant) if it
really is too long

> +    if ($state && $conf->{amd_sev}) {
> +	push @non_migr_res, "amd_sev";
> +    }
> +
> +    if (scalar @non_migr_res && !$noerr) {

Style nit: I'd prefer parentheses for scalar(@stuff)

> +	die "Cannot live-migrate, snapshot (with RAM), or hibernate a VM with:"
> +	    ." @non_migr_res\n";
> +    }
> +
> +    return @non_migr_res;
> +}
> +


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