This kind of firmware build is not shipped in Fedora, where most descriptors in our test suite come from, so we had to make it up. It was based off the Secure Boot-enabled edk2 build, and the filename it points to is the same. That has been fine so far since it's not actually being picked up by any of the test cases, but that's going to change soon and when it does we want to be able to avoid any confusion. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- .../qemufirmwaredata/usr/share/qemu/firmware/90-combined.json | 4 ++-- tests/qemufirmwaretest.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemufirmwaredata/usr/share/qemu/firmware/90-combined.json b/tests/qemufirmwaredata/usr/share/qemu/firmware/90-combined.json index 2c8381adf7..8ecac440b4 100644 --- a/tests/qemufirmwaredata/usr/share/qemu/firmware/90-combined.json +++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/90-combined.json @@ -1,5 +1,5 @@ { - "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled", + "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled (combined)", "interface-types": [ "uefi" ], @@ -7,7 +7,7 @@ "device": "flash", "mode": "combined", "executable": { - "filename": "/usr/share/edk2/ovmf/OVMF.secboot.fd", + "filename": "/usr/share/edk2/ovmf/OVMF.combined.fd", "format": "raw" } }, diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c index f16ea526ff..a4fb5c9b9c 100644 --- a/tests/qemufirmwaretest.c +++ b/tests/qemufirmwaretest.c @@ -317,7 +317,7 @@ mymain(void) "/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd:/usr/share/edk2/ovmf/OVMF_VARS.fd:" "/usr/share/edk2/ovmf/OVMF_CODE_4M.qcow2:/usr/share/edk2/ovmf/OVMF_VARS_4M.qcow2:" "/usr/share/edk2/ovmf/OVMF_CODE.fd:/usr/share/edk2/ovmf/OVMF_VARS.fd:" - "/usr/share/edk2/ovmf/OVMF.secboot.fd:NULL:" + "/usr/share/edk2/ovmf/OVMF.combined.fd:NULL:" "/usr/share/edk2/ovmf/OVMF.amdsev.fd:NULL:" "/usr/share/edk2/ovmf/OVMF.inteltdx.secboot.fd:NULL", VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS, -- 2.50.1

