[nova/libvirt] Support for checking and enabling SMM when needed

Check the features list we get from the firmware descriptor file
to see if we need SMM (requires-smm), if so then enable it as
we aren't using the libvirt built in mechanism to enable it
when grabbing the right firmware.

Closes-Bug: 1958636

Change-Id: I890b3021a29fa546d9e36b21b1111e8537cd0020
Signed-off-by: Imran Hussain <ih@imranh.co.uk>
(cherry picked from commit 6ad789010043dc4dcf8d1c0f497b6c728d230f45)
(cherry picked from commit 62e1a621d19e8833a18afdba86de7f8334171c63)

7 files changed, 82 insertions, 5 deletions

diff --git a/nova/exception.py b/nova/exception.py
index 8b4f11aab7..ad322b5e95 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -1903,6 +1903,10 @@ class SecureBootNotSupported(Invalid):
     msg_fmt = _("Secure Boot is not supported by host")
 
 
+class FirmwareSMMNotSupported(Invalid):
+    msg_fmt = _("This firmware doesn't require (support) SMM")
+
+
 class TriggerCrashDumpNotSupported(Invalid):
     msg_fmt = _("Triggering crash dump is not supported")
 
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 00ee408f45..99d30c4590 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -5054,6 +5054,44 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         self.assertEqual('/usr/share/OVMF/OVMF_CODE.fd', cfg.os_loader)
         self.assertEqual('/usr/share/OVMF/OVMF_VARS.fd', cfg.os_nvram_template)
 
