libvirt: Allow disabling CPU flags via cpu_model_extra_flags

Parse a comma-separated list of CPU flags from
`[libvirt]/cpu_model_extra_flags`.  If the CPU flag starts with '+',
enable the feature in Nova guest CPU guest XML, or if it starts with
'-', disable the feature.  If neither '+' nor '-' is specified, enable
the flag.  For example, on a compute node that is running hardware (e.g.
an Intel server that supports TSX) and virtualization software that
supports the given CPU flags, if a user provides this config:

    [libvirt]
    cpu_mode = custom
    cpu_models = Cascadelake-Server
    cpu_model_extra_flags = -hle, -rtm, +ssbd, mtrr

Then Nova should generate this CPU for the guest:

     <cpu match='exact'>
       <model fallback='forbid'>Cascadelake-Server</model>
       <vendor>Intel</vendor>
       <feature policy='require' name='ssbd'/>
       <feature policy='require' name='mtrr'/>
       <feature policy='disable' name='hle'/>
       <feature policy='disable' name='rtm'/>
     </cpu>

This ability to selectively disable CPU flags lets you avoid any CPU
flags that need to be disabled for any number of reasons.  E.g. disable
a CPU flag that is a potential security risk, or disable one that causes
a performance penalty.

blueprint: allow-disabling-cpu-flags

Change-Id: I2ef7c5bef87bd64c087f3b136c2faac9a3865f10
Signed-off-by: Patrick Uiterwijk <patrick@puiterwijk.org>
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>

nova/conf/libvirt.py

@@ -532,9 +532,8 @@ richer that the previous CPU model.
 
 Possible values:
 
