From e0c5d74e82eca646ed46055d224d32eab3a49e17 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Mon, 28 Feb 2022 08:36:52 -0500 Subject: [PATCH] domain: launch_security: simplify defaults and validation * libvirt fills in cbitpos and reducedPhysBits for us * libvirt errors if type is missing * libvirt errors if host/qemu doesn't support sev So drop it all. This simplifies testing because we don't need sev domcaps in place just to generate the XML Signed-off-by: Cole Robinson --- .../cli/compare/virt-install-many-devices.xml | 7 ++ .../virt-install-singleton-config-1.xml | 3 + ...nstall-x86_64-launch-security-sev-full.xml | 81 ------------------- ...irt-install-x86_64-launch-security-sev.xml | 79 ------------------ tests/test_capabilities.py | 4 + tests/test_cli.py | 12 ++- virtinst/cli.py | 1 - virtinst/domain/launch_security.py | 24 +----- virtinst/domcapabilities.py | 2 - 9 files changed, 20 insertions(+), 193 deletions(-) delete mode 100644 tests/data/cli/compare/virt-install-x86_64-launch-security-sev-full.xml delete mode 100644 tests/data/cli/compare/virt-install-x86_64-launch-security-sev.xml diff --git a/tests/data/cli/compare/virt-install-many-devices.xml b/tests/data/cli/compare/virt-install-many-devices.xml index 088d739c..4c002422 100644 --- a/tests/data/cli/compare/virt-install-many-devices.xml +++ b/tests/data/cli/compare/virt-install-many-devices.xml @@ -910,6 +910,13 @@ + + 47 + 1 + 0x0001 + BASE64SESSION + BASE64CERT + baselabel diff --git a/tests/data/cli/compare/virt-install-singleton-config-1.xml b/tests/data/cli/compare/virt-install-singleton-config-1.xml index 468f3686..56ce94dd 100644 --- a/tests/data/cli/compare/virt-install-singleton-config-1.xml +++ b/tests/data/cli/compare/virt-install-singleton-config-1.xml @@ -61,5 +61,8 @@ + + 0x03 + diff --git a/tests/data/cli/compare/virt-install-x86_64-launch-security-sev-full.xml b/tests/data/cli/compare/virt-install-x86_64-launch-security-sev-full.xml deleted file mode 100644 index 4788aac9..00000000 --- a/tests/data/cli/compare/virt-install-x86_64-launch-security-sev-full.xml +++ /dev/null @@ -1,81 +0,0 @@ - - vm1 - 00000000-1111-2222-3333-444444444444 - 65536 - 65536 - 1 - - hvm - /usr/share/edk2/ovmf/OVMF_CODE.fd - - - - - - - - - - - - - - - - - - - /usr/bin/qemu-system-x86_64 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 47 - 1 - 0x0001 - BASE64SESSION - BASE64CERT - - diff --git a/tests/data/cli/compare/virt-install-x86_64-launch-security-sev.xml b/tests/data/cli/compare/virt-install-x86_64-launch-security-sev.xml deleted file mode 100644 index 612c66e2..00000000 --- a/tests/data/cli/compare/virt-install-x86_64-launch-security-sev.xml +++ /dev/null @@ -1,79 +0,0 @@ - - vm1 - 00000000-1111-2222-3333-444444444444 - 65536 - 65536 - 1 - - hvm - /usr/share/edk2/ovmf/OVMF_CODE.fd - - - - - - - - - - - - - - - - - - - /usr/bin/qemu-system-x86_64 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 47 - 1 - 0x03 - - diff --git a/tests/test_capabilities.py b/tests/test_capabilities.py index 7deadd24..d102e51b 100644 --- a/tests/test_capabilities.py +++ b/tests/test_capabilities.py @@ -93,6 +93,10 @@ def testDomainCapabilitiesx86(): assert caps.supports_filesystem_virtiofs() assert caps.supports_memorybacking_memfd() + xml = open(DATADIR + "/kvm-x86_64-domcaps-amd-sev.xml").read() + caps = DomainCapabilities(utils.URIs.open_testdriver_cached(), xml) + assert caps.supports_sev_launch_security() + def testDomainCapabilitiesAArch64(): xml = open(DATADIR + "/kvm-aarch64-domcaps.xml").read() diff --git a/tests/test_cli.py b/tests/test_cli.py index 04e41026..b19869ad 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -765,6 +765,9 @@ source.reservations.managed=no,source.reservations.source.type=unix,source.reser --seclabel type=dynamic,label=012:345 +--launchSecurity type=sev,reducedPhysBits=1,policy=0x0001,cbitpos=47,dhCert=BASE64CERT,session=BASE64SESSION,kernelHashes=yes + + --qemu-commandline env=DISPLAY=:0.1 --qemu-commandline="-display gtk,gl=on" --qemu-commandline="-device vfio-pci,addr=05.0,sysfsdev=/sys/class/mdev_bus/0000:00:02.0/f321853c-c584-4a6b-b99a-3eee22a3919c" @@ -796,6 +799,7 @@ c.add_compare( "--sysinfo host " # special `--sysinfo host` handling "--noapic --noacpi " # feature backcompat "--boot uefi,cdrom,fd,hd,network,menu=on " # uefi for default devices, + old style bootorder +"--launchSecurity sev " # sev defaults # Disabling all the default device setup """ @@ -1098,6 +1102,7 @@ c.add_invalid("--disk none --location nfs:example.com/fake --nonetworks", grep=" c.add_invalid("--disk none --boot network --machine foobar", grep="domain type None with machine 'foobar'") c.add_invalid("--nodisks --boot network --arch mips --virt-type kvm", grep="any virtualization options for architecture 'mips'") c.add_invalid("--nodisks --boot network --paravirt --arch mips", grep=" 'xen' for architecture 'mips'") +c.add_invalid("--osinfo generic --launchSecurity sev --connect " + utils.URIs.kvm_amd_sev, grep="SEV launch security requires a Q35 UEFI machine") @@ -1171,13 +1176,6 @@ c.add_compare("--connect %(URI-KVM-AARCH64)s --osinfo fedora30 --arch aarch64 -- ################# c = vinst.add_category("kvm-x86_64-launch-security", "--disk none --noautoconsole --osinfo generic") -c.add_compare("--boot uefi --machine q35 --launchSecurity type=sev,reducedPhysBits=1,policy=0x0001,cbitpos=47,dhCert=BASE64CERT,session=BASE64SESSION,kernelHashes=yes --connect " + utils.URIs.kvm_amd_sev, "x86_64-launch-security-sev-full") # Full cmdline -c.add_compare("--boot uefi --machine q35 --launchSecurity sev --connect " + utils.URIs.kvm_amd_sev, "x86_64-launch-security-sev") # Fill in platform data from domcaps -c.add_valid("--boot uefi --machine q35 --launchSecurity sev,reducedPhysBits=1,cbitpos=47 --connect " + utils.URIs.kvm_amd_sev) # Default policy == 0x0003 will be used -c.add_valid("--boot firmware=efi --machine q35 --launchSecurity sev,reducedPhysBits=1,cbitpos=47 --connect " + utils.URIs.kvm_amd_sev) # Default policy == 0x0003 will be used -c.add_invalid("--launchSecurity policy=0x0001 --connect " + utils.URIs.kvm_amd_sev, grep="Missing mandatory attribute 'type'") -c.add_invalid("--boot uefi --launchSecurity sev --connect " + utils.URIs.kvm_amd_sev, grep="SEV launch security requires a Q35 UEFI machine") -c.add_invalid("--boot uefi --machine q35 --launchSecurity sev,policy=0x0001 --connect " + utils.URIs.kvm_x86, grep="SEV launch security is not supported") # Fail with no SEV capabilities diff --git a/virtinst/cli.py b/virtinst/cli.py index f42cb5d9..f85b1da3 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -892,7 +892,6 @@ def add_guest_xml_options(geng): ParserLaunchSecurity.register() geng.add_argument("--launchSecurity", "--launchsecurity", action="append", help=_("Configure VM launch security (e.g. SEV memory encryption). Ex:\n" - "--launchSecurity type=sev,cbitpos=47,reducedPhysBits=1,policy=0x0001,dhCert=BASE64CERT\n" "--launchSecurity sev")) diff --git a/virtinst/domain/launch_security.py b/virtinst/domain/launch_security.py index 03c03dfe..7af71811 100644 --- a/virtinst/domain/launch_security.py +++ b/virtinst/domain/launch_security.py @@ -18,27 +18,10 @@ class DomainLaunchSecurity(XMLBuilder): dhCert = XMLProperty("./dhCert") kernelHashes = XMLProperty("./@kernelHashes", is_yesno=True) - def is_sev(self): - return self.type == "sev" - - def validate(self): - if not self.type: - raise RuntimeError(_("Missing mandatory attribute 'type'")) - def _set_defaults_sev(self, guest): - # SeaBIOS doesn't have support for SEV. Q35 defaults to virtio 1.0, - # which we need so let's not go through the 'virtio-transitional' - # exercise for pc-i440fx to make SEV work, AMD recommends Q35 anyway - # NOTE: at some point both of these platform checks should be put in - # validate(), once that accepts the 'guest' instance if not guest.os.is_q35() or not guest.is_uefi(): raise RuntimeError(_("SEV launch security requires a Q35 UEFI machine")) - # libvirt or QEMU might not support SEV - domcaps = guest.lookup_domcaps() - if not domcaps.supports_sev_launch_security(): - raise RuntimeError(_("SEV launch security is not supported on this platform")) - # 'policy' is a mandatory 4-byte argument for the SEV firmware, # if missing, let's use 0x03 which, according to the table at # https://libvirt.org/formatdomain.html#launchSecurity: @@ -47,11 +30,6 @@ class DomainLaunchSecurity(XMLBuilder): if self.policy is None: self.policy = "0x03" - if self.cbitpos is None: - self.cbitpos = domcaps.features.sev.cbitpos - if self.reducedPhysBits is None: - self.reducedPhysBits = domcaps.features.sev.reducedPhysBits - def set_defaults(self, guest): - if self.is_sev(): + if self.type == "sev": return self._set_defaults_sev(guest) diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py index 431e7dc7..3ebc409d 100644 --- a/virtinst/domcapabilities.py +++ b/virtinst/domcapabilities.py @@ -93,8 +93,6 @@ def _make_capsblock(xml_root_name): class _SEV(XMLBuilder): XML_NAME = "sev" supported = XMLProperty("./@supported", is_yesno=True) - cbitpos = XMLProperty("./cbitpos", is_int=True) - reducedPhysBits = XMLProperty("./reducedPhysBits", is_int=True) #############################