At the moment, if SMM is explicitly disabled in the domain XML but a firmware descriptor that requires SMM to be enabled has the highest priority and otherwise matches the requirements, we pick that firmware only to error out later, when the domain is started.
A better approach is to take into account the fact that SMM is disabled while performing autoselection, and ignore all descriptors that advertise the requires-smm feature. Signed-off-by: Andrea Bolognani <abolo...@redhat.com> --- src/qemu/qemu_firmware.c | 33 +++++++---------- ...rmware-auto-efi-smm-off.x86_64-latest.args | 37 +++++++++++++++++++ ...irmware-auto-efi-smm-off.x86_64-latest.err | 1 - tests/qemuxml2argvtest.c | 2 +- 4 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args delete mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.err diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 572172bc75..c24bca6183 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1102,11 +1102,18 @@ qemuFirmwareMatchDomain(const virDomainDef *def, } } - if (loader && loader->secure == VIR_TRISTATE_BOOL_YES && - !requiresSMM) { - VIR_DEBUG("Domain restricts pflash programming to SMM, " - "but firmware '%s' doesn't support SMM", path); - return false; + if (requiresSMM) { + if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_OFF) { + VIR_DEBUG("Domain explicitly disables SMM, " + "but firmware '%s' requires it to be enabled", path); + return false; + } + } else { + if (loader && loader->secure == VIR_TRISTATE_BOOL_YES) { + VIR_DEBUG("Domain restricts pflash programming to SMM, " + "but firmware '%s' doesn't support SMM", path); + return false; + } } if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_FLASH) { @@ -1244,21 +1251,9 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver, for (i = 0; i < fw->nfeatures; i++) { switch (fw->features[i]) { case QEMU_FIRMWARE_FEATURE_REQUIRES_SMM: - switch (def->features[VIR_DOMAIN_FEATURE_SMM]) { - case VIR_TRISTATE_SWITCH_OFF: - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain has SMM turned off " - "but chosen firmware requires it")); - return -1; - case VIR_TRISTATE_SWITCH_ABSENT: - VIR_DEBUG("Enabling SMM feature"); - def->features[VIR_DOMAIN_FEATURE_SMM] = VIR_TRISTATE_SWITCH_ON; - break; + VIR_DEBUG("Enabling SMM feature"); + def->features[VIR_DOMAIN_FEATURE_SMM] = VIR_TRISTATE_SWITCH_ON; - case VIR_TRISTATE_SWITCH_ON: - case VIR_TRISTATE_SWITCH_LAST: - break; - } VIR_DEBUG("Enabling secure loader"); def->os.loader->secure = VIR_TRISTATE_BOOL_YES; break; diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args b/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args new file mode 100644 index 0000000000..692795194f --- /dev/null +++ b/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/tmp/lib/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":"/tmp/lib/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.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","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ +-machine pc-q35-4.0,usb=off,smm=off,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \ +-accel kvm \ +-cpu qemu64 \ +-m 1024 \ +-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/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.err deleted file mode 100644 index f4e2e13a15..0000000000 --- a/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.err +++ /dev/null @@ -1 +0,0 @@ -Requested operation is not valid: domain has SMM turned off but chosen firmware requires it diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c18958db86..812e87ea90 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1162,7 +1162,7 @@ mymain(void) DO_TEST_CAPS_LATEST("firmware-auto-efi-enrolled-keys"); DO_TEST_CAPS_LATEST("firmware-auto-efi-no-enrolled-keys"); DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-enrolled-keys-no-secboot"); - DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-smm-off"); + DO_TEST_CAPS_LATEST("firmware-auto-efi-smm-off"); DO_TEST_CAPS_ARCH_LATEST("firmware-auto-efi-aarch64", "aarch64"); DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-file"); DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-network-nbd"); -- 2.39.1