* [pve-devel] [PATCH qemu-server/docs v2 0/2] AMD SEV
@ 2022-11-11 14:27 Markus Frank
2022-11-11 14:27 ` [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable Markus Frank
2022-11-11 14:27 ` [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation Markus Frank
0 siblings, 2 replies; 8+ messages in thread
From: Markus Frank @ 2022-11-11 14:27 UTC (permalink / raw)
To: pve-devel
qemu-server:
v2:
* spelling of minimum
* !$conf->{bios} eq 'ovmf' changed to $conf->{bios} ne 'ovmf'
Markus Frank (1):
QEMU AMD SEV enable
PVE/QemuServer.pm | 133 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 133 insertions(+)
docs:
v2:
* added more details for host & clients
* moved things from Limitations to Requirements
* changed order of text
Markus Frank (1):
added Memory Encryption documentation
qm.adoc | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 113 insertions(+)
--
2.30.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable 2022-11-11 14:27 [pve-devel] [PATCH qemu-server/docs v2 0/2] AMD SEV Markus Frank @ 2022-11-11 14:27 ` Markus Frank 2022-11-14 13:06 ` Fiona Ebner 2022-11-11 14:27 ` [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation Markus Frank 1 sibling, 1 reply; 8+ messages in thread From: Markus Frank @ 2022-11-11 14:27 UTC (permalink / raw) To: pve-devel This Patch is for enabling AMD SEV (Secure Encrypted Virtualization) support in QEMU and enabling future memory encryption technologies like INTEL MKTME (Multi-key Total Memory Encryption) and SEV-SNP. Config-Example: memory_encryption: type=sev,cbitpos=47,policy=0x0001,reduced-phys-bits=1 reduced-phys-bios, cbitpos and policy correspond to the varibles with the same name in qemu. reduced-phys-bios and cbitpos are system specific and can be read out with QMP. If not set by the user, a dummy-vm gets started to read QMP for these variables out and save them to config. Afterwards the dummy-vm gets stopped. policy can be calculated with the links in comments & description. To test SEV-ES (CPU register encryption) the policy should be set somewhere between 0x4 and 0x7 or 0xC and 0xF, etc. (Bit-2 has to be set 1 (LSB 0 bit numbering)) SEV needs edk2-OVMF to work. Signed-off-by: Markus Frank <m.frank@proxmox.com> --- PVE/QemuServer.pm | 133 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 513a248..2ea8abd 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -175,6 +175,58 @@ my $agent_fmt = { }, }; +my $memory_encryption_fmt = { + type => { + type => 'string', + default_key => 1, + description => "Memory Encryption Type:" + ." for AMD SEV -> 'memory_encryption: type=sev';" + ." for AMD SEV-ES -> use 'sev' and change policy to between 0x4 and 0x7;" + ." (Bit-2 has to be set 1 (LSB 0 bit numbering))" + #. "for AMD SEV-SNP -> 'memory_encryption: type=sev-snp'" + ." (sev requires edk2-ovmf & sev kernel support by guest operating system &" + ." on host: add kernel-parameters 'mem_encrypt=on kvm_amd.sev=1')" + ." see https://github.com/AMDESE/AMDSEV &" + ." https://documentation.suse.com/sles/15-SP1/html/SLES-amd-sev/index.html", + format_description => "qemu-memory-encryption-type", + # TODO enable sev-snp option when feature can be tested on 3rd-gen EPYC + # https://www.phoronix.com/news/AMD-SEV-SNP-Arrives-Linux-5.19 + # enum => ['sev','sev-snp','mktme'], + enum => ['sev'], + maxLength => 10, + }, + 'reduced-phys-bits' => { + description => "Number of bits the physical address space is reduced by. System dependent", + type => 'integer', + default => 1, + optional => 1, + minimum => 0, + maximum => 100, + }, + cbitpos => { + description => "C-bit: marks if a memory page is protected. System dependent", + type => 'integer', + default => 47, + optional => 1, + minimum => 0, + maximum => 100, + }, + policy => { + description => "SEV Guest Policy" + . "see Capter 3:" + . "https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf" + . "& https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html", + + format_description => "qemu-memory-encryption-policy", + type => 'string', + default => '0x0000', + pattern => '0[xX][0-9a-fA-F]{1,4}', + optional => 1, + maxLength => 6, + }, +}; +PVE::JSONSchema::register_format('pve-qemu-memory-encryption-fmt', $memory_encryption_fmt); + my $vga_fmt = { type => { description => "Select the VGA type.", @@ -349,6 +401,12 @@ my $confdesc = { minimum => 16, default => 512, }, + memory_encryption => { + description => "Memory Encryption", + optional => 1, + format => 'pve-qemu-memory-encryption-fmt', + type => 'string', + }, balloon => { optional => 1, type => 'integer', @@ -2113,6 +2171,17 @@ sub parse_guest_agent { return $res; } +sub parse_memory_encryption { + my ($value) = @_; + + return if !$value; + + my $res = eval { parse_property_string($memory_encryption_fmt, $value) }; + warn $@ if $@; + return $res; +} + + sub get_qga_key { my ($conf, $key) = @_; return undef if !defined($conf->{agent}); @@ -4085,6 +4154,70 @@ sub config_to_command { } push @$machineFlags, "type=${machine_type_min}"; + # Memory Encryption + my $memory_encryption = parse_memory_encryption($conf->{'memory_encryption'}); + + # Die if bios is not ovmf + if ( + $memory_encryption->{'type'} + && $memory_encryption->{type} eq 'sev' + && $conf->{bios} ne 'ovmf' + ) { + die "SEV needs ovmf\n"; + } + + # AMD SEV + if ($memory_encryption->{'type'} && $memory_encryption->{type} =~ /(sev|sev-snp)/) { + # Get reduced-phys-bits & cbitpos from QMP, if not set + if ( + !$memory_encryption->{'reduced-phys-bits'} + || !$memory_encryption->{cbitpos} + ) { + my $fakevmid = -1; + my $qemu_cmd = get_command_for_arch($arch); + my $pidfile = PVE::QemuServer::Helpers::pidfile_name($fakevmid); + my $default_machine = $default_machines->{$arch}; + my $cmd = [ + $qemu_cmd, + '-machine', $default_machine, + '-display', 'none', + '-chardev', "socket,id=qmp,path=/var/run/qemu-server/$fakevmid.qmp,server=on,wait=off", + '-mon', 'chardev=qmp,mode=control', + '-pidfile', $pidfile, + '-S', '-daemonize' + ]; + my $rc = run_command($cmd, noerr => 1, quiet => 0); + die "QEMU flag querying VM exited with code " . $rc . "\n" if $rc; + my $res = mon_cmd($fakevmid, 'query-sev-capabilities'); + $memory_encryption->{'reduced-phys-bits'} = $res->{'reduced-phys-bits'}; + $memory_encryption->{cbitpos} = $res->{cbitpos}; + $conf->{'memory_encryption'} = PVE::JSONSchema::print_property_string( + $memory_encryption, + $memory_encryption_fmt + ); + PVE::QemuConfig->write_config($vmid, $conf); + vm_stop(undef, $fakevmid, 1, 1, 10, 0, 1); + } + # qemu-Example: -object 'sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1'; + # see https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html + my $memobjcmd = ""; + if ($memory_encryption->{type} eq 'sev-snp') { + # future feature: cannot be reached + $memobjcmd = 'sev-snp-guest'; + } else { + $memobjcmd = 'sev-guest'; + } + $memobjcmd .= ',id=sev0,cbitpos='.$memory_encryption->{cbitpos} + .',reduced-phys-bits='.$memory_encryption->{'reduced-phys-bits'}; + if ($memory_encryption->{type} eq 'sev' && $memory_encryption->{policy}) { + $memobjcmd .= ',policy='.$memory_encryption->{policy} + } + push @$devices, '-object' , $memobjcmd; + # old qemu-Example: -machine 'type=q35+pve0,memory-encryption=sev0' + # https://fossies.org/linux/qemu/docs/system/confidential-guest-support.rst + 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); -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable 2022-11-11 14:27 ` [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable Markus Frank @ 2022-11-14 13:06 ` Fiona Ebner 2022-11-17 10:50 ` Markus Frank 0 siblings, 1 reply; 8+ messages in thread From: Fiona Ebner @ 2022-11-14 13:06 UTC (permalink / raw) To: pve-devel, m.frank Am 11.11.22 um 15:27 schrieb Markus Frank: > This Patch is for enabling AMD SEV (Secure Encrypted > Virtualization) support in QEMU and enabling future > memory encryption technologies like INTEL MKTME > (Multi-key Total Memory Encryption) and SEV-SNP. > > Config-Example: > memory_encryption: type=sev,cbitpos=47,policy=0x0001,reduced-phys-bits=1 > > reduced-phys-bios, cbitpos and policy correspond to the varibles with the > same name in qemu. > > reduced-phys-bios and cbitpos are system specific and can be read out > with QMP. If not set by the user, a dummy-vm gets started to read QMP > for these variables out and save them to config. Afterwards the dummy-vm gets > stopped. Why even allow the user to set them if they are system-specific values? Or are there multiple possible values on some systems? If not, it should be a node-specific configuration, rather than a VM-specific one. That would also only require starting the dummy VM once per node, or we could require the user to set the values in some node config (of course mentioning how in the docs :)) > > policy can be calculated with the links in comments & description. > To test SEV-ES (CPU register encryption) the policy should be set > somewhere between 0x4 and 0x7 or 0xC and 0xF, etc. > (Bit-2 has to be set 1 (LSB 0 bit numbering)) > > SEV needs edk2-OVMF to work. > > Signed-off-by: Markus Frank <m.frank@proxmox.com> > --- > PVE/QemuServer.pm | 133 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 133 insertions(+) > > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 513a248..2ea8abd 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -175,6 +175,58 @@ my $agent_fmt = { > }, > }; > > +my $memory_encryption_fmt = { > + type => { > + type => 'string', > + default_key => 1, > + description => "Memory Encryption Type:" Nit: I'd rather have the description be a sentence or two, what it's all about and add a verbose_description to describe the individual variants. > + ." for AMD SEV -> 'memory_encryption: type=sev';" > + ." for AMD SEV-ES -> use 'sev' and change policy to between 0x4 and 0x7;" > + ." (Bit-2 has to be set 1 (LSB 0 bit numbering))" Nit: better to use 0x0004 and 0x0007, because 0x4 and 0x7 are not valid values for 'policy' below. > + #. "for AMD SEV-SNP -> 'memory_encryption: type=sev-snp'" > + ." (sev requires edk2-ovmf & sev kernel support by guest operating system &" > + ." on host: add kernel-parameters 'mem_encrypt=on kvm_amd.sev=1')" > + ." see https://github.com/AMDESE/AMDSEV &" > + ." https://documentation.suse.com/sles/15-SP1/html/SLES-amd-sev/index.html", > + format_description => "qemu-memory-encryption-type", > + # TODO enable sev-snp option when feature can be tested on 3rd-gen EPYC > + # https://www.phoronix.com/news/AMD-SEV-SNP-Arrives-Linux-5.19 > + # enum => ['sev','sev-snp','mktme'], Nit: I feel like these comments don't really belong in the patch. Maybe just add a single high-level TODO comment? The rest should be done by the patch actually adding sev-snp ;) Also, the many links might be better left to the documentation patch. Is the rest of the format even compatible with Intel's MKTME? I.e. does/will that also have reduced-phys-bits, 4 policy bits and cbitpos? If there is some overlap or if we expect to be easily able to translate certain settings, we can still keep a general memory_encryption_fmt, but otherwise, it might be better to have completely distinct formats for Intel and AMD? > + enum => ['sev'], > + maxLength => 10, > + }, > + 'reduced-phys-bits' => { > + description => "Number of bits the physical address space is reduced by. System dependent", > + type => 'integer', > + default => 1, The default is system-dependent and automatically figured out by the dummy VM. Also the kvm man pages states > On EPYC, the value should be 5. so why 1? > + optional => 1, > + minimum => 0, > + maximum => 100, > + }, > + cbitpos => { > + description => "C-bit: marks if a memory page is protected. System dependent", > + type => 'integer', > + default => 47, Same here with regards to auto-magic. > + optional => 1, > + minimum => 0, > + maximum => 100, > + }, > + policy => { > + description => "SEV Guest Policy" > + . "see Capter 3:" Nit: typo > + . "https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf" > + . "& https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html", > + Could have a verbose_description for each bit rather than the links. Or should we go one step further and add explicit flags for the relevant-for-us bits instead? Would be more user-friendly, but the $memory_encryption_fmt would be AMD-specific of course. > + format_description => "qemu-memory-encryption-policy", > + type => 'string', > + default => '0x0000', > + pattern => '0[xX][0-9a-fA-F]{1,4}', > + optional => 1, > + maxLength => 6, > + }, > +}; > +PVE::JSONSchema::register_format('pve-qemu-memory-encryption-fmt', $memory_encryption_fmt); > + > my $vga_fmt = { > type => { > description => "Select the VGA type.", > @@ -349,6 +401,12 @@ my $confdesc = { > minimum => 16, > default => 512, > }, > + memory_encryption => { > + description => "Memory Encryption", > + optional => 1, > + format => 'pve-qemu-memory-encryption-fmt', > + type => 'string', > + }, > balloon => { > optional => 1, > type => 'integer', > @@ -2113,6 +2171,17 @@ sub parse_guest_agent { > return $res; > } > > +sub parse_memory_encryption { > + my ($value) = @_; > + > + return if !$value; > + > + my $res = eval { parse_property_string($memory_encryption_fmt, $value) }; > + warn $@ if $@; > + return $res; > +} Why not fail if parsing fails? > + > + > sub get_qga_key { > my ($conf, $key) = @_; > return undef if !defined($conf->{agent}); > @@ -4085,6 +4154,70 @@ sub config_to_command { > } > push @$machineFlags, "type=${machine_type_min}"; > > + # Memory Encryption Nit: comment contains no additional information. > + my $memory_encryption = parse_memory_encryption($conf->{'memory_encryption'}); > + > + # Die if bios is not ovmf Nit: Same here. > + if ( > + $memory_encryption->{'type'} > + && $memory_encryption->{type} eq 'sev' > + && $conf->{bios} ne 'ovmf' I think $conf->{bios} could be undef here? That would cause an ugly Perl warning. At least other place check for !defined($conf->{bios}) || $conf->{bios} ne 'ovmf' > + ) { > + die "SEV needs ovmf\n"; Nit: maybe a bit too concise of a message, could mention the settings at least > + } > + All of the below should rather live in helper functions, especially the dummy VM stuff. config_to_command is already much too bloated. > + # AMD SEV > + if ($memory_encryption->{'type'} && $memory_encryption->{type} =~ /(sev|sev-snp)/) { Nit: I'd rather have explicit equality testing for the type (or at least add a leading ^ to the regex). The sev-snp type does not exist yet and should be added by a later patch. > + # Get reduced-phys-bits & cbitpos from QMP, if not set > + if ( > + !$memory_encryption->{'reduced-phys-bits'} > + || !$memory_encryption->{cbitpos} > + ) { > + my $fakevmid = -1; > + my $qemu_cmd = get_command_for_arch($arch); > + my $pidfile = PVE::QemuServer::Helpers::pidfile_name($fakevmid); > + my $default_machine = $default_machines->{$arch}; > + my $cmd = [ > + $qemu_cmd, > + '-machine', $default_machine, > + '-display', 'none', > + '-chardev', "socket,id=qmp,path=/var/run/qemu-server/$fakevmid.qmp,server=on,wait=off", > + '-mon', 'chardev=qmp,mode=control', > + '-pidfile', $pidfile, > + '-S', '-daemonize' > + ]; Instead of daemonizing, pidfile etc. we could also use --qmp stdio and pass the commands via stdin like: {"execute": "qmp_capabilities"} {"execute": "query-sev-capabilities"} {"execute": "quit"} which might be a bit more straight-forward. But maybe we prefer re-using the existing infrastructure with the fake ID, not sure? > + my $rc = run_command($cmd, noerr => 1, quiet => 0); > + die "QEMU flag querying VM exited with code " . $rc . "\n" if $rc; > + my $res = mon_cmd($fakevmid, 'query-sev-capabilities'); > + $memory_encryption->{'reduced-phys-bits'} = $res->{'reduced-phys-bits'}; > + $memory_encryption->{cbitpos} = $res->{cbitpos}; > + $conf->{'memory_encryption'} = PVE::JSONSchema::print_property_string( > + $memory_encryption, > + $memory_encryption_fmt > + ); > + PVE::QemuConfig->write_config($vmid, $conf); config_to_command should not write the config. Hope that those settings are truly node-specific and not VM-specific :) > + vm_stop(undef, $fakevmid, 1, 1, 10, 0, 1); > + } > + # qemu-Example: -object 'sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1'; > + # see https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html > + my $memobjcmd = ""; > + if ($memory_encryption->{type} eq 'sev-snp') { > + # future feature: cannot be reached > + $memobjcmd = 'sev-snp-guest'; Nit: should be added by a later patch > + } else { > + $memobjcmd = 'sev-guest'; > + } > + $memobjcmd .= ',id=sev0,cbitpos='.$memory_encryption->{cbitpos} > + .',reduced-phys-bits='.$memory_encryption->{'reduced-phys-bits'}; > + if ($memory_encryption->{type} eq 'sev' && $memory_encryption->{policy}) { > + $memobjcmd .= ',policy='.$memory_encryption->{policy} > + } > + push @$devices, '-object' , $memobjcmd; > + # old qemu-Example: -machine 'type=q35+pve0,memory-encryption=sev0' > + # https://fossies.org/linux/qemu/docs/system/confidential-guest-support.rst Nit: I mean the QEMU docs also mention this so no need for that link and why the "old" example? > + 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); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable 2022-11-14 13:06 ` Fiona Ebner @ 2022-11-17 10:50 ` Markus Frank 2022-11-17 11:27 ` Fiona Ebner 0 siblings, 1 reply; 8+ messages in thread From: Markus Frank @ 2022-11-17 10:50 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion Thanks for the feedback. I will send v3 when I was able to test it on an EPYC CPU. On 11/14/22 14:06, Fiona Ebner wrote: > Am 11.11.22 um 15:27 schrieb Markus Frank: >> This Patch is for enabling AMD SEV (Secure Encrypted >> Virtualization) support in QEMU and enabling future >> memory encryption technologies like INTEL MKTME >> (Multi-key Total Memory Encryption) and SEV-SNP. >> >> Config-Example: >> memory_encryption: type=sev,cbitpos=47,policy=0x0001,reduced-phys-bits=1 >> >> reduced-phys-bios, cbitpos and policy correspond to the varibles with the >> same name in qemu. >> >> reduced-phys-bios and cbitpos are system specific and can be read out >> with QMP. If not set by the user, a dummy-vm gets started to read QMP >> for these variables out and save them to config. Afterwards the dummy-vm gets >> stopped. > > Why even allow the user to set them if they are system-specific values? > Or are there multiple possible values on some systems? If not, it should > be a node-specific configuration, rather than a VM-specific one. That > would also only require starting the dummy VM once per node, or we could > require the user to set the values in some node config (of course > mentioning how in the docs :)) > I moved the system specific parameters to the node config: amd_sev: cbitpos=47,reduced-phys-bits=1 >> >> policy can be calculated with the links in comments & description. >> To test SEV-ES (CPU register encryption) the policy should be set >> somewhere between 0x4 and 0x7 or 0xC and 0xF, etc. >> (Bit-2 has to be set 1 (LSB 0 bit numbering)) >> >> SEV needs edk2-OVMF to work. >> >> Signed-off-by: Markus Frank <m.frank@proxmox.com> >> --- >> PVE/QemuServer.pm | 133 ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 133 insertions(+) >> >> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm >> index 513a248..2ea8abd 100644 >> --- a/PVE/QemuServer.pm >> +++ b/PVE/QemuServer.pm >> @@ -175,6 +175,58 @@ my $agent_fmt = { >> }, >> }; >> >> +my $memory_encryption_fmt = { >> + type => { >> + type => 'string', >> + default_key => 1, >> + description => "Memory Encryption Type:" > > Nit: I'd rather have the description be a sentence or two, what it's all > about and add a verbose_description to describe the individual variants. > >> + ." for AMD SEV -> 'memory_encryption: type=sev';" >> + ." for AMD SEV-ES -> use 'sev' and change policy to between 0x4 and 0x7;" >> + ." (Bit-2 has to be set 1 (LSB 0 bit numbering))" > > Nit: better to use 0x0004 and 0x0007, because 0x4 and 0x7 are not valid > values for 'policy' below. > >> + #. "for AMD SEV-SNP -> 'memory_encryption: type=sev-snp'" >> + ." (sev requires edk2-ovmf & sev kernel support by guest operating system &" >> + ." on host: add kernel-parameters 'mem_encrypt=on kvm_amd.sev=1')" >> + ." see https://github.com/AMDESE/AMDSEV &" >> + ." https://documentation.suse.com/sles/15-SP1/html/SLES-amd-sev/index.html", >> + format_description => "qemu-memory-encryption-type", >> + # TODO enable sev-snp option when feature can be tested on 3rd-gen EPYC >> + # https://www.phoronix.com/news/AMD-SEV-SNP-Arrives-Linux-5.19 >> + # enum => ['sev','sev-snp','mktme'], > > Nit: I feel like these comments don't really belong in the patch. Maybe > just add a single high-level TODO comment? The rest should be done by > the patch actually adding sev-snp ;) > removed > Also, the many links might be better left to the documentation patch. > > Is the rest of the format even compatible with Intel's MKTME? I.e. > does/will that also have reduced-phys-bits, 4 policy bits and cbitpos? > If there is some overlap or if we expect to be easily able to translate > certain settings, we can still keep a general memory_encryption_fmt, but > otherwise, it might be better to have completely distinct formats for > Intel and AMD? Yes. Let's separate Intel and AMD. > >> + enum => ['sev'], >> + maxLength => 10, >> + }, >> + 'reduced-phys-bits' => { >> + description => "Number of bits the physical address space is reduced by. System dependent", >> + type => 'integer', >> + default => 1, > > The default is system-dependent and automatically figured out by the > dummy VM. Also the kvm man pages states > >> On EPYC, the value should be 5. > > so why 1? On the EPYC CPUs I have used, the value was 1. And on https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html they also use reduced-phys-bits=1 > >> + optional => 1, >> + minimum => 0, >> + maximum => 100, >> + }, >> + cbitpos => { >> + description => "C-bit: marks if a memory page is protected. System dependent", >> + type => 'integer', >> + default => 47, > > Same here with regards to auto-magic. > >> + optional => 1, >> + minimum => 0, >> + maximum => 100, >> + }, >> + policy => { >> + description => "SEV Guest Policy" >> + . "see Capter 3:" > > Nit: typo > >> + . "https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf" >> + . "& https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html", >> + > > Could have a verbose_description for each bit rather than the links. Or > should we go one step further and add explicit flags for the > relevant-for-us bits instead? Would be more user-friendly, but the > $memory_encryption_fmt would be AMD-specific of course. I made everything AMD-specific: amd_sev: std,es=1,nodbg=1,noks=1 policy will be calculated with these parameters. Also I would set policy bit 3 (nosend) to 1, because migration features for sev are not upstream yet and are attackable. > >> + format_description => "qemu-memory-encryption-policy", >> + type => 'string', >> + default => '0x0000', >> + pattern => '0[xX][0-9a-fA-F]{1,4}', >> + optional => 1, >> + maxLength => 6, >> + }, >> +}; >> +PVE::JSONSchema::register_format('pve-qemu-memory-encryption-fmt', $memory_encryption_fmt); >> + >> my $vga_fmt = { >> type => { >> description => "Select the VGA type.", >> @@ -349,6 +401,12 @@ my $confdesc = { >> minimum => 16, >> default => 512, >> }, >> + memory_encryption => { >> + description => "Memory Encryption", >> + optional => 1, >> + format => 'pve-qemu-memory-encryption-fmt', >> + type => 'string', >> + }, >> balloon => { >> optional => 1, >> type => 'integer', >> @@ -2113,6 +2171,17 @@ sub parse_guest_agent { >> return $res; >> } >> >> +sub parse_memory_encryption { >> + my ($value) = @_; >> + >> + return if !$value; >> + >> + my $res = eval { parse_property_string($memory_encryption_fmt, $value) }; >> + warn $@ if $@; >> + return $res; >> +} > > Why not fail if parsing fails? replaced warn with die > >> + >> + >> sub get_qga_key { >> my ($conf, $key) = @_; >> return undef if !defined($conf->{agent}); >> @@ -4085,6 +4154,70 @@ sub config_to_command { >> } >> push @$machineFlags, "type=${machine_type_min}"; >> >> + # Memory Encryption > > Nit: comment contains no additional information. > >> + my $memory_encryption = parse_memory_encryption($conf->{'memory_encryption'}); >> + >> + # Die if bios is not ovmf > > Nit: Same here. > >> + if ( >> + $memory_encryption->{'type'} >> + && $memory_encryption->{type} eq 'sev' >> + && $conf->{bios} ne 'ovmf' > > I think $conf->{bios} could be undef here? That would cause an ugly Perl > warning. At least other place check for > !defined($conf->{bios}) || $conf->{bios} ne 'ovmf' > >> + ) { >> + die "SEV needs ovmf\n"; > > Nit: maybe a bit too concise of a message, could mention the settings at > least > >> + } >> + > > All of the below should rather live in helper functions, especially the > dummy VM stuff. config_to_command is already much too bloated. > >> + # AMD SEV >> + if ($memory_encryption->{'type'} && $memory_encryption->{type} =~ /(sev|sev-snp)/) { > > Nit: I'd rather have explicit equality testing for the type (or at least > add a leading ^ to the regex). The sev-snp type does not exist yet and > should be added by a later patch. > >> + # Get reduced-phys-bits & cbitpos from QMP, if not set >> + if ( >> + !$memory_encryption->{'reduced-phys-bits'} >> + || !$memory_encryption->{cbitpos} >> + ) { >> + my $fakevmid = -1; >> + my $qemu_cmd = get_command_for_arch($arch); >> + my $pidfile = PVE::QemuServer::Helpers::pidfile_name($fakevmid); >> + my $default_machine = $default_machines->{$arch}; >> + my $cmd = [ >> + $qemu_cmd, >> + '-machine', $default_machine, >> + '-display', 'none', >> + '-chardev', "socket,id=qmp,path=/var/run/qemu-server/$fakevmid.qmp,server=on,wait=off", >> + '-mon', 'chardev=qmp,mode=control', >> + '-pidfile', $pidfile, >> + '-S', '-daemonize' >> + ]; > > Instead of daemonizing, pidfile etc. we could also use --qmp stdio and > pass the commands via stdin like: > {"execute": "qmp_capabilities"} > {"execute": "query-sev-capabilities"} > {"execute": "quit"} > which might be a bit more straight-forward. But maybe we prefer re-using > the existing infrastructure with the fake ID, not sure? What would be the best way to send stdin to "kvm -qmp stdio" here? Not the same way like I would do in shell or yes?: echo '{"execute": "qmp_capabilities"} {"execute": "query-sev-capabilities"} {"execute": "quit"}' | kvm -qmp stdio > >> + my $rc = run_command($cmd, noerr => 1, quiet => 0); >> + die "QEMU flag querying VM exited with code " . $rc . "\n" if $rc; >> + my $res = mon_cmd($fakevmid, 'query-sev-capabilities'); >> + $memory_encryption->{'reduced-phys-bits'} = $res->{'reduced-phys-bits'}; >> + $memory_encryption->{cbitpos} = $res->{cbitpos}; >> + $conf->{'memory_encryption'} = PVE::JSONSchema::print_property_string( >> + $memory_encryption, >> + $memory_encryption_fmt >> + ); >> + PVE::QemuConfig->write_config($vmid, $conf); > > config_to_command should not write the config. Hope that those settings > are truly node-specific and not VM-specific :) > >> + vm_stop(undef, $fakevmid, 1, 1, 10, 0, 1); >> + } >> + # qemu-Example: -object 'sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1'; >> + # see https://www.qemu.org/docs/master/system/i386/amd-memory-encryption.html >> + my $memobjcmd = ""; >> + if ($memory_encryption->{type} eq 'sev-snp') { >> + # future feature: cannot be reached >> + $memobjcmd = 'sev-snp-guest'; > > Nit: should be added by a later patch removed > >> + } else { >> + $memobjcmd = 'sev-guest'; >> + } >> + $memobjcmd .= ',id=sev0,cbitpos='.$memory_encryption->{cbitpos} >> + .',reduced-phys-bits='.$memory_encryption->{'reduced-phys-bits'}; >> + if ($memory_encryption->{type} eq 'sev' && $memory_encryption->{policy}) { >> + $memobjcmd .= ',policy='.$memory_encryption->{policy} >> + } >> + push @$devices, '-object' , $memobjcmd; >> + # old qemu-Example: -machine 'type=q35+pve0,memory-encryption=sev0' >> + # https://fossies.org/linux/qemu/docs/system/confidential-guest-support.rst > > Nit: I mean the QEMU docs also mention this so no need for that link and > why the "old" example? removed > >> + 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); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable 2022-11-17 10:50 ` Markus Frank @ 2022-11-17 11:27 ` Fiona Ebner 0 siblings, 0 replies; 8+ messages in thread From: Fiona Ebner @ 2022-11-17 11:27 UTC (permalink / raw) To: Markus Frank, Proxmox VE development discussion Am 17.11.22 um 11:50 schrieb Markus Frank:>>> @@ -2113,6 +2171,17 @@ sub parse_guest_agent { >>> return $res; >>> } >>> +sub parse_memory_encryption { >>> + my ($value) = @_; >>> + >>> + return if !$value; >>> + >>> + my $res = eval { parse_property_string($memory_encryption_fmt, >>> $value) }; >>> + warn $@ if $@; >>> + return $res; >>> +} >> >> Why not fail if parsing fails? > replaced warn with die You can also just remove the eval to make the error propagate ;) And I guess the whole helper could be replaced at the call side with an inline parse_property_string($memory_encryption_fmt, $value) if $value but no big deal. >>> + # Get reduced-phys-bits & cbitpos from QMP, if not set >>> + if ( >>> + !$memory_encryption->{'reduced-phys-bits'} >>> + || !$memory_encryption->{cbitpos} >>> + ) { >>> + my $fakevmid = -1; >>> + my $qemu_cmd = get_command_for_arch($arch); >>> + my $pidfile = >>> PVE::QemuServer::Helpers::pidfile_name($fakevmid); >>> + my $default_machine = $default_machines->{$arch}; >>> + my $cmd = [ >>> + $qemu_cmd, >>> + '-machine', $default_machine, >>> + '-display', 'none', >>> + '-chardev', >>> "socket,id=qmp,path=/var/run/qemu-server/$fakevmid.qmp,server=on,wait=off", >>> + '-mon', 'chardev=qmp,mode=control', >>> + '-pidfile', $pidfile, >>> + '-S', '-daemonize' >>> + ]; >> >> Instead of daemonizing, pidfile etc. we could also use --qmp stdio and >> pass the commands via stdin like: >> {"execute": "qmp_capabilities"} >> {"execute": "query-sev-capabilities"} >> {"execute": "quit"} >> which might be a bit more straight-forward. But maybe we prefer re-using >> the existing infrastructure with the fake ID, not sure? > What would be the best way to send stdin to "kvm -qmp stdio" here? > > Not the same way like I would do in shell or yes?: > echo '{"execute": "qmp_capabilities"} {"execute": > "query-sev-capabilities"} {"execute": "quit"}' | kvm -qmp stdio The run_command helper can take an "input => $input" parameter. Haven't actually used it myself, but there are existing examples in our code base :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation 2022-11-11 14:27 [pve-devel] [PATCH qemu-server/docs v2 0/2] AMD SEV Markus Frank 2022-11-11 14:27 ` [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable Markus Frank @ 2022-11-11 14:27 ` Markus Frank 2022-11-11 14:48 ` Matthias Heiserer 2022-11-14 13:07 ` Fiona Ebner 1 sibling, 2 replies; 8+ messages in thread From: Markus Frank @ 2022-11-11 14:27 UTC (permalink / raw) To: pve-devel added AMD SEV documentation for "[PATCH qemu-server] QEMU AMD SEV enable" Signed-off-by: Markus Frank <m.frank@proxmox.com> --- qm.adoc | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/qm.adoc b/qm.adoc index e7d0c07..5ba43a2 100644 --- a/qm.adoc +++ b/qm.adoc @@ -598,6 +598,119 @@ 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 +^^^^^^^ + +Memory Encryption per VM using AES-128 Encryption and the AMD Secure Processor. +See https://developer.amd.com/sev/[AMD SEV] + +*Host-Requirements:* + +* AMD EPYC/Ryzen PRO CPU +* configured SEV BIOS settings on 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 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 Host-Machine 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-VM-Requirements:* + +* edk2-OVMF +* advisable to use Q35 +* The guest operating system inside the VM must contain SEV-support +* if there are problems while booting (stops at blank/splash screen or "Guest has not +initialized the display (yet)") try to add virtio-rng and/or set "freeze: 1" +so that you wait a few seconds before you click on *Resume* to boot. + +*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 +* KVM is unsupported when running as an SEV guest +* PCI passthrough is not supported + +Example Configuration: + +---- +# qm set <vmid> -memory_encryption type=sev,cbitpos=47,policy=0x0001,reduced-phys-bits=1 +---- + +*SEV Parameters* + +*type* defines the encryption technology ("type=" is not necessary): +currently-supported: *sev* +and in the future: sev-snp, mktme + +*reduced-phys-bios*, *cbitpos* and *policy* correspond to the variables with the +same name in qemu. + +*reduced-phys-bios* and *cbitpos* are system specific and can be read out +with QMP. If not set, qm starts a dummy-vm to read QMP +for these variables out and saves them to config. + +*policy* can be calculated with +https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf[AMD SEV API Specification Chapter 3] + +To use SEV-ES (CPU register encryption) the *policy* should be set +somewhere between 0x4 and 0x7 or 0xC and 0xF, etc. +(Bit-2 has to be set 1 (LSB 0 bit numbering)) + +*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://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 + +// Commented because cannot be tested without new EPYC-CPU +// AMD SEV-SNP +// ^^^^^^^^^^^ +// * SEV-SNP needs EPYC 7003 "Milan" processors. +// * SEV-SNP should in Kernel 5.19: +// https://www.phoronix.com/scan.php?page=news_item&px=AMD-SEV-SNP-Arrives-Linux-5.19 [[qm_network_device]] Network Device -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation 2022-11-11 14:27 ` [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation Markus Frank @ 2022-11-11 14:48 ` Matthias Heiserer 2022-11-14 13:07 ` Fiona Ebner 1 sibling, 0 replies; 8+ messages in thread From: Matthias Heiserer @ 2022-11-11 14:48 UTC (permalink / raw) To: Proxmox VE development discussion, Markus Frank Inline, I have some suggestions regarding wording and spelling On 11.11.2022 15:27, Markus Frank wrote: > added AMD SEV documentation for "[PATCH qemu-server] QEMU AMD SEV > enable" > > Signed-off-by: Markus Frank <m.frank@proxmox.com> > --- > qm.adoc | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 113 insertions(+) > > diff --git a/qm.adoc b/qm.adoc > index e7d0c07..5ba43a2 100644 > --- a/qm.adoc > +++ b/qm.adoc > @@ -598,6 +598,119 @@ 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 > +^^^^^^^ > + > +Memory Encryption per VM using AES-128 Encryption and the AMD Secure Processor. > +See https://developer.amd.com/sev/[AMD SEV] > + > +*Host-Requirements:* > + > +* AMD EPYC/Ryzen PRO CPU > +* configured SEV BIOS settings on 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 encrypt memory on the "to encrypt" or "encrypted" > +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 Host-Machine search for `sev` in dmesg I think "on the host" would be the best wording > +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-VM-Requirements:* Also here, we spell guest/host without hyphen. Maybe just drop the VM, i.e. "Guest Requirements" > + > +* edk2-OVMF > +* advisable to use Q35 > +* The guest operating system inside the VM must contain SEV-support > +* if there are problems while booting (stops at blank/splash screen or "Guest has not > +initialized the display (yet)") try to add virtio-rng and/or set "freeze: 1" > +so that you wait a few seconds before you click on *Resume* to boot. There's inconsistent capitalization > + > +*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 > +* KVM is unsupported when running as an SEV guest > +* PCI passthrough is not supported > + > +Example Configuration: > + > +---- > +# qm set <vmid> -memory_encryption type=sev,cbitpos=47,policy=0x0001,reduced-phys-bits=1 > +---- > + > +*SEV Parameters* I'd use the same format as e.g. in "10.13.3. Options". Regarding the following sentences: Consistently end them with a dot or without, don't mix. > + > +*type* defines the encryption technology ("type=" is not necessary): > +currently-supported: *sev* > +and in the future: sev-snp, mktme > + > +*reduced-phys-bios*, *cbitpos* and *policy* correspond to the variables with the > +same name in qemu. > + > +*reduced-phys-bios* and *cbitpos* are system specific and can be read out > +with QMP. If not set, qm starts a dummy-vm to read QMP > +for these variables out and saves them to config. > + > +*policy* can be calculated with > +https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf[AMD SEV API Specification Chapter 3] > + > +To use SEV-ES (CPU register encryption) the *policy* should be set > +somewhere between 0x4 and 0x7 or 0xC and 0xF, etc. > +(Bit-2 has to be set 1 (LSB 0 bit numbering)) > + > +*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://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 In case you haven't done so, you could make a snapshot of these pages with archive.org or similarly, so there's a backup if they ever get removed. > + > +// Commented because cannot be tested without new EPYC-CPU > +// AMD SEV-SNP > +// ^^^^^^^^^^^ > +// * SEV-SNP needs EPYC 7003 "Milan" processors. > +// * SEV-SNP should in Kernel 5.19: > +// https://www.phoronix.com/scan.php?page=news_item&px=AMD-SEV-SNP-Arrives-Linux-5.19 > > [[qm_network_device]] > Network Device ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation 2022-11-11 14:27 ` [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation Markus Frank 2022-11-11 14:48 ` Matthias Heiserer @ 2022-11-14 13:07 ` Fiona Ebner 1 sibling, 0 replies; 8+ messages in thread From: Fiona Ebner @ 2022-11-14 13:07 UTC (permalink / raw) To: pve-devel, m.frank Am 11.11.22 um 15:27 schrieb Markus Frank: > +* if there are problems while booting (stops at blank/splash screen or "Guest has not > +initialized the display (yet)") try to add virtio-rng and/or set "freeze: 1" > +so that you wait a few seconds before you click on *Resume* to boot. Doesn't sound very nice from a user perspective. Do you have an idea what's going on there? Can we automatically add some other kvm parameter to make it work (better) when SEV is configured? > + > +*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 What happens if a user attempts a migration or snapshot right now? I think we should cleanly error out before even attempting if it can't work (yet). ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-17 11:27 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-11 14:27 [pve-devel] [PATCH qemu-server/docs v2 0/2] AMD SEV Markus Frank 2022-11-11 14:27 ` [pve-devel] [PATCH qemu-server v2 1/2] QEMU AMD SEV enable Markus Frank 2022-11-14 13:06 ` Fiona Ebner 2022-11-17 10:50 ` Markus Frank 2022-11-17 11:27 ` Fiona Ebner 2022-11-11 14:27 ` [pve-devel] [PATCH docs v2 2/2] added Memory Encryption documentation Markus Frank 2022-11-11 14:48 ` Matthias Heiserer 2022-11-14 13:07 ` Fiona Ebner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox