99cbc7
From 05b6b6d0d034e0a2ecb9b826bf5052a3bada218d Mon Sep 17 00:00:00 2001
99cbc7
Message-Id: <05b6b6d0d034e0a2ecb9b826bf5052a3bada218d@dist-git>
99cbc7
From: Erik Skultety <eskultet@redhat.com>
99cbc7
Date: Mon, 20 Aug 2018 17:18:53 +0200
99cbc7
Subject: [PATCH] qemu: caps: Format SEV platform data into qemuCaps cache
99cbc7
MIME-Version: 1.0
99cbc7
Content-Type: text/plain; charset=UTF-8
99cbc7
Content-Transfer-Encoding: 8bit
99cbc7
99cbc7
Since we're not saving the platform-specific data into a cache, we're
99cbc7
not going to populate the structure, which in turn will cause a crash
99cbc7
upon calling virNodeGetSEVInfo because of a NULL pointer dereference.
99cbc7
Ultimately, we should start caching this data along with host-specific
99cbc7
capabilities like NUMA and SELinux stuff into a separate cache, but for
99cbc7
the time being, this is a semi-proper fix for a potential crash.
99cbc7
99cbc7
Backtrace (requires libvirtd restart to load qemu caps from cache):
99cbc7
    #0 qemuGetSEVInfoToParams
99cbc7
    #1 qemuNodeGetSEVInfo
99cbc7
    #2 virNodeGetSEVInfo
99cbc7
    #3 remoteDispatchNodeGetSevInfo
99cbc7
    #4 remoteDispatchNodeGetSevInfoHelper
99cbc7
    #5 virNetServerProgramDispatchCall
99cbc7
    #6 virNetServerProgramDispatch
99cbc7
    #7 virNetServerProcessMsg
99cbc7
    #8 virNetServerHandleJob
99cbc7
    #9 virThreadPoolWorker
99cbc7
    #10 virThreadHelper
99cbc7
99cbc7
https: //bugzilla.redhat.com/show_bug.cgi?id=1612009
99cbc7
Signed-off-by: Erik Skultety <eskultet@redhat.com>
99cbc7
Acked-by: Peter Krempa <pkrempa@redhat.com>
99cbc7
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
99cbc7
(cherry picked from commit 77f51ab52049734d80a8ccb79b80189c7fb95c41)
99cbc7
99cbc7
https://bugzilla.redhat.com/show_bug.cgi?id=1612009
99cbc7
https://bugzilla.redhat.com/show_bug.cgi?id=1619150
99cbc7
99cbc7
Amend:
99cbc7
    - fixed the VIR_AUTOPTR bits which downstream doesn't support
99cbc7
    and wouldn't compile
99cbc7
99cbc7
Signed-off-by: Erik Skultety <eskultet@redhat.com>
99cbc7
Reviewed-by: Ján Tomko <jtomko@redhat.com>
99cbc7
---
99cbc7
 src/qemu/qemu_capabilities.c                  | 109 ++++++++++++++++++
99cbc7
 .../qemu_2.12.0.x86_64.xml                    |   5 +-
99cbc7
 .../caps_2.12.0.x86_64.xml                    |   6 +
99cbc7
 3 files changed, 119 insertions(+), 1 deletion(-)
99cbc7
99cbc7
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
99cbc7
index 470f25a5a6..4f2051a2bb 100644
99cbc7
--- a/src/qemu/qemu_capabilities.c
99cbc7
+++ b/src/qemu/qemu_capabilities.c
99cbc7
@@ -1573,6 +1573,30 @@ virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData)
99cbc7
 }
99cbc7
 
99cbc7
 
99cbc7
+static int
99cbc7
+virQEMUCapsSEVInfoCopy(virSEVCapabilityPtr *dst,
99cbc7
+                       virSEVCapabilityPtr src)
99cbc7
+{
99cbc7
+    int ret = -1;
99cbc7
+    virSEVCapabilityPtr tmp = NULL;
99cbc7
+
99cbc7
+    if (VIR_ALLOC(tmp) < 0 ||
99cbc7
+        VIR_STRDUP(tmp->pdh, src->pdh) < 0 ||
99cbc7
+        VIR_STRDUP(tmp->cert_chain, src->cert_chain) < 0)
99cbc7
+        goto cleanup;
99cbc7
+
99cbc7
+    tmp->cbitpos = src->cbitpos;
99cbc7
+    tmp->reduced_phys_bits = src->reduced_phys_bits;
99cbc7
+
99cbc7
+    VIR_STEAL_PTR(*dst, tmp);
99cbc7
+
99cbc7
+    ret = 0;
99cbc7
+ cleanup:
99cbc7
+    virSEVCapabilitiesFree(tmp);
99cbc7
+    return ret;
99cbc7
+}
99cbc7
+
99cbc7
+
99cbc7
 virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
