openstack
/
nova
Code Issues Proposed changes

Ensure non-q35 machine type is not used when booting with SEV 

As explained in the SEV spec[0], SEV needs a q35 machine type in order
to bind all the virtio devices to the PCIe bridge so that they use
virtio 1.0 and not virtio 0.9, since QEMU's iommu_platform feature was
added in virtio 1.0 only.

So add an extra check to be run in the API layer whenever SEV is
requested, to ensure that a machine type *outside* the q35
family (e.g. 'pc', or something like 'pc-i440fx-2.11') is not
explicitly selected in the image via the hw_machine_type property.
Since code in the API layer doesn't run on the compute host, at this
stage we can't check CONF.libvirt.hw_machine_type via libvirt.utils.
A subsequent commit will change the libvirt driver to trigger an
extra, later check on the compute node, although if that late check
fails, the best we can do is to fail the operation on that compute
host, in which case it will potentially be retried on another compute
host.

nova's hardcoded default for x86_64 is 'pc' (which in fact matches
QEMU's current default).  This means that it will be recommended that
SEV-capable compute hosts have CONF.libvirt.hw_machine_type configured
to include 'x86_64=q35', otherwise attempts to boot SEV guests without
the image property 'hw_machine_type=q35' will fail unpleasantly.

In the future it is expected that both of these defaults will change
to 'q35'[1].  Once that happens, x86_64 images will be bootable
without needing to set either the hw_machine_type image property or
CONF.libvirt.hw_machine_type.

While extending the unit tests for invalid combinations of image
properties, add tests for the case where the 'hw_firmware_type'
property is not specified at all; previously the tests only covered
the case where it was set to 'bios'.

[0] http://specs.openstack.org/openstack/nova-specs/specs/train/approved/amd-sev-libvirt-support.html
[1] https://bugs.launchpad.net/nova/+bug/1780138

blueprint: amd-sev-libvirt-support
Change-Id: Ibf66a0b371685c673644493bf12663dbf71fab6c
This commit is contained in:
Adam Spiers 2019-09-04 16:08:20 +01:00
parent f7f5e1846c
commit c1179d1ff2
6 changed files with 154 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
nova/api/openstack/compute/servers.py
View File

@ -75,6 +75,7 @@ INVALID_FLAVOR_IMAGE_EXCEPTIONS = (
exception.InvalidCPUAllocationPolicy,
exception.InvalidCPUThreadAllocationPolicy,
exception.InvalidEmulatorThreadsPolicy,
exception.InvalidMachineType,
exception.InvalidNUMANodesNumber,
exception.InvalidRequest,
exception.MemoryPageSizeForbidden,

5
nova/exception.py
View File

@ -1977,6 +1977,11 @@ class InvalidHypervisorVirtType(Invalid):
"recognised")
class InvalidMachineType(Invalid):
msg_fmt = _("Machine type '%(mtype)s' is not compatible with image "
"%(image_name)s (%(image_id)s): %(reason)s")
class InvalidVirtualMachineMode(Invalid):
msg_fmt = _("Virtual machine mode '%(vmmode)s' is not recognised")

9
nova/tests/unit/api/openstack/compute/test_serversV21.py
View File

@ -6140,6 +6140,15 @@ class ServersControllerCreateTest(test.TestCase):
self.controller.create,
self.req, body=self.body)
@mock.patch('nova.virt.hardware.get_mem_encryption_constraint',
side_effect=exception.InvalidMachineType(
message="fake conflict reason"))
def test_create_instance_raise_invalid_machine_type(
self, mock_conflict):
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create,
self.req, body=self.body)
@mock.patch('nova.virt.hardware.numa_get_constraints',
side_effect=exception.ImageCPUPinningForbidden())
def test_create_instance_raise_image_cpu_pinning_forbidden(

6
nova/tests/unit/scheduler/test_utils.py
View File

@ -1317,7 +1317,9 @@ class TestEncryptedMemoryTranslation(TestUtilsBase):
'hw:mem_encryption extra spec',
{'hw:mem_encryption': extra_spec},
image=objects.ImageMeta(
id='005249be-3c2f-4351-9df7-29bb13c21b14',
properties=objects.ImageMetaProps(
hw_machine_type='q35',
hw_firmware_type='uefi'))
)
@ -1327,8 +1329,10 @@ class TestEncryptedMemoryTranslation(TestUtilsBase):
'hw_mem_encryption image property',
{},
image=objects.ImageMeta(
id='005249be-3c2f-4351-9df7-29bb13c21b14',
name=self.image_name,
properties=objects.ImageMetaProps(
hw_machine_type='q35',
hw_firmware_type='uefi',
hw_mem_encryption=image_prop))
)
@ -1341,8 +1345,10 @@ class TestEncryptedMemoryTranslation(TestUtilsBase):
'hw_mem_encryption image property',
{'hw:mem_encryption': extra_spec},
image=objects.ImageMeta(
id='005249be-3c2f-4351-9df7-29bb13c21b14',
name=self.image_name,
properties=objects.ImageMetaProps(
hw_machine_type='q35',
hw_firmware_type='uefi',
hw_mem_encryption=image_prop))
)

