Merge "[nova/libvirt] Support for checking and enabling SMM when needed" into stable/wallaby

Author: Zuul

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

nova/tests/unit/virt/libvirt/test_driver.py

@@ -5054,6 +5054,44 @@ def test_get_guest_config_with_uefi(self):
         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,

nova/tests/unit/virt/libvirt/test_host.py

@@ -1793,6 +1793,14 @@ def fake_get_mtype(arch, machine):
         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,

nova/virt/libvirt/config.py

@@ -2687,6 +2687,19 @@ def format_dom(self):
         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):

nova/virt/libvirt/driver.py

@@ -6252,9 +6252,10 @@ def _configure_guest_by_virt_type(
                     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 @@ def _configure_guest_by_virt_type(
                 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

nova/virt/libvirt/host.py

@@ -1625,11 +1625,11 @@ def get_loader(
         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 @@ def get_loader(
             return (
                 loader['mapping']['executable']['filename'],
                 loader['mapping']['nvram-template']['filename'],
+                'requires-smm' in loader['features'],
             )
 
         raise exception.UEFINotSupported()

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.