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

Reply via email to