From 6ad789010043dc4dcf8d1c0f497b6c728d230f45 Mon Sep 17 00:00:00 2001 From: Imran Hussain Date: Thu, 20 Jan 2022 12:26:41 +0000 Subject: [PATCH] [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 --- nova/exception.py | 4 ++ nova/tests/unit/virt/libvirt/test_driver.py | 38 +++++++++++++++++++ nova/tests/unit/virt/libvirt/test_host.py | 8 ++++ nova/virt/libvirt/config.py | 13 +++++++ nova/virt/libvirt/driver.py | 10 ++++- nova/virt/libvirt/host.py | 7 ++-- .../bug-1958636-smm-check-and-enable.yaml | 7 ++++ 7 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/bug-1958636-smm-check-and-enable.yaml diff --git a/nova/exception.py b/nova/exception.py index c0f25bd260bb..94ab256c1bf3 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1920,6 +1920,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 71b8f7274f86..fd6d2d2c4609 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -5066,6 +5066,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 192909d72194..57036ff827c5 100644 --- a/nova/tests/unit/virt/libvirt/test_host.py +++ b/nova/tests/unit/virt/libvirt/test_host.py @@ -1811,6 +1811,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 7129933f3474..6d881bc7d5b3 100644 --- a/nova/virt/libvirt/config.py +++ b/nova/virt/libvirt/config.py @@ -2688,6 +2688,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 e88d9068c6de..3cccc7a6d3d3 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -6292,9 +6292,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 @@ -6306,6 +6307,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 5c39dd320fec..73628b0d0bcb 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -1642,11 +1642,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) @@ -1676,6 +1676,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 000000000000..81afceeb5f0b --- /dev/null +++ b/releasenotes/notes/bug-1958636-smm-check-and-enable.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`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.