* [pve-devel] [PATCH qemu-server v9 2/3] config: add AMD SEV support
2024-04-26 9:58 [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID Markus Frank
@ 2024-04-26 9:58 ` Markus Frank
2024-05-17 14:50 ` Fiona Ebner
2024-04-26 9:58 ` [pve-devel] [PATCH docs v9 3/3] add AMD SEV documentation Markus Frank
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Markus Frank @ 2024-04-26 9:58 UTC (permalink / raw)
To: pve-devel
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-bios & 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.service on boot 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 nodbg
& noks. 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>
---
v8:
* renamed "params" to "capabilities" or "caps"
* renamed "nodbg" to "no-debug" and "noks" to "no-key-sharing"
* untainted json_text as it prevented a SEV VM from starting via GUI
v7:
* adjustments for the changes made in the query-machine-params
C program
v6:
* rebase on master
* removed unused $sev_node_fmt object
v5:
* parse /run/amd-sev-params for hardware parameters
* removed NodeConfig dependency
* only disallow live-migration and snapshots with vmstate
-> allow offline migration and snapshots without vmstate
v4:
* reduced lines of code
* added text that SEV-ES is experimental
PVE/API2/Qemu.pm | 11 +++++++
PVE/QemuMigrate.pm | 4 +++
PVE/QemuServer.pm | 79 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 94 insertions(+)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2a349c8..c29809d 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4512,6 +4512,11 @@ __PACKAGE__->register_method({
push $local_resources->@*, "clipboard=vnc";
}
+ # do not allow live migration with AMD SEV enabled
+ if ($res->{running} && $vmconf->{amd_sev}) {
+ push $local_resources->@*, "amd_sev";
+ }
+
# if vm is not running, return target nodes where local storage/mapped devices are available
# for offline migration
if (!$res->{running}) {
@@ -5192,6 +5197,12 @@ __PACKAGE__->register_method({
die "unable to use snapshot name 'pending' (reserved name)\n"
if lc($snapname) eq 'pending';
+ my $conf = PVE::QemuConfig->load_config($vmid);
+ if ($param->{vmstate} && $conf->{amd_sev}) {
+ die "Snapshots that include memory are not supported while memory"
+ ." is encrypted by AMD SEV.\n"
+ }
+
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 8d9b35a..340402a 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -260,6 +260,10 @@ sub prepare {
die "VMs with 'clipboard' set to 'vnc' are not live migratable!\n";
}
+ if ($running && $conf->{'amd_sev'}) {
+ die "cannot live-migrate VM when AMD SEV is enabled.\n";
+ }
+
my $vollist = PVE::QemuServer::get_vm_volumes($conf);
my $storages = {};
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 82e7d6a..3417a86 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -177,6 +177,37 @@ my $agent_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);
+
my $vga_fmt = {
type => {
description => "Select the VGA type.",
@@ -358,6 +389,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',
@@ -4091,6 +4128,39 @@ sub config_to_command {
}
}
+ if ($conf->{amd_sev}) {
+ if ($conf->{bios} && $conf->{bios} ne 'ovmf') {
+ die "For using SEV you need to change your guest bios to ovmf.\n";
+ }
+
+ my $amd_sev_conf = 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'};
+
+ my $policy = 0b0;
+ $policy += 0b1 if ($amd_sev_conf->{'no-debug'});
+ $policy += 0b10 if ($amd_sev_conf->{'no-key-sharing'});
+ $policy += 0b100 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'});
+
+ push @$devices, '-object' , $sev_mem_object;
+ 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);
@@ -4134,6 +4204,15 @@ sub check_rng_source {
}
}
+sub get_hw_capabilities {
+ # Get reduced-phys-bits & cbitpos from host-hw-capabilities.json
+ my $filename = '/run/qemu-server/host-hw-capabilities.json';
+ 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 spice_port {
my ($vmid) = @_;
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v9 2/3] config: add AMD SEV support
2024-04-26 9:58 ` [pve-devel] [PATCH qemu-server v9 2/3] config: add AMD SEV support Markus Frank
@ 2024-05-17 14:50 ` Fiona Ebner
0 siblings, 0 replies; 7+ messages in thread
From: Fiona Ebner @ 2024-05-17 14:50 UTC (permalink / raw)
To: Proxmox VE development discussion, Markus Frank
Am 26.04.24 um 11:58 schrieb 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-bios & cbitpos correspond to the variables
> with the same name in qemu.
s/reduced-phys-bios/reduced-phys-bits/
s/qemu/QEMU/
>
> kernel-hashes=1 adds kernel-hashes to enable measured linux kernel
The second time it should be "kernel hashes" instead of "kernel-hashes"
> launch since it is per default off for backward compatibility.
>
---snip---
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 2a349c8..c29809d 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -4512,6 +4512,11 @@ __PACKAGE__->register_method({
> push $local_resources->@*, "clipboard=vnc";
> }
>
> + # do not allow live migration with AMD SEV enabled
The comment does not add any information (it's already obvious from the
code). Maybe mention the migration attack instead?
> + if ($res->{running} && $vmconf->{amd_sev}) {
> + push $local_resources->@*, "amd_sev";
> + }
> +
> # if vm is not running, return target nodes where local storage/mapped devices are available
> # for offline migration
> if (!$res->{running}) {
> @@ -5192,6 +5197,12 @@ __PACKAGE__->register_method({
> die "unable to use snapshot name 'pending' (reserved name)\n"
> if lc($snapname) eq 'pending';
>
> + my $conf = PVE::QemuConfig->load_config($vmid);
> + if ($param->{vmstate} && $conf->{amd_sev}) {
> + die "Snapshots that include memory are not supported while memory"
> + ." is encrypted by AMD SEV.\n"
> + }
> +
> my $realcmd = sub {
> PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: $snapname");
> PVE::QemuConfig->snapshot_create($vmid, $snapname, $param->{vmstate},
What about hibernate? That uses the very same mechanism under the hood
(savevm-start QMP command), so that should be prevented as well, right?
A helper would be good for the common checks. Could be called
check_non_migratable_resources() and start out with the checks for
clipboard and hostpci devices (currently present for hibernation). Your
series could then add the AMD-SEV check.
The helper can then be called by check_local_resources() (although then
we should avoid adding hostpci devices twice to the list of local
resources), as well as the snapshot and hibernation API calls (when
state is included).
Off-topic: I noticed the clipboard check is also missing from snapshot
and hibernate API calls, but it's not 100% clear to me if they should be
added right now or if we should wait for a (minor) release, see:
https://lists.proxmox.com/pipermail/pve-devel/2024-May/063896.html
So you could start out with just the AMD-SEV check until we decide to
enforce the VNC clipboard check for snapshot/hibernate and how to
properly avoid duplicates with the hostpci check. (A TODO comment for
those would be good).
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 8d9b35a..340402a 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -260,6 +260,10 @@ sub prepare {
> die "VMs with 'clipboard' set to 'vnc' are not live migratable!\n";
> }
>
> + if ($running && $conf->{'amd_sev'}) {
> + die "cannot live-migrate VM when AMD SEV is enabled.\n";
> + }
Then you could also re-use the helper here :)
> +
> my $vollist = PVE::QemuServer::get_vm_volumes($conf);
>
> my $storages = {};
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 82e7d6a..3417a86 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -177,6 +177,37 @@ my $agent_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",
You don't need a format_description if there is an enum.
> + enum => ['std', 'es'],
> + maxLength => 3,
You don't need a maxLenght if there is an enum.
> + },
> + '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);
> +
> my $vga_fmt = {
> type => {
> description => "Select the VGA type.",
> @@ -358,6 +389,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',
> @@ -4091,6 +4128,39 @@ sub config_to_command {
> }
> }
>
> + if ($conf->{amd_sev}) {
Please factor this out into a helper function instead of adding a big
block to config_to_command. It's already huge enough.
> + if ($conf->{bios} && $conf->{bios} ne 'ovmf') {
> + die "For using SEV you need to change your guest bios to ovmf.\n";
s/bios/BIOS/
> + }
> +
> + my $amd_sev_conf = parse_property_string($sev_fmt, $conf->{amd_sev});
> + my $sev_hw_caps = get_hw_capabilities()->{'amd-sev'};
Maybe error out if the parsed caps are not a hash like we expect (before
accessing 'amd-sev')? And error out if the keys we expect do not exist
in the result.
> +
> + 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'};
> +
> + my $policy = 0b0;
> + $policy += 0b1 if ($amd_sev_conf->{'no-debug'});
> + $policy += 0b10 if ($amd_sev_conf->{'no-key-sharing'});
> + $policy += 0b100 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'});
Style nit: superfluous parentheses for post-if
> +
> + push @$devices, '-object' , $sev_mem_object;
> + 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);
> @@ -4134,6 +4204,15 @@ sub check_rng_source {
> }
> }
>
> +sub get_hw_capabilities {
> + # Get reduced-phys-bits & cbitpos from host-hw-capabilities.json
> + my $filename = '/run/qemu-server/host-hw-capabilities.json';
> + my $json_text = PVE::Tools::file_get_contents($filename);
> + ($json_text) = $json_text =~ /(.*)/; # untaint json text> + my $hw_capabilities = decode_json($json_text);
Maybe eval and use a nice error message/prefix if the decoding dies here?
> + return $hw_capabilities;
> +}
> +
> sub spice_port {
> my ($vmid) = @_;
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH docs v9 3/3] add AMD SEV documentation
2024-04-26 9:58 [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID Markus Frank
2024-04-26 9:58 ` [pve-devel] [PATCH qemu-server v9 2/3] config: add AMD SEV support Markus Frank
@ 2024-04-26 9:58 ` Markus Frank
2024-05-07 9:25 ` [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID Filip Schauer
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Markus Frank @ 2024-04-26 9:58 UTC (permalink / raw)
To: pve-devel
add documentation for the "[PATCH qemu-server] config: add AMD SEV
support" patch.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
v8:
* adjust changed parameter names in examples
v5:
* removed NodeConfig part
v4:
* added text that SEV-ES is experimental
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID
2024-04-26 9:58 [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID Markus Frank
2024-04-26 9:58 ` [pve-devel] [PATCH qemu-server v9 2/3] config: add AMD SEV support Markus Frank
2024-04-26 9:58 ` [pve-devel] [PATCH docs v9 3/3] add AMD SEV documentation Markus Frank
@ 2024-05-07 9:25 ` Filip Schauer
2024-05-17 12:45 ` Fiona Ebner
2024-05-17 14:52 ` Fiona Ebner
4 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2024-05-07 9:25 UTC (permalink / raw)
To: pve-devel
Ran this on an Intel(R) Core(TM) i7-7700K CPU at Markus' request to see
how this behaves on an Intel processor. This results in the following
being written to /run/qemu-server/host-hw-capabilities.json:
{ "amd-sev": { "cbitpos": 0, "reduced-phys-bits": 0, "sev-support":
false, "sev-support-es": false, "sev-support-snp": false } }
PS: The sys/types.h include can be omitted. Otherwise the code looks
fine to me.
On 26/04/2024 11:58, Markus Frank wrote:
> Implement a systemd service that runs 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/hw-params.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>
> ---
> v9:
> * added directory existance check
> * print error messages
>
> v8:
> * renamed query-machine-params to query-machine-capabilities
>
> v7:
> * renamed amd-sev-support to query-machine-params
> * mv /run/amd-sev-params to /run/qemu-server/hw-params.json
> * add "mkdir /run/qemu-server" to ensure that the directory exists
> * moved json content to amd-sev property inside a bigger json
> so that other hardware parameters could also be read at boot time and
> included in this json file.
>
>
> Makefile | 1 +
> query-machine-capabilities/Makefile | 21 ++++++
> .../query-machine-capabilities.c | 71 +++++++++++++++++++
> .../query-machine-capabilities.service | 12 ++++
> 4 files changed, 105 insertions(+)
> create mode 100644 query-machine-capabilities/Makefile
> create mode 100644 query-machine-capabilities/query-machine-capabilities.c
> create mode 100644 query-machine-capabilities/query-machine-capabilities.service
>
> 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..c5f6348
> --- /dev/null
> +++ b/query-machine-capabilities/Makefile
> @@ -0,0 +1,21 @@
> +DESTDIR=
> +PREFIX=/usr
> +SBINDIR=${PREFIX}/libexec/qemu-server
> +SERVICEDIR=/lib/systemd/system
> +
> +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}/${SBINDIR}
> + install -d ${DESTDIR}${SERVICEDIR}
> + install -m 0644 query-machine-capabilities.service ${DESTDIR}${SERVICEDIR}
> + install -m 0755 query-machine-capabilities ${DESTDIR}${SBINDIR}
> +
> +.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..7b8c59f
> --- /dev/null
> +++ b/query-machine-capabilities/query-machine-capabilities.c
> @@ -0,0 +1,71 @@
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <sys/stat.h>
> +#include <sys/types.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 stats = stat(path, &statbuf);
> + if (stats == 0 && S_ISDIR(statbuf.st_mode)) {
> + printf("Directory %s already exists.\n", path);
> + } else if (errno == ENOENT) {
> + printf("%s does not exist. Creating directory.\n", path);
> + 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;
> + }
> +
> + 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"
> + );
> +
> + fclose(file);
> + return 0;
> +}
> diff --git a/query-machine-capabilities/query-machine-capabilities.service b/query-machine-capabilities/query-machine-capabilities.service
> new file mode 100644
> index 0000000..f926074
> --- /dev/null
> +++ b/query-machine-capabilities/query-machine-capabilities.service
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=read AMD SEV parameters
> +RequiresMountsFor=/run
> +Before=pve-ha-lrm.service
> +Before=pve-guests.service
> +
> +[Service]
> +ExecStart=/usr/libexec/qemu-server/query-machine-capabilities
> +Type=oneshot
> +
> +[Install]
> +WantedBy=multi-user.target
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID
2024-04-26 9:58 [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID Markus Frank
` (2 preceding siblings ...)
2024-05-07 9:25 ` [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID Filip Schauer
@ 2024-05-17 12:45 ` Fiona Ebner
2024-05-17 14:52 ` Fiona Ebner
4 siblings, 0 replies; 7+ messages in thread
From: Fiona Ebner @ 2024-05-17 12:45 UTC (permalink / raw)
To: Proxmox VE development discussion, Markus Frank
Am 26.04.24 um 11:58 schrieb Markus Frank:
> Implement a systemd service that runs 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/hw-params.json.
>
In the code, it's /run/qemu-server/host-hw-capabilities.json
> This programm can also be used to read and save other hardware
> information at boot time.
>
Nit: s/programm/program/
Question is if it should live in a more central place/package then? E.g.
what if the next time we need to query a capability, it's needed by
pve-container? But I guess we can still move it later when the need arises.
> diff --git a/query-machine-capabilities/Makefile b/query-machine-capabilities/Makefile
> new file mode 100644
> index 0000000..c5f6348
> --- /dev/null
> +++ b/query-machine-capabilities/Makefile
> @@ -0,0 +1,21 @@
> +DESTDIR=
> +PREFIX=/usr
> +SBINDIR=${PREFIX}/libexec/qemu-server
Why call it SBINDIR?
[...]
> +
> + const char *path = "/run/qemu-server/";
> + // Check that the directory exists and create it if it does not.
> + struct stat statbuf;
> + int stats = stat(path, &statbuf);
A bit confused about the name "stats", because this is just the success
indicator. Maybe "ret" or something that doesn't sound like it's the
actual stats. You could also branch on the function call itself and on
success once more with S_ISDR, to not miss the "success, but not a
directory" case, see below.
> + if (stats == 0 && S_ISDIR(statbuf.st_mode)) {
> + printf("Directory %s already exists.\n", path);
> + } else if (errno == ENOENT) {
> + printf("%s does not exist. Creating directory.\n", path);
Not sure if the above two messages are worth logging, but either way is
fine.
> + 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));
Unlikely, but if errno is 0 (can be when the file exists but is not a
directory for example) then strerror(errno) will be "Success" and result
in a confusing error message.
> + 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;
> + }
> +
> + 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"
> + );
> +
> + fclose(file);
Maybe check for errors for fprintf and fclose?
> + return 0;
> +}
> diff --git a/query-machine-capabilities/query-machine-capabilities.service b/query-machine-capabilities/query-machine-capabilities.service
> new file mode 100644
> index 0000000..f926074
> --- /dev/null
> +++ b/query-machine-capabilities/query-machine-capabilities.service
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=read AMD SEV parameters
Should also be updated to reflect the more general (future) use case.
> +RequiresMountsFor=/run
> +Before=pve-ha-lrm.service
> +Before=pve-guests.service
> +
> +[Service]
> +ExecStart=/usr/libexec/qemu-server/query-machine-capabilities
> +Type=oneshot
> +
> +[Install]
> +WantedBy=multi-user.target
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID
2024-04-26 9:58 [pve-devel] [PATCH qemu-server v9 1/3] add C program to get hardware capabilities from CPUID Markus Frank
` (3 preceding siblings ...)
2024-05-17 12:45 ` Fiona Ebner
@ 2024-05-17 14:52 ` Fiona Ebner
4 siblings, 0 replies; 7+ messages in thread
From: Fiona Ebner @ 2024-05-17 14:52 UTC (permalink / raw)
To: Proxmox VE development discussion, Markus Frank
Ah, sorry, only just noticed that there is a v10 already. But maybe my
comments still apply there.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread