From fccbbe89fadebe350bf2452ef4dd3368bcf41803 Mon Sep 17 00:00:00 2001
Message-ID: <fccbbe89fadebe350bf2452ef4dd3368bcf41803.1772815313.git.jdenemar@redhat.com>
From: Andrea Bolognani <abologna@redhat.com>
Date: Mon, 19 Jan 2026 18:48:50 +0100
Subject: [PATCH] qemu_firmware: Allow matching stateful ROMs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Stateful ROMs are those that use the uefi-vars QEMU device to
implement access to UEFI variable storage.

Matching works much the same as it does for pflash-based
firmware images. Notably, the <varstore> element is only
allowed for ROM and the <nvram> element is only allowed for
pflash.

The firmware-auto-efi-varstore-q35 and
firmware-auto-efi-varstore-aarch64 fail in a different way
after this change: the input XML is now considered valid, and
the only remaining issue is that the firmware autoselection
process is unable to find a match.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit fa74aae490f9f13c469eba8b3837ab8cd84c64fd)

Conflicts:

  * tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml

    - GIC version mismatch caused by capabilities files
      being outdated or missing downstream

https://issues.redhat.com/browse/RHEL-82645

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_firmware.c                      | 40 +++++++++++++++++--
 ...to-efi-varstore-aarch64.aarch64-latest.err |  2 +-
 ...to-efi-varstore-aarch64.aarch64-latest.xml | 28 +++++++++++++
 ...re-auto-efi-varstore-q35.x86_64-latest.err |  2 +-
 ...re-auto-efi-varstore-q35.x86_64-latest.xml | 36 +++++++++++++++++
 tests/qemuxmlconftest.c                       |  4 +-
 6 files changed, 104 insertions(+), 8 deletions(-)
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 1851ed4a80..60635b559f 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -893,15 +893,18 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw,
  * qemuFirmwareMatchesPaths:
  * @fw: firmware definition
  * @loader: loader definition
+ * @varstore: varstore definition
  *
  * Checks whether @fw is compatible with the information provided as
  * part of the domain definition.
  *
- * Returns: true if @fw is compatible with @loader, false otherwise
+ * Returns: true if @fw is compatible with @loader and @varstore,
+ *          false otherwise
  */
 static bool
 qemuFirmwareMatchesPaths(const qemuFirmware *fw,
-                         const virDomainLoaderDef *loader)
+                         const virDomainLoaderDef *loader,
+                         const virDomainVarstoreDef *varstore)
 {
     const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash;
     const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory;
@@ -922,6 +925,9 @@ qemuFirmwareMatchesPaths(const qemuFirmware *fw,
         if (loader && loader->path &&
             !virFileComparePaths(loader->path, memory->filename))
             return false;
+        if (varstore && varstore->template &&
+            !virFileComparePaths(varstore->template, memory->template))
+            return false;
         break;
     case QEMU_FIRMWARE_DEVICE_NONE:
     case QEMU_FIRMWARE_DEVICE_LAST:
@@ -1112,6 +1118,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
                         const char *path)
 {
     const virDomainLoaderDef *loader = def->os.loader;
+    const virDomainVarstoreDef *varstore = def->os.varstore;
     size_t i;
     qemuFirmwareOSInterface want;
     bool wantUEFI = false;
@@ -1166,7 +1173,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
         return false;
     }
 
-    if (!qemuFirmwareMatchesPaths(fw, def->os.loader)) {
+    if (!qemuFirmwareMatchesPaths(fw, def->os.loader, def->os.varstore)) {
         VIR_DEBUG("No matching path in '%s'", path);
         return false;
     }
@@ -1279,6 +1286,9 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
     if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_FLASH) {
         const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash;
 
+        if (varstore)
+            return false;
+
         if (loader && loader->type &&
             loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH) {
             VIR_DEBUG("Discarding flash loader");
@@ -1377,16 +1387,38 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
             }
         }
     } else if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_MEMORY) {
+        const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory;
+
+        if (loader && loader->nvram)
+            return false;
+
         if (loader && loader->type &&
             loader->type != VIR_DOMAIN_LOADER_TYPE_ROM) {
             VIR_DEBUG("Discarding rom loader");
             return false;
         }
 
-        if (loader && loader->stateless == VIR_TRISTATE_BOOL_NO) {
+        /* Explicit requests for either a stateless or stateful
+         * firmware should be fulfilled, but if no preference is
+         * provided either one is fine as long as the other match
+         * criteria are satisfied. varstore implies stateful */
+        if (loader &&
+            loader->stateless == VIR_TRISTATE_BOOL_NO &&
+            !memory->template) {
             VIR_DEBUG("Discarding stateless loader");
             return false;
         }
+        if (varstore &&
+            !memory->template) {
+            VIR_DEBUG("Discarding stateless loader");
+            return false;
+        }
+        if (loader &&
+            loader->stateless == VIR_TRISTATE_BOOL_YES &&
+            memory->template) {
+            VIR_DEBUG("Discarding non-stateless loader");
+            return false;
+        }
 
         if (loader && loader->readonly == VIR_TRISTATE_BOOL_NO) {
             VIR_DEBUG("Discarding readonly loader");
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err
index b45d304221..3edb2b3451 100644
--- a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err
@@ -1 +1 @@
-Only one of NVRAM/varstore can be used
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml
new file mode 100644
index 0000000000..867d8f03e3
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml
@@ -0,0 +1,28 @@
+<domain type='kvm'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='aarch64' machine='virt-8.2'>hvm</type>
+    <loader format='raw'/>
+    <varstore/>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <gic version='3'/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-aarch64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <audio id='1' type='none'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err
index b45d304221..3edb2b3451 100644
--- a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err
@@ -1 +1 @@
-Only one of NVRAM/varstore can be used
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml
new file mode 100644
index 0000000000..c4d70c9fc5
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml
@@ -0,0 +1,36 @@
+<domain type='kvm'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='x86_64' machine='pc-q35-8.2'>hvm</type>
+    <loader format='raw'/>
+    <varstore/>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <watchdog model='itco' action='reset'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 48c2649aa5..e38a80c57a 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1667,8 +1667,8 @@ mymain(void)
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-file");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-network-nbd");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-network-iscsi");
-    DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-varstore-q35");
-    DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("firmware-auto-efi-varstore-aarch64", "aarch64");
+    DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-varstore-q35");
+    DO_TEST_CAPS_ARCH_LATEST_FAILURE("firmware-auto-efi-varstore-aarch64", "aarch64");
 
     DO_TEST_CAPS_LATEST("firmware-auto-efi-format-loader-qcow2");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-format-loader-qcow2-rom");
-- 
2.53.0