qemu: Don't pick firmware that requires SMM when smm=off

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 <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

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;

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

tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.err

@@ -1 +0,0 @@
-Requested operation is not valid: domain has SMM turned off but chosen firmware requires it

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");