conf: domain: sev: Make 'cbitpos' & 'reducedPhysBits' attrs optional

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@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c26408c4007182daa1ecca09e22392099d7093ef..ae25b9b1bcc8d20c34176768a86f1d0d57a00772 100644 (file)
--- 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 bbe59f61d08f28938424a4a73599b8ecd6ae69e4..efa5ac527bca92ab5da51bf414b4c57d633c87f3 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16764,6 +16764,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
     virDomainSEVDefPtr def;
     unsigned long policy;
     g_autofree char *type = NULL;
+    int rc = -1;
 
     def = g_new0(virDomainSEVDef, 1);
 
@@ -16788,22 +16789,32 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
         goto error;
     }
 
-    if (virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0) {
+    if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("failed to get launch security cbitpos"));
+                       _("failed to get launch security policy"));
         goto error;
     }
 
-    if (virXPathUInt("string(./reducedPhysBits)", ctxt,
-                     &def->reduced_phys_bits) < 0) {
+    /* 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 = true;
+    } else if (rc == -2) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("failed to get launch security reduced-phys-bits"));
+                       _("Invalid format for launch security cbitpos"));
         goto error;
     }
 
-    if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) {
+    rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
+                      &def->reduced_phys_bits);
+    if (rc == 0) {
+        def->haveReducedPhysBits = true;
+    } else if (rc == -2) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("failed to get launch security policy"));
+                       _("Invalid format for launch security "
+                         "reduced-phys-bits"));
         goto error;
     }
 
@@ -28958,9 +28969,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 (file)
index 0000000..378c3b6
--- /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 (file)
index 0000000..41ec4cb
--- /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 f0f6ae95e584c54fb01e0ec32ce913975242bfe0..092e80e9c0d800b756a50018f616983e3841646c 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3319,6 +3319,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");