libvirt: Set driver_iommu when attaching virtio devices to SEV instance

As called out in the original spec [1] virtio devices attached to a SEV
enabled instance must have the iommu attribute enabled. This was done
within the original implementation of the spec for all virtio devices
defined when initially spawning the instance but does not include volume
and interfaces that are later hot plugged.

This change corrects this for both volumes and nics and in doing so
slightly refactors the original designer code to make it usable in both
cases.

[1] https://specs.openstack.org/openstack/nova-specs/specs/train/implemented/amd-sev-libvirt-support.html#proposed-change

Closes-Bug: #1930734
Change-Id: I11131a3f90b8af85e7151b519fb26d225629c391
(cherry picked from commit 4d8bf15)

Author: lyarwood

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

@@ -244,7 +244,7 @@ def test_set_vif_mtu_config(self):
         designer.set_vif_mtu_config(conf, 9000)
         self.assertEqual(9000, conf.mtu)
 
-    def test_set_driver_iommu_for_sev(self):
+    def test_set_driver_iommu(self):
         conf = fake_libvirt_data.fake_kvm_guest()
 
         # obj.devices[11]
@@ -253,7 +253,7 @@ def test_set_driver_iommu_for_sev(self):
         controller.index = 0
         conf.add_device(controller)
 
-        designer.set_driver_iommu_for_sev(conf)
+        designer.set_driver_iommu_for_all_devices(conf)
 
         # All disks/interfaces/memballoon are expected to be virtio,
         # thus driver_iommu should be on

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

@@ -3299,7 +3299,7 @@ def test_get_guest_config_sev_no_feature(self):
                           self._setup_sev_guest)
 
     @mock.patch.object(host.Host, 'get_domain_capabilities')
-    @mock.patch.object(designer, 'set_driver_iommu_for_sev')
+    @mock.patch.object(designer, 'set_driver_iommu_for_all_devices')
     def test_get_guest_config_sev(self, mock_designer, fake_domain_caps):
         self._setup_fake_domain_caps(fake_domain_caps)
         cfg = self._setup_sev_guest()
@@ -6459,7 +6459,7 @@ def test_get_guest_config_with_qga_through_image_meta(self):
         self.assertEqual(cfg.devices[6].target_name, "org.qemu.guest_agent.0")
 
     @mock.patch.object(host.Host, 'get_domain_capabilities')
-    @mock.patch.object(designer, 'set_driver_iommu_for_sev')
+    @mock.patch.object(designer, 'set_driver_iommu_for_all_devices')
     def test_get_guest_config_with_qga_through_image_meta_with_sev(
         self, mock_designer, fake_domain_caps,
     ):
@@ -8817,22 +8817,60 @@ def _fake_libvirt_config_guest_disk(self):
 
         return fake_config
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_sev_enabled',
+                       new=mock.Mock(return_value=False))
     @mock.patch.object(volume_drivers.LibvirtFakeVolumeDriver, 'get_config')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_set_cache_mode')
-    def test_get_volume_config(self, _set_cache_mode, get_config):
+    def test_get_volume_config(self, mock_set_cache_mode, mock_get_config):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
-        connection_info = {'driver_volume_type': 'fake',
-                           'data': {'device_path': '/fake',
-                                    'access_mode': 'rw'}}
-        disk_info = {'bus': 'fake-bus', 'type': 'fake-type',
-                     'dev': 'vdb'}
-        config_guest_disk = self._fake_libvirt_config_guest_disk()
+        instance = objects.Instance(**self.test_instance)
+        connection_info = {
+            'driver_volume_type': 'fake',
+            'data': {
+                'device_path': '/fake',
+                'access_mode': 'rw'
+            }
+        }
+        generated_config = self._fake_libvirt_config_guest_disk()
+        mock_get_config.return_value = copy.deepcopy(generated_config)
+
+        returned_config = drvr._get_volume_config(
+            instance,
+            connection_info,
+            mock.sentinel.disk_info)
+
+        mock_get_config.assert_called_once_with(
+            connection_info,
+            mock.sentinel.disk_info)
+        mock_set_cache_mode.assert_called_once_with(returned_config)
+        self.assertEqual(generated_config.to_xml(), returned_config.to_xml())
+
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_sev_enabled',
+                       new=mock.Mock(return_value=True))
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_set_cache_mode',
+                       new=mock.Mock())
+    @mock.patch.object(volume_drivers.LibvirtFakeVolumeDriver, 'get_config')
+    def test_get_volume_config_sev(self, mock_get_config):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instance = objects.Instance(**self.test_instance)
+        connection_info = {
+            'driver_volume_type': 'fake',
+            'data': {
+                'device_path': '/fake',
+                'access_mode': 'rw'
+            }
+        }
+        generated_config = self._fake_libvirt_config_guest_disk()
+        generated_config.target_bus = 'virtio'
+        mock_get_config.return_value = copy.deepcopy(generated_config)
+
+        returned_config = drvr._get_volume_config(
+            instance,
+            connection_info,
+            mock.sentinel.disk_info)
 
