[pve-devel] [PATCH qemu-server 02/31] introduce OVMF module

From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server 02/31] introduce OVMF module
Date: Wed, 25 Jun 2025 17:56:25 +0200	[thread overview]
Message-ID: <20250625155751.268047-3-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250625155751.268047-1-f.ebner@proxmox.com>

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes since previous series:
* keep get_efivars_size() as a wrapper in QemuServer module
* keep early check for CPU bitness in QemuServer module

 src/PVE/API2/Qemu.pm             |   3 +-
 src/PVE/QemuServer.pm            | 155 +++--------------------------
 src/PVE/QemuServer/Makefile      |   1 +
 src/PVE/QemuServer/OVMF.pm       | 166 +++++++++++++++++++++++++++++++
 src/test/MigrationTest/Shared.pm |   4 +
 5 files changed, 185 insertions(+), 144 deletions(-)
 create mode 100644 src/PVE/QemuServer/OVMF.pm

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index ce6f362d..6830ea1e 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -36,6 +36,7 @@ use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
 use PVE::QemuServer::MetaInfo;
+use PVE::QemuServer::OVMF;
 use PVE::QemuServer::PCI;
 use PVE::QemuServer::QMPHelpers;
 use PVE::QemuServer::RNG;
@@ -612,7 +613,7 @@ my sub create_disks : prototype($$$$$$$$$$$) {
                         "SEV-SNP uses consolidated read-only firmware and does not require an EFI disk\n"
                         if $amd_sev_type && $amd_sev_type eq 'snp';
 
-                    ($volid, $size) = PVE::QemuServer::create_efidisk(
+                    ($volid, $size) = PVE::QemuServer::OVMF::create_efidisk(
                         $storecfg, $storeid, $vmid, $fmt, $arch, $disk, $smm, $amd_sev_type,
                     );
                 } elsif ($ds eq 'tpmstate0') {
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 6926182b..bb10f116 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -71,6 +71,7 @@ use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
 use PVE::QemuServer::MetaInfo;
 use PVE::QemuServer::Monitor qw(mon_cmd);
+use PVE::QemuServer::OVMF;
 use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr print_pcie_root_port parse_hostpci);
 use PVE::QemuServer::QemuImage;
 use PVE::QemuServer::QMPHelpers qw(qemu_deviceadd qemu_devicedel qemu_objectadd qemu_objectdel);
@@ -98,41 +99,6 @@ my sub vm_is_ha_managed {
     return PVE::HA::Config::vm_is_ha_managed($vmid);
 }
 
-my $EDK2_FW_BASE = '/usr/share/pve-edk2-firmware/';
-my $OVMF = {
-    x86_64 => {
-        '4m-no-smm' => [
-            "$EDK2_FW_BASE/OVMF_CODE_4M.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.fd",
-        ],
-        '4m-no-smm-ms' => [
-            "$EDK2_FW_BASE/OVMF_CODE_4M.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
-        ],
-        '4m' => [
-            "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.fd",
-        ],
-        '4m-ms' => [
-            "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
-        ],
-        '4m-sev' => [
-            "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd", "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd",
-        ],
-        '4m-snp' => [
-            "$EDK2_FW_BASE/OVMF_CVM_4M.fd",
-        ],
-        # FIXME: These are legacy 2MB-sized images that modern OVMF doesn't supports to build
-        # anymore. how can we deperacate this sanely without breaking existing instances, or using
-        # older backups and snapshot?
-        default => [
-            "$EDK2_FW_BASE/OVMF_CODE.fd", "$EDK2_FW_BASE/OVMF_VARS.fd",
-        ],
-    },
-    aarch64 => {
-        default => [
-            "$EDK2_FW_BASE/AAVMF_CODE.fd", "$EDK2_FW_BASE/AAVMF_VARS.fd",
-        ],
-    },
-};
-
 my $cpuinfo = PVE::ProcFSTools::read_cpuinfo();
 
 # Note about locking: we use flock on the config file protect against concurrent actions.
@@ -3293,36 +3259,6 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-sub get_ovmf_files($$$$) {
-    my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
-
-    my $types = $OVMF->{$arch}
-        or die "no OVMF images known for architecture '$arch'\n";
-
-    my $type = 'default';
-    if ($arch eq 'x86_64') {
-        if ($amd_sev_type && $amd_sev_type eq 'snp') {
-            $type = "4m-snp";
-            my ($ovmf) = $types->{$type}->@*;
-            die "EFI base image '$ovmf' not found\n" if !-f $ovmf;
-            return ($ovmf);
-        } elsif ($amd_sev_type) {
-            $type = "4m-sev";
-        } elsif (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
-            $type = $smm ? "4m" : "4m-no-smm";
-            $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
-        } else {
-            # TODO: log_warn about use of legacy images for x86_64 with Promxox VE 9
-        }
-    }
-
-    my ($ovmf_code, $ovmf_vars) = $types->{$type}->@*;
-    die "EFI base image '$ovmf_code' not found\n" if !-f $ovmf_code;
-    die "EFI vars image '$ovmf_vars' not found\n" if !-f $ovmf_vars;
-
-    return ($ovmf_code, $ovmf_vars);
-}
-
 # To use query_supported_cpu_flags and query_understood_cpu_flags to get flags
 # to use in a QEMU command line (-cpu element), first array_intersect the result
 # of query_supported_ with query_understood_. This is necessary because:
@@ -3464,49 +3400,6 @@ my sub should_disable_smm {
         && $vga->{type} =~ m/^(serial\d+|none)$/;
 }
 
-my sub print_ovmf_drive_commandlines {
-    my ($conf, $storecfg, $vmid, $hw_info, $version_guard) = @_;
-
-    my ($amd_sev_type, $arch, $q35) = $hw_info->@{qw(amd-sev-type arch q35)};
-
-    my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
-
-    die "Attempting to configure SEV-SNP with pflash devices instead of using `-bios`\n"
-        if $amd_sev_type && $amd_sev_type eq 'snp';
-
-    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35, $amd_sev_type);
-
-    my $var_drive_str = "if=pflash,unit=1,id=drive-efidisk0";
-    if ($d) {
-        my ($storeid, $volname) = PVE::Storage::parse_volume_id($d->{file}, 1);
-        my ($path, $format) = $d->@{ 'file', 'format' };
-        if ($storeid) {
-            $path = PVE::Storage::path($storecfg, $d->{file});
-            $format //= checked_volume_format($storecfg, $d->{file});
-        } elsif (!defined($format)) {
-            die "efidisk format must be specified\n";
-        }
-        # SPI flash does lots of read-modify-write OPs, without writeback this gets really slow #3329
-        if ($path =~ m/^rbd:/) {
-            $var_drive_str .= ',cache=writeback';
-            $path .= ':rbd_cache_policy=writeback'; # avoid write-around, we *need* to cache writes too
-        }
-        $var_drive_str .= ",format=$format,file=$path";
-
-        $var_drive_str .= ",size=" . (-s $ovmf_vars)
-            if $format eq 'raw' && $version_guard->(4, 1, 2);
-        $var_drive_str .= ',readonly=on' if drive_is_read_only($conf, $d);
-    } else {
-        log_warn("no efidisk configured! Using temporary efivars disk.");
-        my $path = "/tmp/$vmid-ovmf.fd";
-        PVE::Tools::file_copy($ovmf_vars, $path, -s $ovmf_vars);
-        $var_drive_str .= ",format=raw,file=$path";
-        $var_drive_str .= ",size=" . (-s $ovmf_vars) if $version_guard->(4, 1, 2);
-    }
-
-    return ("if=pflash,unit=0,format=raw,readonly=on,file=$ovmf_code", $var_drive_str);
-}
-
 my sub get_vga_properties {
     my ($conf, $arch, $machine_version, $winversion) = @_;
 
@@ -3684,23 +3577,15 @@ sub config_to_command {
         die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n"
             if !$forcecpu && get_cpu_bitness($conf->{cpu}, $arch) == 32;
 
-        my $amd_sev_type = get_amd_sev_type($conf);
-        if ($amd_sev_type && $amd_sev_type eq 'snp') {
-            if (defined($conf->{efidisk0})) {
-                log_warn("EFI disks are not supported with SEV-SNP and will be ignored");
-            }
-            push $cmd->@*, '-bios', get_ovmf_files($arch, undef, undef, $amd_sev_type);
-        } else {
-            my $hw_info = {
-                'amd-sev-type' => $amd_sev_type,
-                arch => $arch,
-                q35 => $q35,
-            };
-            my ($code_drive_str, $var_drive_str) =
-                print_ovmf_drive_commandlines($conf, $storecfg, $vmid, $hw_info, $version_guard);
-            push $cmd->@*, '-drive', $code_drive_str;
-            push $cmd->@*, '-drive', $var_drive_str;
-        }
+        my $hw_info = {
+            'amd-sev-type' => get_amd_sev_type($conf),
+            arch => $arch,
+            q35 => $q35,
+        };
+        my $ovmf_cmd = PVE::QemuServer::OVMF::print_ovmf_commandline(
+            $conf, $storecfg, $vmid, $hw_info, $version_guard,
+        );
+        push $cmd->@*, $ovmf_cmd->@*;
     }
 
     if ($q35) { # tell QEMU to load q35 config early
@@ -8866,8 +8751,8 @@ sub get_efivars_size {
     $efidisk //= $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
     my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
     my $amd_sev_type = get_amd_sev_type($conf);
-    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm, $amd_sev_type);
-    return -s $ovmf_vars;
+
+    return PVE::QemuServer::OVMF::get_efivars_size($arch, $efidisk, $smm, $amd_sev_type);
 }
 
 sub update_efidisk_size {
@@ -8890,22 +8775,6 @@ sub update_tpmstate_size {
     $conf->{tpmstate0} = print_drive($disk);
 }
 
-sub create_efidisk($$$$$$$$) {
-    my ($storecfg, $storeid, $vmid, $fmt, $arch, $efidisk, $smm, $amd_sev_type) = @_;
-
-    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm, $amd_sev_type);
-
-    my $vars_size_b = -s $ovmf_vars;
-    my $vars_size = PVE::Tools::convert_size($vars_size_b, 'b' => 'kb');
-    my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, undef, $vars_size);
-    PVE::Storage::activate_volumes($storecfg, [$volid]);
-
-    PVE::QemuServer::QemuImage::convert($ovmf_vars, $volid, $vars_size_b);
-    my $size = PVE::Storage::volume_size_info($storecfg, $volid, 3);
-
-    return ($volid, $size / 1024);
-}
-
 sub vm_iothreads_list {
     my ($vmid) = @_;
 
diff --git a/src/PVE/QemuServer/Makefile b/src/PVE/QemuServer/Makefile
index a34ec83b..dd6fe505 100644
--- a/src/PVE/QemuServer/Makefile
+++ b/src/PVE/QemuServer/Makefile
@@ -14,6 +14,7 @@ SOURCES=Agent.pm	\
 	Memory.pm	\
 	MetaInfo.pm	\
 	Monitor.pm	\
+	OVMF.pm		\
 	PCI.pm		\
 	QemuImage.pm	\
 	QMPHelpers.pm	\
diff --git a/src/PVE/QemuServer/OVMF.pm b/src/PVE/QemuServer/OVMF.pm
new file mode 100644
index 00000000..66da21ce
--- /dev/null
+++ b/src/PVE/QemuServer/OVMF.pm
@@ -0,0 +1,166 @@
+package PVE::QemuServer::OVMF;
+
+use strict;
+use warnings;
+
+use PVE::RESTEnvironment qw(log_warn);
+use PVE::Storage;
+use PVE::Tools;
+
+use PVE::QemuServer::Drive qw(checked_volume_format drive_is_read_only parse_drive print_drive);
+use PVE::QemuServer::QemuImage;
+
+my $EDK2_FW_BASE = '/usr/share/pve-edk2-firmware/';
+my $OVMF = {
+    x86_64 => {
+        '4m-no-smm' => [
+            "$EDK2_FW_BASE/OVMF_CODE_4M.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.fd",
+        ],
+        '4m-no-smm-ms' => [
+            "$EDK2_FW_BASE/OVMF_CODE_4M.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
+        ],
+        '4m' => [
+            "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.fd",
+        ],
+        '4m-ms' => [
+            "$EDK2_FW_BASE/OVMF_CODE_4M.secboot.fd", "$EDK2_FW_BASE/OVMF_VARS_4M.ms.fd",
+        ],
+        '4m-sev' => [
+            "$EDK2_FW_BASE/OVMF_CVM_CODE_4M.fd", "$EDK2_FW_BASE/OVMF_CVM_VARS_4M.fd",
+        ],
+        '4m-snp' => [
+            "$EDK2_FW_BASE/OVMF_CVM_4M.fd",
+        ],
+        # FIXME: These are legacy 2MB-sized images that modern OVMF doesn't supports to build
+        # anymore. how can we deperacate this sanely without breaking existing instances, or using
+        # older backups and snapshot?
+        default => [
+            "$EDK2_FW_BASE/OVMF_CODE.fd", "$EDK2_FW_BASE/OVMF_VARS.fd",
+        ],
+    },
+    aarch64 => {
+        default => [
+            "$EDK2_FW_BASE/AAVMF_CODE.fd", "$EDK2_FW_BASE/AAVMF_VARS.fd",
+        ],
+    },
+};
+
+my sub get_ovmf_files($$$$) {
+    my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
+
+    my $types = $OVMF->{$arch}
+        or die "no OVMF images known for architecture '$arch'\n";
+
+    my $type = 'default';
+    if ($arch eq 'x86_64') {
+        if ($amd_sev_type && $amd_sev_type eq 'snp') {
+            $type = "4m-snp";
+            my ($ovmf) = $types->{$type}->@*;
+            die "EFI base image '$ovmf' not found\n" if !-f $ovmf;
+            return ($ovmf);
+        } elsif ($amd_sev_type) {
+            $type = "4m-sev";
+        } elsif (defined($efidisk->{efitype}) && $efidisk->{efitype} eq '4m') {
+            $type = $smm ? "4m" : "4m-no-smm";
+            $type .= '-ms' if $efidisk->{'pre-enrolled-keys'};
+        } else {
+            # TODO: log_warn about use of legacy images for x86_64 with Promxox VE 9
+        }
+    }
+
+    my ($ovmf_code, $ovmf_vars) = $types->{$type}->@*;
+    die "EFI base image '$ovmf_code' not found\n" if !-f $ovmf_code;
+    die "EFI vars image '$ovmf_vars' not found\n" if !-f $ovmf_vars;
+
+    return ($ovmf_code, $ovmf_vars);
+}
+
+my sub print_ovmf_drive_commandlines {
+    my ($conf, $storecfg, $vmid, $hw_info, $version_guard) = @_;
+
+    my ($amd_sev_type, $arch, $q35) = $hw_info->@{qw(amd-sev-type arch q35)};
+
+    my $d = $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
+
+    die "Attempting to configure SEV-SNP with pflash devices instead of using `-bios`\n"
+        if $amd_sev_type && $amd_sev_type eq 'snp';
+
+    my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35, $amd_sev_type);
+
+    my $var_drive_str = "if=pflash,unit=1,id=drive-efidisk0";
+    if ($d) {
+        my ($storeid, $volname) = PVE::Storage::parse_volume_id($d->{file}, 1);
+        my ($path, $format) = $d->@{ 'file', 'format' };
+        if ($storeid) {
+            $path = PVE::Storage::path($storecfg, $d->{file});
+            $format //= checked_volume_format($storecfg, $d->{file});
+        } elsif (!defined($format)) {
+            die "efidisk format must be specified\n";
+        }
+        # SPI flash does lots of read-modify-write OPs, without writeback this gets really slow #3329
+        if ($path =~ m/^rbd:/) {
+            $var_drive_str .= ',cache=writeback';
+            $path .= ':rbd_cache_policy=writeback'; # avoid write-around, we *need* to cache writes too
+        }
+        $var_drive_str .= ",format=$format,file=$path";
+
+        $var_drive_str .= ",size=" . (-s $ovmf_vars)
+            if $format eq 'raw' && $version_guard->(4, 1, 2);
+        $var_drive_str .= ',readonly=on' if drive_is_read_only($conf, $d);
+    } else {
+        log_warn("no efidisk configured! Using temporary efivars disk.");
+        my $path = "/tmp/$vmid-ovmf.fd";
+        PVE::Tools::file_copy($ovmf_vars, $path, -s $ovmf_vars);
+        $var_drive_str .= ",format=raw,file=$path";
+        $var_drive_str .= ",size=" . (-s $ovmf_vars) if $version_guard->(4, 1, 2);
+    }
+
+    return ("if=pflash,unit=0,format=raw,readonly=on,file=$ovmf_code", $var_drive_str);
+}
+
+sub get_efivars_size {
+    my ($arch, $efidisk, $smm, $amd_sev_type) = @_;
+
+    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm, $amd_sev_type);
+    return -s $ovmf_vars;
+}
+
+sub create_efidisk($$$$$$$$) {
+    my ($storecfg, $storeid, $vmid, $fmt, $arch, $efidisk, $smm, $amd_sev_type) = @_;
+
+    my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm, $amd_sev_type);
+
+    my $vars_size_b = -s $ovmf_vars;
+    my $vars_size = PVE::Tools::convert_size($vars_size_b, 'b' => 'kb');
+    my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, undef, $vars_size);
+    PVE::Storage::activate_volumes($storecfg, [$volid]);
+
+    PVE::QemuServer::QemuImage::convert($ovmf_vars, $volid, $vars_size_b);
+    my $size = PVE::Storage::volume_size_info($storecfg, $volid, 3);
+
+    return ($volid, $size / 1024);
+}
+
+sub print_ovmf_commandline {
+    my ($conf, $storecfg, $vmid, $hw_info, $version_guard) = @_;
+
+    my $amd_sev_type = $hw_info->{'amd-sev-type'};
+
+    my $cmd = [];
+
+    if ($amd_sev_type && $amd_sev_type eq 'snp') {
+        if (defined($conf->{efidisk0})) {
+            log_warn("EFI disks are not supported with SEV-SNP and will be ignored");
+        }
+        push $cmd->@*, '-bios', get_ovmf_files($hw_info->{arch}, undef, undef, $amd_sev_type);
+    } else {
+        my ($code_drive_str, $var_drive_str) =
+            print_ovmf_drive_commandlines($conf, $storecfg, $vmid, $hw_info, $version_guard);
+        push $cmd->@*, '-drive', $code_drive_str;
+        push $cmd->@*, '-drive', $var_drive_str;
+    }
+
+    return $cmd;
+}
+
+1;
diff --git a/src/test/MigrationTest/Shared.pm b/src/test/MigrationTest/Shared.pm
index 0b1ac7a0..e29cd1df 100644
--- a/src/test/MigrationTest/Shared.pm
+++ b/src/test/MigrationTest/Shared.pm
@@ -150,6 +150,10 @@ $qemu_server_module->mock(
     vm_stop_cleanup => sub {
         return;
     },
+);
+
+our $qemu_server_ovmf_module = Test::MockModule->new("PVE::QemuServer::OVMF");
+$qemu_server_ovmf_module->mock(
     get_efivars_size => sub {
         return 128 * 1024;
     },
-- 
2.47.2



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