-* The named CPU models listed in ``/usr/share/libvirt/cpu_map.xml`` for
+* The named CPU models can be found via ``virsh cpu-models ARCH``, where
-  libvirt prior to version 4.7.0 or ``/usr/share/libvirt/cpu_map/*.xml``
+  ARCH is your host architecture.
-  for version 4.7.0 and higher.
 
 Related options:
 
@@ -554,52 +553,59 @@ Related options:
         ),
         default=[],
         help="""
-This allows specifying granular CPU feature flags when configuring CPU
+Enable or disable guest CPU flags.
-models.  For example, to explicitly specify the ``pcid``
+
-(Process-Context ID, an Intel processor feature -- which is now required
+To explicitly enable or disable CPU flags, use the ``+flag`` or
-to address the guest performance degradation as a result of applying the
+``-flag`` notation -- the ``+`` sign will enable the CPU flag for the
-"Meltdown" CVE fixes to certain Intel CPU models) flag to the
+guest, while a ``-`` sign will disable it.  If neither ``+`` nor ``-``
-"IvyBridge" virtual CPU model::
+is specified, the flag will be enabled, which is the default behaviour.
+For example, if you specify the following (assuming the said CPU model
+and features are supported by the host hardware and software)::
 
     [libvirt]
     cpu_mode = custom
-    cpu_models = IvyBridge
+    cpu_models = Cascadelake-Server
-    cpu_model_extra_flags = pcid
+    cpu_model_extra_flags = -hle, -rtm, +ssbd, mtrr
 
-To specify multiple CPU flags (e.g. the Intel ``VMX`` to expose the
+Nova will disable the ``hle`` and ``rtm`` flags for the guest; and it
-virtualization extensions to the guest, or ``pdpe1gb`` to configure 1GB
+will enable ``ssbd`` and ``mttr`` (because it was specified with neither
-huge pages for CPU models that do not provide it)::
+``+`` nor ``-`` prefix).
+
+The CPU flags are case-insensitive.  In the following example, the
+``pdpe1gb`` flag will be disabled for the guest; ``vmx`` and ``pcid``
+flags will be enabled::
 
     [libvirt]
     cpu_mode = custom
     cpu_models = Haswell-noTSX-IBRS
-    cpu_model_extra_flags = PCID, VMX, pdpe1gb
+    cpu_model_extra_flags = -PDPE1GB, +VMX, pcid
 
-As it can be noticed from above, the ``cpu_model_extra_flags`` config
+Specifying extra CPU flags is valid in combination with all the three
-attribute is case insensitive.  And specifying extra flags is valid in
+possible values of ``cpu_mode`` config attribute: ``custom`` (this also
-combination with all the three possible values for ``cpu_mode``:
+requires an explicit CPU model to be specified via the ``cpu_models``
-``custom`` (this also requires an explicit ``cpu_models`` to be
+config attribute), ``host-model``, or ``host-passthrough``.
-specified), ``host-model``, or ``host-passthrough``.  A valid example
+
-for allowing extra CPU flags even for ``host-passthrough`` mode is that
+There can be scenarios where you may need to configure extra CPU flags
-sometimes QEMU may disable certain CPU features -- e.g. Intel's
+even for ``host-passthrough`` CPU mode, because sometimes QEMU may
-"invtsc", Invariable Time Stamp Counter, CPU flag.  And if you need to
+disable certain CPU features.  An example of this is Intel's "invtsc"
-expose that CPU flag to the Nova instance, the you need to explicitly
+(Invariable Time Stamp Counter) CPU flag -- if you need to expose this
-ask for it.
+flag to a Nova instance, you need to explicitly enable it.
 
 The possible values for ``cpu_model_extra_flags`` depends on the CPU
-model in use. Refer to ``/usr/share/libvirt/cpu_map.xml`` for libvirt
+model in use.  Refer to `/usr/share/libvirt/cpu_map/*.xml`` for possible
-prior to version 4.7.0 or ``/usr/share/libvirt/cpu_map/*.xml`` thereafter
+CPU feature flags for a given CPU model.
-for possible CPU feature flags for a given CPU model.
 
-Note that when using this config attribute to set the 'PCID' CPU flag
+A special note on a particular CPU flag: ``pcid`` (an Intel processor
-with the ``custom`` CPU mode, not all virtual (i.e. libvirt / QEMU) CPU
+feature that alleviates guest performance degradation as a result of
-models need it:
+applying the 'Meltdown' CVE fixes).  When configuring this flag with the
+``custom`` CPU mode, not all CPU models (as defined by QEMU and libvirt)
+need it:
 
-* The only virtual CPU models that include the 'PCID' capability are
+* The only virtual CPU models that include the ``pcid`` capability are
   Intel "Haswell", "Broadwell", and "Skylake" variants.
 
 * The libvirt / QEMU CPU models "Nehalem", "Westmere", "SandyBridge",
-  and "IvyBridge" will _not_ expose the 'PCID' capability by default,
+  and "IvyBridge" will _not_ expose the ``pcid`` capability by default,
   even if the host CPUs by the same name include it.  I.e. 'PCID' needs
   to be explicitly specified when using the said virtual CPU models.
 

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

@@ -1474,6 +1474,36 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             str(mock_log.call_args[0]),
         )
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
+    def test__prepare_cpu_flag(self):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+
+        # The `+` means the guest "require[s]" (i.e. enable in libvirt
+        # parlance) the said CPU feature; and `-` will disable it
+        feat1 = drvr._prepare_cpu_flag('+md-clear')
+        feat2 = drvr._prepare_cpu_flag('pdpe1gb')
+        feat3 = drvr._prepare_cpu_flag('-ssbd')
+
+        self.assertIsInstance(feat1, vconfig.LibvirtConfigGuestCPUFeature)
+        self.assertIsInstance(feat2, vconfig.LibvirtConfigGuestCPUFeature)
+        self.assertIsInstance(feat3, vconfig.LibvirtConfigGuestCPUFeature)
+
+        cpu = vconfig.LibvirtConfigGuestCPU()
+        cpu.add_feature(feat1)
+        cpu.add_feature(feat2)
+        cpu.add_feature(feat3)
+
+        # Verify that the resulting guest XML records both enabled
+        # _and_ disabled CPU features
+        expected_xml = '''
+              <cpu match='exact'>
+                <feature policy='require' name='md-clear'/>
+                <feature policy='require' name='pdpe1gb'/>
+                <feature policy='disable' name='ssbd'/>
+              </cpu>'''
+        self.assertXmlEqual(expected_xml, cpu.to_xml())
+
     @mock.patch.object(libvirt_driver.LibvirtDriver,
                        '_register_instance_machine_type', new=mock.Mock())
     def test__check_cpu_compatibility_start_ok(self):
@@ -1530,6 +1560,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
 
     @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU')
     def test__check_cpu_compatibility_wrong_flag(self, mocked_compare):
+        # here, and in the surrounding similar tests, the non-zero error
+        # code in the compareCPU() side effect indicates error
         mocked_compare.side_effect = (2, 0)
         self.flags(cpu_mode="custom",
                    cpu_models=["Broadwell-noTSX"],
@@ -1539,6 +1571,20 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         self.assertRaises(exception.InvalidCPUInfo,
                           drvr.init_host, "dummyhost")
 
+    @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU')
+    def test__check_cpu_compatibility_enabled_and_disabled_flags(
+            self, mocked_compare
+    ):
+        mocked_compare.side_effect = (2, 0)
+        self.flags(
+            cpu_mode="custom",
+            cpu_models=["Cascadelake-Server"],
+            cpu_model_extra_flags = ["-hle", "-rtm", "+ssbd", "mttr"],
+            group="libvirt")
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        self.assertRaises(exception.InvalidCPUInfo,
+                          drvr.init_host, "dummyhost")
+
     def test__check_cpu_compatibility_invalid_virt_type(self):
         """Test getting CPU traits when using a virt_type that doesn't support
         the feature, only kvm and qemu supports reporting CPU traits.
@@ -8108,6 +8154,43 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         self.assertEqual(conf.cpu.threads, 1)
         mock_warn.assert_not_called()
 
+    @mock.patch.object(libvirt_driver.LOG, 'warning')
+    def test_get_guest_cpu_config_custom_flags_enabled_and_disabled(
+            self, mock_warn
+    ):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        instance_ref = objects.Instance(**self.test_instance)
+        image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
+
+        self.flags(
+            cpu_mode="custom",
+            cpu_models=["Cascadelake-Server"],
+            cpu_model_extra_flags=['-hle', '-rtm', '+ssbd', 'mtrr'],
+            group='libvirt')
+        disk_info = blockinfo.get_disk_info(
+            CONF.libvirt.virt_type,
+            instance_ref,
+            image_meta)
+        conf = drvr._get_guest_config(
+            instance_ref,
+            _fake_network_info(self),
+            image_meta, disk_info)
+        features = [(feature.name, feature.policy)
+                    for feature in conf.cpu.features]
+        self.assertIsInstance(
+            conf.cpu,
+            vconfig.LibvirtConfigGuestCPU)
+        self.assertEqual(conf.cpu.mode, "custom")
+        self.assertEqual(conf.cpu.model, "Cascadelake-Server")
+        self.assertIn(("ssbd", 'require'), features)
+        self.assertIn(("mtrr", 'require'), features)
+        self.assertIn(("hle", 'disable'), features)
+        self.assertIn(("rtm", 'disable'), features)
+        self.assertEqual(conf.cpu.sockets, instance_ref.flavor.vcpus)
+        self.assertEqual(conf.cpu.cores, 1)
+        self.assertEqual(conf.cpu.threads, 1)
+        mock_warn.assert_not_called()
+
     def test_get_guest_cpu_config_custom_upper_cpu_model(self):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
         instance_ref = objects.Instance(**self.test_instance)

nova/virt/libvirt/config.py

@@ -765,10 +765,10 @@ class LibvirtConfigCPU(LibvirtConfigObject):
 
 class LibvirtConfigGuestCPUFeature(LibvirtConfigCPUFeature):
 
-    def __init__(self, name=None, **kwargs):
+    def __init__(self, name=None, policy="require", **kwargs):
         super(LibvirtConfigGuestCPUFeature, self).__init__(name, **kwargs)
 
-        self.policy = "require"
+        self.policy = policy
 
     def format_dom(self):
         ft = super(LibvirtConfigGuestCPUFeature, self).format_dom()

nova/virt/libvirt/driver.py

@@ -678,6 +678,25 @@ class LibvirtDriver(driver.ComputeDriver):
             LOG.debug("Instance machine_type updated to %s", hw_machine_type,
                       instance=instance)
 
+    def _prepare_cpu_flag(self, flag):
+        # NOTE(kchamart) This helper method will be used while computing
+        # guest CPU compatibility.  It will take into account a
+        # comma-separated list of CPU flags from
+        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts
+        # with '+', it is enabled for the guest; if it starts with '-',
+        # it is disabled.  If neither '+' nor '-' is specified, the CPU
+        # flag is enabled.
+        if flag.startswith('-'):
+            flag = flag.lstrip('-')
+            policy_value = 'disable'
+        else:
+            flag = flag.lstrip('+')
+            policy_value = 'require'
+
+        cpu_feature = vconfig.LibvirtConfigGuestCPUFeature(
+                        flag, policy=policy_value)
+        return cpu_feature
+
     def _check_cpu_compatibility(self):
         mode = CONF.libvirt.cpu_mode
         models = CONF.libvirt.cpu_models
@@ -717,7 +736,8 @@ class LibvirtDriver(driver.ComputeDriver):
         cpu = vconfig.LibvirtConfigGuestCPU()
         cpu.model = self._host.get_capabilities().host.cpu.model
         for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):
-            cpu.add_feature(vconfig.LibvirtConfigCPUFeature(flag))
+            cpu_feature = self._prepare_cpu_flag(flag)
+            cpu.add_feature(cpu_feature)
             try:
                 self._compare_cpu(cpu, self._get_cpu_info(), None)
             except exception.InvalidCPUInfo as e:
@@ -4621,9 +4641,15 @@ class LibvirtDriver(driver.ComputeDriver):
         # do fine-grained validation of a certain CPU model + CPU flags
         # against a specific QEMU binary (the libvirt RFE bug for that:
         # https://bugzilla.redhat.com/show_bug.cgi?id=1559832).
+        #
+        # NOTE(kchamart) Similar to what was done in
+        # _check_cpu_compatibility(), the below parses a comma-separated
+        # list of CPU flags from `[libvirt]cpu_model_extra_flags` and
+        # will selectively enable or disable a given CPU flag for the
+        # guest, before it is launched by Nova.
         for flag in extra_flags:
-            cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))
+            cpu_feature = self._prepare_cpu_flag(flag)
-
+            cpu.add_feature(cpu_feature)
         return cpu
 
     def _match_cpu_model_by_flags(self, models, flags):

releasenotes/notes/allow-disabling-cpu-flags-cc861a3bdfffadf8.yaml

@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    The libvirt driver now allows explicitly disabling CPU flags for
+    guests via the ``[libvirt]cpu_model_extra_flags`` config attribute.
+    This is possible via a ``+`` / ``-`` notation, where if you specify
+    a CPU flag prefixed with a ``+`` sign (without quotes), it will be
+    enabled for the guest, while a prefix of ``-`` will disable it.  If
+    neither ``+`` nor ``-`` is specified, the CPU flag will be enabled,
+    which is the default behaviour.
+
+    Refer to the ``[libvirt]cpu_model_extra_flags`` documentation for
+    more information.