-        get_config.return_value = copy.deepcopy(config_guest_disk)
-        config = drvr._get_volume_config(connection_info, disk_info)
-        get_config.assert_called_once_with(connection_info, disk_info)
-        _set_cache_mode.assert_called_once_with(config)
-        self.assertEqual(config_guest_disk.to_xml(), config.to_xml())
+        # Assert that driver_iommu is enabled for this virtio volume
+        self.assertTrue(returned_config.driver_iommu)
 
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_driver')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_attach_encryptor')
@@ -9292,7 +9330,7 @@ def test_attach_volume_with_vir_domain_affect_live_flag(self,
                 mock_connect_volume.assert_called_with(
                     self.context, connection_info, instance, encryption=None)
                 mock_get_volume_config.assert_called_with(
-                    connection_info, disk_info)
+                    instance, connection_info, disk_info)
                 mock_dom.attachDeviceFlags.assert_called_with(
                     mock_conf.to_xml(), flags=flags)
                 mock_check_discard.assert_called_with(mock_conf, instance)
@@ -11492,8 +11530,13 @@ def test_live_migration_update_volume_xml(self, mock_xml,
                 drvr._live_migration_uri(target_connection),
                 params=params, flags=0)
         mock_updated_guest_xml.assert_called_once_with(
-                guest, migrate_data, mock.ANY, get_vif_config=None,
-                new_resources=None)
+                instance_ref,
+                guest,
+                migrate_data,
+                mock.ANY,
+                get_vif_config=None,
+                new_resources=None
+        )
 
     def test_live_migration_update_vifs_xml(self):
         """Tests that when migrate_data.vifs is populated, the destination
@@ -11519,9 +11562,14 @@ def test_live_migration_update_vifs_xml(self):
         guest = libvirt_guest.Guest(mock.MagicMock())
         fake_xml = '<domain type="qemu"/>'
 
-        def fake_get_updated_guest_xml(guest, migrate_data, get_volume_config,
-                                       get_vif_config=None,
-                                       new_resources=None):
+        def fake_get_updated_guest_xml(
+            instance,
+            guest,
+            migrate_data,
+            get_volume_config,
+            get_vif_config=None,
+            new_resources=None
+        ):
             self.assertIsNotNone(get_vif_config)
             return fake_xml
 
@@ -11688,18 +11736,30 @@ def test_update_volume_xml(self):
         conf.source_type = "block"
         conf.source_path = bdmi.connection_info['data'].get('device_path')
 
+        instance = objects.Instance(**self.test_instance)
         guest = libvirt_guest.Guest(mock.MagicMock())
+
         with test.nested(
-                mock.patch.object(drvr, '_get_volume_config',
-                                  return_value=conf),
-                mock.patch.object(guest, 'get_xml_desc',
-                                  return_value=initial_xml)):
-            config = libvirt_migrate.get_updated_guest_xml(guest,
-                            objects.LibvirtLiveMigrateData(
-                                bdms=[bdmi],
-                                serial_listen_addr='127.0.0.1',
-                                serial_listen_ports=[1234]),
-                            drvr._get_volume_config)
+            mock.patch.object(
+                drvr, '_get_volume_config', return_value=conf
+            ),
+            mock.patch.object(
+                guest, 'get_xml_desc', return_value=initial_xml
+            ),
+            mock.patch.object(
+                drvr, '_sev_enabled', new=mock.Mock(return_value=False)
+            )
+        ):
+            config = libvirt_migrate.get_updated_guest_xml(
+                instance,
+                guest,
+                objects.LibvirtLiveMigrateData(
+                    bdms=[bdmi],
+                    serial_listen_addr='127.0.0.1',
+                    serial_listen_ports=[1234]
+                ),
+                drvr._get_volume_config
+            )
             parser = etree.XMLParser(remove_blank_text=True)
             config = etree.fromstring(config, parser)
             target_xml = etree.fromstring(target_xml, parser)
@@ -11884,18 +11944,30 @@ def test_update_volume_xml_no_serial(self):
         conf.source_type = "block"
         conf.source_path = bdmi.connection_info['data'].get('device_path')
 
+        instance = objects.Instance(**self.test_instance)
         guest = libvirt_guest.Guest(mock.MagicMock())
+
         with test.nested(
-                mock.patch.object(drvr, '_get_volume_config',
-                                  return_value=conf),
-                mock.patch.object(guest, 'get_xml_desc',
-                                  return_value=initial_xml)):
-            config = libvirt_migrate.get_updated_guest_xml(guest,
+            mock.patch.object(
+                drvr, '_get_volume_config', return_value=conf
+            ),
+            mock.patch.object(
+                guest, 'get_xml_desc', return_value=initial_xml
+            ),
+            mock.patch.object(
+                drvr, '_sev_enabled', new=mock.Mock(return_value=False)
+            )
+        ):
+            config = libvirt_migrate.get_updated_guest_xml(
+                instance,
+                guest,
                 objects.LibvirtLiveMigrateData(
                     bdms=[bdmi],
                     serial_listen_addr = '127.0.0.1',
-                    serial_listen_ports = [1234]),
-                drvr._get_volume_config)
+                    serial_listen_ports = [1234]
+                ),
+                drvr._get_volume_config
+            )
             self.assertEqual(target_xml, config)
 
     def test_update_volume_xml_no_connection_info(self):
@@ -11918,19 +11990,30 @@ def test_update_volume_xml_no_connection_info(self):
                                                  format='qcow')
         bdmi.connection_info = {}
         conf = vconfig.LibvirtConfigGuestDisk()
+        instance = objects.Instance(**self.test_instance)
         guest = libvirt_guest.Guest(mock.MagicMock())
+
         with test.nested(
-                mock.patch.object(drvr, '_get_volume_config',
-                                  return_value=conf),
-                mock.patch.object(guest, 'get_xml_desc',
-                                  return_value=initial_xml)):
+            mock.patch.object(
+                drvr, '_get_volume_config', return_value=conf
+            ),
+            mock.patch.object(
+                guest, 'get_xml_desc', return_value=initial_xml
+            ),
+            mock.patch.object(
+                drvr, '_sev_enabled', new=mock.Mock(return_value=False)
+            )
+        ):
             config = libvirt_migrate.get_updated_guest_xml(
+                instance,
                 guest,
                 objects.LibvirtLiveMigrateData(
                     bdms=[bdmi],
                     serial_listen_addr='127.0.0.1',
-                    serial_listen_ports=[1234]),
-                drvr._get_volume_config)
+                    serial_listen_ports=[1234]
+                ),
+                drvr._get_volume_config
+            )
             self.assertEqual(target_xml, config)
 
     @mock.patch.object(host.Host, 'has_min_version', return_value=True)
@@ -19077,7 +19160,9 @@ def test_get_guest_storage_config(self):
             self.assertIsNone(instance.default_swap_device)
             connect_volume.assert_called_with(self.context,
                 bdm['connection_info'], instance)
-            get_volume_config.assert_called_with(bdm['connection_info'],
+            get_volume_config.assert_called_with(
+                instance,
+                bdm['connection_info'],
                 {'bus': 'virtio', 'type': 'disk', 'dev': 'vdc'})
             volume_save.assert_called_once_with()
 
@@ -22855,16 +22940,19 @@ def test_attach_interface_guest_set_metadata(self):
             mock_save.assert_called_once_with()
             mock_set_metadata.assert_called_once_with(config_meta)
 
+    @mock.patch('nova.virt.libvirt.designer.set_driver_iommu_for_device')
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_sev_enabled')
     @mock.patch.object(objects.Instance, 'get_network_info')
     @mock.patch.object(objects.Instance, 'save')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata')
     @mock.patch.object(FakeVirtDomain, 'info')
     @mock.patch.object(FakeVirtDomain, 'attachDeviceFlags')
     @mock.patch.object(host.Host, '_get_domain')
     def _test_attach_interface(self, power_state, expected_flags,
-                               mock_get_domain, mock_attach,
-                               mock_info, mock_build, mock_save,
-                               mock_get_network_info):
+                               mock_get_domain, mock_attach, mock_info,
+                               mock_build, mock_save, mock_get_network_info,
+                               mock_sev_enabled, mock_designer_set_iommu,
+                               sev_enabled=False):
         instance = self._create_instance()
         network_info = _fake_network_info(self)
         domain = FakeVirtDomain(fake_xml="""