+    def test_get_guest_config_with_secure_boot_and_smm_required(self):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        # uefi only used with secure boot
+        drvr._host._supports_uefi = True
+        # smm only used with secure boot
+        drvr._host._supports_secure_boot = True
+
+        # NOTE(imranh2): Current way of gathering firmwares is inflexible
+        # nova/tests/fixtures/libvirt.py FakeLoaders has requires-smm
+        # defined. do the following to make sure we get this programtically
+        # in the future we should test firmwares that both do and don't
+        # require smm but the current way firmware is selected doesn't
+        # make it possible to do so.
+        loader, nvram_template, requires_smm = drvr._host.get_loader(
+            'x86_64', 'q35', True)
+
+        image_meta = objects.ImageMeta.from_dict({
+            'disk_format': 'raw',
+            # secure boot requires UEFI
+            'properties': {
+                'hw_firmware_type': 'uefi',
+                'hw_machine_type': 'q35',
+                'os_secure_boot': 'required',
+            },
+        })
+        instance_ref = objects.Instance(**self.test_instance)
+
+        disk_info = blockinfo.get_disk_info(
+            CONF.libvirt.virt_type, instance_ref, image_meta)
+
+        cfg = drvr._get_guest_config(
+            instance_ref, [], image_meta, disk_info)
+        # if we require it make sure it's there
+        if requires_smm:
+            self.assertTrue(any(isinstance(feature,
+              vconfig.LibvirtConfigGuestFeatureSMM)
+                for feature in cfg.features))
+
     @ddt.data(True, False)
     def test_get_guest_config_with_secure_boot_required(
         self, host_has_support,
diff --git a/nova/tests/unit/virt/libvirt/test_host.py b/nova/tests/unit/virt/libvirt/test_host.py
index 20279a91a2..0628c18aed 100644
--- a/nova/tests/unit/virt/libvirt/test_host.py
+++ b/nova/tests/unit/virt/libvirt/test_host.py
@@ -1793,6 +1793,14 @@ cg /cgroup/memory cg opt1,opt2 0 0
         loader = self.host.get_loader('x86_64', 'q35', has_secure_boot=True)
         self.assertIsNotNone(loader)
 
+        # check that SMM bool is false as we don't need it
+        self.assertFalse(loader[2])
+
+        # check that we get SMM bool correctly (True) when required
+        loaders[0]['features'].append('requires-smm')
+        loader = self.host.get_loader('x86_64', 'q35', has_secure_boot=True)
+        self.assertTrue(loader[2])
+
         # while it should fail here since we don't want it now
         self.assertRaises(
             exception.UEFINotSupported,
diff --git a/nova/virt/libvirt/config.py b/nova/virt/libvirt/config.py
index 106d0413f8..7a99cce27e 100644
--- a/nova/virt/libvirt/config.py
+++ b/nova/virt/libvirt/config.py
@@ -2687,6 +2687,19 @@ class LibvirtConfigGuestFeatureKvmHidden(LibvirtConfigGuestFeature):
         return root
 
 
+class LibvirtConfigGuestFeatureSMM(LibvirtConfigGuestFeature):
+
+    def __init__(self, **kwargs):
+        super(LibvirtConfigGuestFeatureSMM, self).__init__("smm", **kwargs)
+
+    def format_dom(self):
+        root = super(LibvirtConfigGuestFeatureSMM, self).format_dom()
+
+        root.append(etree.Element("smm", state="on"))
+
+        return root
+
+
 class LibvirtConfigGuestFeaturePMU(LibvirtConfigGuestFeature):
 
     def __init__(self, state, **kwargs):
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 73ea0d2c56..8719d67af3 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -6252,9 +6252,10 @@ class LibvirtDriver(driver.ComputeDriver):
                     guest.os_loader_secure = False
 
                 try:
-                    loader, nvram_template = self._host.get_loader(
+                    loader, nvram_template, requires_smm = (
+                    self._host.get_loader(
                         arch, mach_type,
-                        has_secure_boot=guest.os_loader_secure)
+                        has_secure_boot=guest.os_loader_secure))
                 except exception.UEFINotSupported as exc:
                     if guest.os_loader_secure:
                         # we raise a specific exception if we requested secure
@@ -6266,6 +6267,11 @@ class LibvirtDriver(driver.ComputeDriver):
                 guest.os_loader_type = 'pflash'
                 guest.os_nvram_template = nvram_template
 
+                # if the feature set says we need SMM then enable it
+                if requires_smm:
+                    guest.features.append(
+                        vconfig.LibvirtConfigGuestFeatureSMM())
+
             # NOTE(lyarwood): If the machine type isn't recorded in the stashed
             # image metadata then record it through the system metadata table.
             # This will allow the host configuration to change in the future
diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py
index 1de93137b5..14adf9c653 100644
--- a/nova/virt/libvirt/host.py
+++ b/nova/virt/libvirt/host.py
@@ -1625,11 +1625,11 @@ class Host(object):
         arch: str,
         machine: str,
         has_secure_boot: bool,
-    ) -> ty.Tuple[str, str]:
+    ) -> ty.Tuple[str, str, bool]:
         """Get loader for the specified architecture and machine type.
 
-        :returns: A tuple of the bootloader executable path and the NVRAM
-            template path.
+        :returns: A the bootloader executable path and the NVRAM
+            template path and a bool indicating if we need to enable SMM.
         """
 
         machine = self.get_canonical_machine_type(arch, machine)
@@ -1659,6 +1659,7 @@ class Host(object):
             return (
                 loader['mapping']['executable']['filename'],
                 loader['mapping']['nvram-template']['filename'],
+                'requires-smm' in loader['features'],
             )
 
         raise exception.UEFINotSupported()
diff --git a/releasenotes/notes/bug-1958636-smm-check-and-enable.yaml b/releasenotes/notes/bug-1958636-smm-check-and-enable.yaml
new file mode 100644
index 0000000000..81afceeb5f
--- /dev/null
+++ b/releasenotes/notes/bug-1958636-smm-check-and-enable.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    [`bug 1958636 <https://bugs.launchpad.net/nova/+bug/1958636>`_]
+    Explicitly check for and enable SMM when firmware requires it.
+    Previously we assumed libvirt would do this for us but this is
+    not true in all cases.