The current code assumes that a stateless firmware has to be explicitly requested by the user, and should never be picked otherwise. This means that, for example, domains configured to use SEV-SNP are forced to explicitly request for the firmware to be stateless. Additionally, we assume that only split firmware is suitable for the stateful use case, whereas a combined firmware image would also do the job. As a result of these changes, the failing SEV-SNP test case that was added recently passes, and so do the test cases requesting read/write firmware. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/qemu/qemu_firmware.c | 32 ++++++++++------ ...ware-auto-efi-rw-pflash.x86_64-latest.args | 36 ++++++++++++++++++ ...mware-auto-efi-rw-pflash.x86_64-latest.err | 1 - ...mware-auto-efi-rw-pflash.x86_64-latest.xml | 6 ++- .../firmware-auto-efi-rw.x86_64-latest.args | 36 ++++++++++++++++++ .../firmware-auto-efi-rw.x86_64-latest.err | 1 - .../firmware-auto-efi-rw.x86_64-latest.xml | 6 ++- ...auto-efi-sev-snp.x86_64-latest+amdsev.args | 37 +++++++++++++++++++ ...-auto-efi-sev-snp.x86_64-latest+amdsev.err | 1 - ...-auto-efi-sev-snp.x86_64-latest+amdsev.xml | 6 ++- tests/qemuxmlconftest.c | 5 +-- 11 files changed, 147 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.args delete mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.args delete mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args delete mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index f10137144e..22e0eb83f0 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1300,16 +1300,21 @@ qemuFirmwareMatchDomain(const virDomainDef *def, return false; } - if (loader && loader->stateless == VIR_TRISTATE_BOOL_YES) { - if (flash->mode != QEMU_FIRMWARE_FLASH_MODE_STATELESS) { - VIR_DEBUG("Discarding loader without stateless flash"); - return false; - } - } else { - if (flash->mode != QEMU_FIRMWARE_FLASH_MODE_SPLIT) { - VIR_DEBUG("Discarding loader without split flash"); - return false; - } + /* 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 */ + if (loader && + loader->stateless == VIR_TRISTATE_BOOL_YES && + flash->mode != QEMU_FIRMWARE_FLASH_MODE_STATELESS) { + VIR_DEBUG("Discarding loader without stateless flash"); + return false; + } + if (loader && + loader->stateless == VIR_TRISTATE_BOOL_NO && + flash->mode == QEMU_FIRMWARE_FLASH_MODE_STATELESS) { + VIR_DEBUG("Discarding loader with stateless flash"); + return false; } if (loader && @@ -1424,9 +1429,14 @@ qemuFirmwareEnableFeaturesModern(virDomainDef *def, loader = def->os.loader; loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; - loader->readonly = VIR_TRISTATE_BOOL_YES; loader->format = format; + /* Combined mode implies read/write, other modes imply read-only */ + if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_COMBINED) + loader->readonly = VIR_TRISTATE_BOOL_NO; + else + loader->readonly = VIR_TRISTATE_BOOL_YES; + VIR_FREE(loader->path); loader->path = g_strdup(flash->executable.filename); diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.args b/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.args new file mode 100644 index 0000000000..d06de24db8 --- /dev/null +++ b/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF.combined.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":false,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-machine pc-q35-10.0,usb=off,smm=on,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,acpi=on \ +-accel kvm \ +-cpu qemu64 \ +-global driver=cfi.pflash01,property=secure,value=on \ +-m size=1048576k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-global ICH9-LPC.noreboot=off \ +-watchdog-action reset \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.err deleted file mode 100644 index 3edb2b3451..0000000000 --- a/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.err +++ /dev/null @@ -1 +0,0 @@ -operation failed: Unable to find 'efi' firmware that is compatible with the current configuration diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.xml index 217c1f4b94..7b79738d98 100644 --- a/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.xml @@ -6,11 +6,15 @@ <vcpu placement='static'>1</vcpu> <os firmware='efi'> <type arch='x86_64' machine='pc-q35-10.0'>hvm</type> - <loader readonly='no' type='pflash' format='raw'/> + <firmware> + <feature enabled='yes' name='secure-boot'/> + </firmware> + <loader readonly='no' secure='yes' type='pflash' format='raw'>/usr/share/edk2/ovmf/OVMF.combined.fd</loader> <boot dev='hd'/> </os> <features> <acpi/> + <smm state='on'/> </features> <cpu mode='custom' match='exact' check='none'> <model fallback='forbid'>qemu64</model> diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.args b/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.args new file mode 100644 index 0000000000..d06de24db8 --- /dev/null +++ b/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF.combined.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":false,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-machine pc-q35-10.0,usb=off,smm=on,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,acpi=on \ +-accel kvm \ +-cpu qemu64 \ +-global driver=cfi.pflash01,property=secure,value=on \ +-m size=1048576k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-global ICH9-LPC.noreboot=off \ +-watchdog-action reset \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.err deleted file mode 100644 index 3edb2b3451..0000000000 --- a/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.err +++ /dev/null @@ -1 +0,0 @@ -operation failed: Unable to find 'efi' firmware that is compatible with the current configuration diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.xml index 0f6b965067..7b79738d98 100644 --- a/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.xml @@ -6,11 +6,15 @@ <vcpu placement='static'>1</vcpu> <os firmware='efi'> <type arch='x86_64' machine='pc-q35-10.0'>hvm</type> - <loader readonly='no' format='raw'/> + <firmware> + <feature enabled='yes' name='secure-boot'/> + </firmware> + <loader readonly='no' secure='yes' type='pflash' format='raw'>/usr/share/edk2/ovmf/OVMF.combined.fd</loader> <boot dev='hd'/> </os> <features> <acpi/> + <smm state='on'/> </features> <cpu mode='custom' match='exact' check='none'> <model fallback='forbid'>qemu64</model> diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args new file mode 100644 index 0000000000..ab8fed6593 --- /dev/null +++ b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF.amdsev.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","read-only":false}' \ +-machine pc-q35-10.0,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-storage,acpi=on \ +-accel kvm \ +-cpu qemu64 \ +-m size=1048576k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-global ICH9-LPC.noreboot=off \ +-watchdog-action reset \ +-object '{"qom-type":"sev-snp-guest","id":"lsec0","cbitpos":51,"reduced-phys-bits":1,"policy":196608}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err deleted file mode 100644 index 3edb2b3451..0000000000 --- a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err +++ /dev/null @@ -1 +0,0 @@ -operation failed: Unable to find 'efi' firmware that is compatible with the current configuration diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml index 81ac7888ea..1cc08ff4fd 100644 --- a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml +++ b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml @@ -6,7 +6,11 @@ <vcpu placement='static'>1</vcpu> <os firmware='efi'> <type arch='x86_64' machine='pc-q35-10.0'>hvm</type> - <loader format='raw'/> + <firmware> + <feature enabled='no' name='secure-boot'/> + </firmware> + <loader readonly='yes' type='pflash' format='raw'>/usr/share/edk2/ovmf/OVMF.amdsev.fd</loader> + <nvram format='raw'>/var/lib/libvirt/qemu/nvram/guest_VARS.fd</nvram> <boot dev='hd'/> </os> <features> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 272d314195..2d048746ea 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1443,8 +1443,8 @@ mymain(void) DO_TEST_CAPS_LATEST("firmware-auto-efi"); DO_TEST_CAPS_LATEST_ABI_UPDATE("firmware-auto-efi"); DO_TEST_CAPS_LATEST("firmware-auto-efi-stateless"); - DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-rw"); - DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-rw-pflash"); + DO_TEST_CAPS_LATEST("firmware-auto-efi-rw"); + DO_TEST_CAPS_LATEST("firmware-auto-efi-rw-pflash"); DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-secure"); DO_TEST_CAPS_LATEST_ABI_UPDATE("firmware-auto-efi-loader-secure"); DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-insecure"); @@ -1478,7 +1478,6 @@ mymain(void) DO_TEST_CAPS_LATEST("firmware-auto-efi-format-mismatch"); DO_TEST_CAPS_ARCH_LATEST_FULL("firmware-auto-efi-sev-snp", "x86_64", - ARG_FLAGS, FLAG_EXPECT_FAILURE, ARG_CAPS_VARIANT, "+amdsev", ARG_END); -- 2.50.1

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 5a320b7f33..b3c7dcb141 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -96,6 +96,12 @@ v11.6.0 (unreleased) ebtables, creating the base chains only if they did not already exist. + * qemu: Fix selection of stateless/combined firmware + + A stateless firmware will now be correctly chosen when appropriate, + e.g. for domains configured to use SEV-SNP. + + v11.5.0 (2025-07-01) ==================== -- 2.50.1

On 8/1/25 03:14, Andrea Bolognani wrote: With this addition, the correct firmware is detected, but it's not properly provided to qemu internal error: QEMU unexpectedly closed the monitor (vm='sles15sp7-snp'): 2025-08-01T17:11:20.589614Z qemu-system-x86_64: pflash with kvm requires KVM readonly memory support The pertinent command line pieces being -blockdev '{"driver":"file","filename":"/usr/share/qemu/ovmf-x86_64-sev.bin","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard": "unmap"}' -blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' But for SNP, it needs to be provided as bios, e.g. -bios /usr/share/qemu/ovmf-x86_64-sev.bin Are we correctly identifying this firmware in the descriptor file? It's advertised as a "flash" device, although I'm not sure if any of the other "FirmwareDevice" options [1] are appropriate. Perhaps the "FirmwareOSInterface" should be 'bios'? Regards, Jim [1] https://gitlab.com/qemu/qemu/-/blob/master/docs/interop/firmware.json?ref...