123
nova/tests/unit/virt/test_hardware.py
View File

@ -3798,32 +3798,50 @@ class MemEncryptionFlavorImageConflictTestCase(test.NoDBTestCase):
)
class MemEncryptionRequestedWithoutUEFITestCase(test.NoDBTestCase):
class MemEncryptionRequestedInvalidImagePropsTestCase(test.NoDBTestCase):
flavor_name = 'm1.faketiny'
image_name = 'fakecirros'
image_id = '7ec4448e-f3fd-44b1-b172-9a7980f0f29f'
def _test_encrypted_memory_support_no_uefi(self, extra_spec, image_prop,
requesters):
def _test_encrypted_memory_support_raises(self, enc_extra_spec,
enc_image_prop, image_props,
error_data):
extra_specs = {}
if extra_spec:
extra_specs['hw:mem_encryption'] = extra_spec
if enc_extra_spec:
extra_specs['hw:mem_encryption'] = enc_extra_spec
flavor = objects.Flavor(name=self.flavor_name, extra_specs=extra_specs)
if enc_image_prop:
image_props['hw_mem_encryption'] = enc_image_prop
image_meta = fake_image_obj(
{'name': self.image_name}, {'hw_firmware_type': 'bios'},
{'hw_mem_encryption': True} if image_prop else {})
error = (
"Memory encryption requested by %(requesters)s but image "
"%(image_name)s doesn't have 'hw_firmware_type' property "
"set to 'uefi'"
)
exc = self.assertRaises(
exception.FlavorImageConflict,
hw.get_mem_encryption_constraint,
flavor, image_meta
)
{'id': self.image_id, 'name': self.image_name},
{}, image_props)
exc = self.assertRaises(self.expected_exception,
hw.get_mem_encryption_constraint,
flavor, image_meta)
self.assertEqual(self.expected_error % error_data, str(exc))
class MemEncryptionRequestedWithoutUEFITestCase(
MemEncryptionRequestedInvalidImagePropsTestCase):
expected_exception = exception.FlavorImageConflict
expected_error = (
"Memory encryption requested by %(requesters)s but image "
"%(image_name)s doesn't have 'hw_firmware_type' property "
"set to 'uefi'"
)
def _test_encrypted_memory_support_no_uefi(self, enc_extra_spec,
enc_image_prop, requesters):
error_data = {'requesters': requesters,
'image_name': self.image_name}
self.assertEqual(error % error_data, str(exc))
for image_props in ({},
{'hw_machine_type': 'q35'},
{'hw_firmware_type': 'bios'},
{'hw_machine_type': 'q35',
'hw_firmware_type': 'bios'}):
self._test_encrypted_memory_support_raises(enc_extra_spec,
enc_image_prop,
image_props, error_data)
def test_flavor_requires_encrypted_memory_support_no_uefi(self):
for extra_spec in ('1', 'true', 'True'):
@ -3847,28 +3865,73 @@ class MemEncryptionRequestedWithoutUEFITestCase(test.NoDBTestCase):
% (self.flavor_name, self.image_name))
class MemEncryptionRequestedWithInvalidMachineTypeTestCase(
MemEncryptionRequestedInvalidImagePropsTestCase):
expected_exception = exception.InvalidMachineType
expected_error = (
"Machine type '%(mtype)s' is not compatible with image %(image_name)s "
"(%(image_id)s): q35 type is required for SEV to work")
def _test_encrypted_memory_support_pc(self, enc_extra_spec,
enc_image_prop):
error_data = {'image_id': self.image_id,
'image_name': self.image_name,
'mtype': 'pc'}
image_props = {'hw_firmware_type': 'uefi',
'hw_machine_type': 'pc'}
self._test_encrypted_memory_support_raises(enc_extra_spec,
enc_image_prop,
image_props, error_data)
def test_flavor_requires_encrypted_memory_support_pc(self):
for extra_spec in ('1', 'true', 'True'):
self._test_encrypted_memory_support_pc(extra_spec, None)
def test_image_requires_encrypted_memory_support_pc(self):
for image_prop in ('1', 'true', 'True'):
self._test_encrypted_memory_support_pc(None, image_prop)
def test_flavor_image_require_encrypted_memory_support_pc(self):
for extra_spec in ('1', 'true', 'True'):
for image_prop in ('1', 'true', 'True'):
self._test_encrypted_memory_support_pc(
extra_spec, image_prop)
class MemEncryptionRequiredTestCase(test.NoDBTestCase):
flavor_name = "m1.faketiny"
image_name = 'fakecirros'
image_id = '8a71a380-be23-47eb-9e72-9998586a0268'
@mock.patch.object(hw, 'LOG')
def _test_encrypted_memory_support_required(self, extra_specs,
image_props,
requesters, mock_log):
flavor = objects.Flavor(name=self.flavor_name, extra_specs=extra_specs)
image_meta = objects.ImageMeta(name=self.image_name,
properties=image_props)
image_props['hw_firmware_type'] = 'uefi'
self.assertTrue(hw.get_mem_encryption_constraint(flavor, image_meta))
mock_log.debug.assert_has_calls([
mock.call("Memory encryption requested by %s", requesters)
])
def _test_get_mem_encryption_constraint():
flavor = objects.Flavor(name=self.flavor_name,
extra_specs=extra_specs)
image_meta = objects.ImageMeta.from_dict({
'id': self.image_id,
'name': self.image_name,
'properties': image_props})
self.assertTrue(hw.get_mem_encryption_constraint(flavor,
image_meta))
mock_log.debug.assert_has_calls([
mock.call("Memory encryption requested by %s", requesters)
])
_test_get_mem_encryption_constraint()
for mtype in ('q35', 'pc-q35-2.1'):
image_props['hw_machine_type'] = mtype
_test_get_mem_encryption_constraint()
def test_require_encrypted_memory_support_extra_spec(self):
for extra_spec in ('1', 'true', 'True'):
self._test_encrypted_memory_support_required(
{'hw:mem_encryption': extra_spec},
objects.ImageMetaProps(hw_firmware_type='uefi'),
{},
"hw:mem_encryption extra spec in %s flavor" % self.flavor_name
)
@ -3876,9 +3939,7 @@ class MemEncryptionRequiredTestCase(test.NoDBTestCase):
for image_prop in ('1', 'true', 'True'):
self._test_encrypted_memory_support_required(
{},
objects.ImageMetaProps(
hw_mem_encryption=image_prop,
hw_firmware_type='uefi'),
{'hw_mem_encryption': image_prop},
"hw_mem_encryption property of image %s" % self.image_name
)
@ -3887,9 +3948,7 @@ class MemEncryptionRequiredTestCase(test.NoDBTestCase):
for image_prop in ('1', 'true', 'True'):
self._test_encrypted_memory_support_required(
{'hw:mem_encryption': extra_spec},
objects.ImageMetaProps(
hw_mem_encryption=image_prop,
hw_firmware_type='uefi'),
{'hw_mem_encryption': image_prop},
"hw:mem_encryption extra spec in %s flavor and "
"hw_mem_encryption property of image %s" %
(self.flavor_name, self.image_name)

43
nova/virt/hardware.py
View File

@ -1153,9 +1153,17 @@ def get_mem_encryption_constraint(flavor, image_meta):
2) the flavor and/or image request memory encryption, but the
image is missing hw_firmware_type=uefi
3) the flavor and/or image request memory encryption, but the
machine type is set to a value which does not contain 'q35'
This is called from the API layer, so get_machine_type() cannot be
called since it relies on being run from the compute node in order
to retrieve CONF.libvirt.hw_machine_type.
:param instance_type: Flavor object
:param image: an ImageMeta object
:raises: nova.exception.FlavorImageConflict
:raises: nova.exception.InvalidMachineType
:returns: boolean indicating whether encryption of guest memory
was requested
"""
@ -1188,6 +1196,7 @@ def get_mem_encryption_constraint(flavor, image_meta):
image_meta.name)
_check_mem_encryption_uses_uefi_image(requesters, image_meta)
_check_mem_encryption_machine_type(image_meta)
LOG.debug("Memory encryption requested by %s", " and ".join(requesters))
return True
@ -1215,7 +1224,7 @@ def _check_for_mem_encryption_requirement_conflicts(
def _check_mem_encryption_uses_uefi_image(requesters, image_meta):
if image_meta.properties.hw_firmware_type == 'uefi':
if image_meta.properties.get('hw_firmware_type') == 'uefi':
return
emsg = _(
@ -1227,6 +1236,38 @@ def _check_mem_encryption_uses_uefi_image(requesters, image_meta):
raise exception.FlavorImageConflict(emsg % data)
def _check_mem_encryption_machine_type(image_meta):
# NOTE(aspiers): As explained in the SEV spec, SEV needs a q35
# machine type in order to bind all the virtio devices to the PCIe
# bridge so that they use virtio 1.0 and not virtio 0.9, since
# QEMU's iommu_platform feature was added in virtio 1.0 only:
#
# http://specs.openstack.org/openstack/nova-specs/specs/train/approved/amd-sev-libvirt-support.html
#
# So if the image explicitly requests a machine type which is not
# in the q35 family, raise an exception.
#
# Note that this check occurs at API-level, therefore we can't
# check here what value of CONF.libvirt.hw_machine_type may have
# been configured on the compute node.
mach_type = image_meta.properties.get('hw_machine_type')
# If hw_machine_type is not specified on the image and is not
# configured correctly on SEV compute nodes, then a separate check
# in the driver will catch that and potentially retry on other
# compute nodes.
if mach_type is None:
return
# Could be something like pc-q35-2.11 if a specific version of the
# machine type is required, so do substring matching.
if 'q35' not in mach_type:
raise exception.InvalidMachineType(
mtype=mach_type,
image_id=image_meta.id, image_name=image_meta.name,
reason=_("q35 type is required for SEV to work"))
def _get_numa_pagesize_constraint(flavor, image_meta):
"""Return the requested memory page size