@@ -22882,6 +22970,7 @@ def _test_attach_interface(self, power_state, expected_flags,
                 </domain>""")
         mock_get_domain.return_value = domain
         mock_info.return_value = [power_state, 1, 2, 3, 4]
+        mock_sev_enabled.return_value = sev_enabled
 
         fake_image_meta = objects.ImageMeta.from_dict(
             {'id': instance.image_ref})
@@ -22907,13 +22996,22 @@ def _test_attach_interface(self, power_state, expected_flags,
             mock_get_network_info.assert_called_once_with()
             mock_attach.assert_called_once_with(expected.to_xml(),
                                                 flags=expected_flags)
+            if sev_enabled:
+                mock_designer_set_iommu.assert_called_once_with(expected)
 
     def test_attach_interface_with_running_instance(self):
         self._test_attach_interface(
             power_state.RUNNING,
             (fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |
              fakelibvirt.VIR_DOMAIN_AFFECT_LIVE))
 
+    def test_attach_interface_with_sev(self):
+        self._test_attach_interface(
+            power_state.RUNNING,
+            (fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |
+             fakelibvirt.VIR_DOMAIN_AFFECT_LIVE),
+            sev_enabled=True)
+
     def test_attach_interface_with_pause_instance(self):
         self._test_attach_interface(
             power_state.PAUSED,