99cbc7
 {
99cbc7
     virQEMUCapsPtr ret = virQEMUCapsNew();
99cbc7
@@ -1635,6 +1659,11 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
99cbc7
     for (i = 0; i < qemuCaps->ngicCapabilities; i++)
99cbc7
         ret->gicCapabilities[i] = qemuCaps->gicCapabilities[i];
99cbc7
 
99cbc7
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST) &&
99cbc7
+        virQEMUCapsSEVInfoCopy(&ret->sevCapabilities,
99cbc7
+                               qemuCaps->sevCapabilities) < 0)
99cbc7
+        goto error;
99cbc7
+
99cbc7
     return ret;
99cbc7
 
99cbc7
  error:
99cbc7
@@ -3273,6 +3302,62 @@ virQEMUCapsCachePrivFree(void *privData)
99cbc7
 }
99cbc7
 
99cbc7
 
99cbc7
+static int
99cbc7
+virQEMUCapsParseSEVInfo(virQEMUCapsPtr qemuCaps, xmlXPathContextPtr ctxt)
99cbc7
+{
99cbc7
+    int ret = -1;
99cbc7
+    virSEVCapabilityPtr sev = NULL;
99cbc7
+
99cbc7
+    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST))
99cbc7
+        return 0;
99cbc7
+
99cbc7
+    if (virXPathBoolean("boolean(./sev)", ctxt) == 0) {
99cbc7
+        virReportError(VIR_ERR_XML_ERROR, "%s",
99cbc7
+                       _("missing SEV platform data in QEMU "
99cbc7
+                         "capabilities cache"));
99cbc7
+        return -1;
99cbc7
+    }
99cbc7
+
99cbc7
+    if (VIR_ALLOC(sev) < 0)
99cbc7
+        return -1;
99cbc7
+
99cbc7
+    if (virXPathUInt("string(./sev/cbitpos)", ctxt, &sev->cbitpos) < 0) {
99cbc7
+        virReportError(VIR_ERR_XML_ERROR, "%s",
99cbc7
+                       _("missing or malformed SEV cbitpos information "
99cbc7
+                         "in QEMU capabilities cache"));
99cbc7
+        goto cleanup;
99cbc7
+    }
99cbc7
+
99cbc7
+    if (virXPathUInt("string(./sev/reducedPhysBits)", ctxt,
99cbc7
+                     &sev->reduced_phys_bits) < 0) {
99cbc7
+        virReportError(VIR_ERR_XML_ERROR, "%s",
99cbc7
+                       _("missing or malformed SEV reducedPhysBits information "
99cbc7
+                         "in QEMU capabilities cache"));
99cbc7
+        goto cleanup;
99cbc7
+    }
99cbc7
+
99cbc7
+    if (!(sev->pdh = virXPathString("string(./sev/pdh)", ctxt)))  {
99cbc7
+        virReportError(VIR_ERR_XML_ERROR, "%s",
99cbc7
+                       _("missing SEV pdh information "
99cbc7
+                         "in QEMU capabilities cache"));
99cbc7
+        goto cleanup;
99cbc7
+    }
99cbc7
+
99cbc7
+    if (!(sev->cert_chain = virXPathString("string(./sev/certChain)", ctxt))) {
99cbc7
+        virReportError(VIR_ERR_XML_ERROR, "%s",
99cbc7
+                       _("missing SEV certChain information "
99cbc7
+                         "in QEMU capabilities cache"));
99cbc7
+        goto cleanup;
99cbc7
+    }
99cbc7
+
99cbc7
+    VIR_STEAL_PTR(qemuCaps->sevCapabilities, sev);
99cbc7
+    ret = 0;
99cbc7
+ cleanup:
99cbc7
+    virSEVCapabilitiesFree(sev);
99cbc7
+    return ret;
99cbc7
+}
99cbc7
+
99cbc7
+
99cbc7
 /*
99cbc7
  * Parsing a doc that looks like
99cbc7
  *
99cbc7
@@ -3521,6 +3606,9 @@ virQEMUCapsLoadCache(virArch hostArch,
99cbc7
     }
99cbc7
     VIR_FREE(nodes);
99cbc7
 
99cbc7
+    if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0)
99cbc7
+        goto cleanup;
99cbc7
+
99cbc7
     virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
99cbc7
     virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
99cbc7
 
99cbc7
@@ -3638,6 +3726,24 @@ virQEMUCapsFormatCPUModels(virQEMUCapsPtr qemuCaps,
99cbc7
 }
99cbc7
 
99cbc7
 
99cbc7
+static void
99cbc7
+virQEMUCapsFormatSEVInfo(virQEMUCapsPtr qemuCaps, virBufferPtr buf)
99cbc7
+{
99cbc7
+    virSEVCapabilityPtr sev = virQEMUCapsGetSEVCapabilities(qemuCaps);
99cbc7
+
99cbc7
+    virBufferAddLit(buf, "<sev>\n");
99cbc7
+    virBufferAdjustIndent(buf, 2);
99cbc7
+    virBufferAsprintf(buf, "<cbitpos>%u</cbitpos>\n", sev->cbitpos);
99cbc7
+    virBufferAsprintf(buf, "<reducedPhysBits>%u</reducedPhysBits>\n",
99cbc7
+                      sev->reduced_phys_bits);
99cbc7
+    virBufferEscapeString(buf, "<pdh>%s</pdh>\n", sev->pdh);
99cbc7
+    virBufferEscapeString(buf, "<certChain>%s</certChain>\n",
99cbc7
+                          sev->cert_chain);
99cbc7
+    virBufferAdjustIndent(buf, -2);
99cbc7
+    virBufferAddLit(buf, "</sev>\n");
99cbc7
+}
99cbc7
+
99cbc7
+
99cbc7
 char *
99cbc7
 virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps)
99cbc7
 {
99cbc7
@@ -3719,6 +3825,9 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps)
99cbc7
                           emulated ? "yes" : "no");
99cbc7
     }
99cbc7
 
99cbc7
+    if (qemuCaps->sevCapabilities)
99cbc7
+        virQEMUCapsFormatSEVInfo(qemuCaps, &buf;;
99cbc7
+
99cbc7
     virBufferAdjustIndent(&buf, -2);
99cbc7
     virBufferAddLit(&buf, "</qemuCaps>\n");
99cbc7
 
99cbc7
diff --git a/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
99cbc7
index 7a1be4c093..a8d6a4d629 100644
99cbc7
--- a/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
99cbc7
+++ b/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
99cbc7
@@ -142,6 +142,9 @@
99cbc7
     <gic supported='no'/>
99cbc7
     <vmcoreinfo supported='yes'/>
99cbc7
     <genid supported='yes'/>
99cbc7
-    <sev supported='no'/>
99cbc7
+    <sev supported='yes'>
99cbc7
+      <cbitpos>47</cbitpos>
99cbc7
+      <reducedPhysBits>1</reducedPhysBits>
99cbc7
+    </sev>
99cbc7
   </features>
99cbc7
 </domainCapabilities>
99cbc7
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
99cbc7
index 78889facce..f0dc082640 100644
99cbc7
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
99cbc7
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
99cbc7
@@ -1254,4 +1254,10 @@
99cbc7
   <machine name='pc-0.11' hotplugCpus='yes' maxCpus='255'/>
99cbc7
   <machine name='pc-0.12' hotplugCpus='yes' maxCpus='255'/>
99cbc7
   <machine name='pc-0.10' hotplugCpus='yes' maxCpus='255'/>
99cbc7
+  <sev>
99cbc7
+    <cbitpos>47</cbitpos>
99cbc7
+    <reducedPhysBits>1</reducedPhysBits>
99cbc7
+    <pdh>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</pdh>
99cbc7
+    <certChain>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</certChain>
99cbc7
+  </sev>
99cbc7
 </qemuCaps>
99cbc7
-- 
99cbc7
2.18.0
99cbc7