[libvirt PATCH 3/3] conf: domain: sev: Make 'cbitpos' & 'reducedPhysBits' attrs optional
Daniel Henrique Barboza
danielhb413 at gmail.com
Wed Oct 14 21:45:13 UTC 2020
On 10/9/20 11:13 AM, Erik Skultety wrote:
> These XML attributes have been mandatory since the introduction of SEV
> support to libvirt. This design decision was based on QEMU's
> requirement for these to be mandatory for migration purposes, as
> differences in these values across platforms must result in the
> pre-migration checks failing (not that migration with SEV works at the
> time of this patch).
>
> Expecting the user to specify these is cumbersome and the same XML
> cannot be re-used across different revisions of SEV. Since
> we have SEV platform information saved in QEMU capabilities, we can
> make the attributes optional and should fill them in automatically
> in the QEMU driver right before starting it.
>
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/57
>
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
Already considering that changed that was squashed in later:
Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> docs/schemas/domaincommon.rng | 16 ++++---
> src/conf/domain_conf.c | 46 ++++++++++++-------
> ...v-missing-platform-info.x86_64-2.12.0.args | 37 +++++++++++++++
> ...nch-security-sev-missing-platform-info.xml | 35 ++++++++++++++
> tests/qemuxml2argvtest.c | 1 +
> 5 files changed, 113 insertions(+), 22 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args
> create mode 100644 tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.xml
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 7d4b105981..9963fad4a6 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -467,12 +467,16 @@
> <value>sev</value>
> </attribute>
> <interleave>
> - <element name="cbitpos">
> - <data type="unsignedInt"/>
> - </element>
> - <element name="reducedPhysBits">
> - <data type="unsignedInt"/>
> - </element>
> + <optional>
> + <element name="cbitpos">
> + <data type="unsignedInt"/>
> + </element>
> + </optional>
> + <optional>
> + <element name="reducedPhysBits">
> + <data type="unsignedInt"/>
> + </element>
> + </optional>
> <element name="policy">
> <ref name="hexuint"/>
> </element>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 51efeb0e42..648a47ac84 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -16756,6 +16756,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
> virDomainSEVDefPtr def;
> unsigned long policy;
> g_autofree char *type = NULL;
> + int rc = -1;
>
> def = g_new0(virDomainSEVDef, 1);
>
> @@ -16780,25 +16781,35 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
> goto error;
> }
>
> - if (virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0) {
> - virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("failed to get launch security cbitpos"));
> - goto error;
> - }
> -
> - if (virXPathUInt("string(./reducedPhysBits)", ctxt,
> - &def->reduced_phys_bits) < 0) {
> - virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("failed to get launch security reduced-phys-bits"));
> - goto error;
> - }
> -
> if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("failed to get launch security policy"));
> goto error;
> }
>
> + /* the following attributes are platform dependent and if missing, we can
> + * autofill them from domain capabilities later
> + */
> + rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos);
> + if (rc == 0) {
> + def->haveCbitpos = VIR_TRISTATE_BOOL_YES;
> + } else if (rc == -2) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Invalid format for launch security cbitpos"));
> + goto error;
> + }
> +
> + rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
> + &def->reduced_phys_bits);
> + if (rc == 0) {
> + def->haveReducedPhysBits = VIR_TRISTATE_BOOL_YES;
> + } else if (rc == -2) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Invalid format for launch security "
> + "reduced-phys-bits"));
> + goto error;
> + }
> +
> def->policy = policy;
> def->dh_cert = virXPathString("string(./dhCert)", ctxt);
> def->session = virXPathString("string(./session)", ctxt);
> @@ -28937,9 +28948,12 @@ virDomainSEVDefFormat(virBufferPtr buf, virDomainSEVDefPtr sev)
> virDomainLaunchSecurityTypeToString(sev->sectype));
> virBufferAdjustIndent(buf, 2);
>
> - virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
> - virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n",
> - sev->reduced_phys_bits);
> + if (sev->haveCbitpos)
> + virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
> +
> + if (sev->haveReducedPhysBits)
> + virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n",
> + sev->reduced_phys_bits);
> virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy);
> if (sev->dh_cert)
> virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert);
> diff --git a/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args b/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args
> new file mode 100644
> index 0000000000..378c3b681c
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args
> @@ -0,0 +1,37 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-QEMUGuest1 \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-x86_64 \
> +-name guest=QEMUGuest1,debug-threads=on \
> +-S \
> +-object secret,id=masterKey0,format=raw,\
> +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> +-machine pc-1.0,accel=kvm,usb=off,dump-guest-core=off,memory-encryption=sev0 \
> +-m 214 \
> +-realtime mlock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot strict=on \
> +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
> +-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x1,\
> +dh-cert-file=/tmp/lib/domain--1-QEMUGuest1/dh_cert.base64,\
> +session-file=/tmp/lib/domain--1-QEMUGuest1/session.base64 \
> +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
> +resourcecontrol=deny \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.xml b/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.xml
> new file mode 100644
> index 0000000000..41ec4cb872
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.xml
> @@ -0,0 +1,35 @@
> +<domain type='kvm'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219100</memory>
> + <currentMemory unit='KiB'>219100</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='x86_64' machine='pc-1.0'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu-system-x86_64</emulator>
> + <disk type='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
> + <controller type='usb' index='0'/>
> + <controller type='ide' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <memballoon model='none'/>
> + </devices>
> + <launchSecurity type='sev'>
> + <policy>0x0001</policy>
> + <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert>
> + <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session>
> + </launchSecurity>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 49567326d4..56247177e1 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -3314,6 +3314,7 @@ mymain(void)
> DO_TEST_CAPS_ARCH_LATEST("vhost-vsock-ccw-auto", "s390x");
>
> DO_TEST_CAPS_VER("launch-security-sev", "2.12.0");
> + DO_TEST_CAPS_VER("launch-security-sev-missing-platform-info", "2.12.0");
>
> DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory");
> DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages");
>
More information about the libvir-list
mailing list