On 05/28/2018 05:28 AM, Erik Skultety wrote:
On Wed, May 23, 2018 at 04:18:27PM -0500, Brijesh Singh wrote:
Extend hypervisor capabilities to include sev feature. When available,
hypervisor supports launching an encrypted VM on AMD platform. The
sev feature tag provides additional details like Platform Diffie-Hellman
(PDH) key and certificate chain which can be used by the guest owner to
establish a cryptographic session with the SEV firmware to negotiate
keys used for attestation or to provide secret during launch.
Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
---
...and this one should be IMHO named
conf: Expose SEV in domain capabilities
docs/formatdomaincaps.html.in | 40 ++++++++++++++++++++++++++++++++++++++++
docs/schemas/domaincaps.rng | 20 ++++++++++++++++++++
src/conf/domain_capabilities.c | 20 ++++++++++++++++++++
src/conf/domain_capabilities.h | 1 +
src/qemu/qemu_capabilities.c | 2 ++
5 files changed, 83 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index b68ae4b4f1f3..f37b059ba6b1 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -434,6 +434,12 @@
</enum>
</gic>
<vmcoreinfo supported='yes'/>
+ <sev>
+ <pdh>UWxKSlNrVlRTRk5KVGtkSVFVMUU=</pdh>
+
<cert-chain>VVd4S1NsTnJWbFJUUms1S1ZHdGtTVkZWTVVVPQ==</cert-chain>
+ <cbitpos>47</cbitpos>
+ <reduced-phys-bits>1</reduced-phys-bits>
+ </sev>
</features>
</domainCapabilities>
</pre>
@@ -462,5 +468,39 @@
<p>Reports whether the vmcoreinfo feature can be enabled</p>
+ <h4><a id="elementsSEV">SEV capabilities</a></h4>
+
+ <p>AMD Secure Encrypted Virtualization (SEV) capabilities are exposed under
+ the <code>sev</code> element.
+ SEV is an extension to the AMD-V architecture which supports running
+ virtual machines (VMs) under the control of a hypervisor. When supported,
+ guest owner can create a VM whose memory contents will be transparently
+ encrypted with a key unique to that VM.</p>
+
+ <p>
+ For more details on SEV feature see:
+ <a
href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf";>
+ SEV API spec</a> and <a
href="http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf";>
+ SEV White Paper</a>
+ </p>
+
+ <dl>
+ <dt><code>pdh</code></dt>
+ <dd>A base64 encoded platform Diffie-Hellman public key which can be
+ exported to remote entities that desire to establish a secure transport
+ context with the SEV platform in order to transmit data securely.</dd>
+ <dt><code>cert-chain</code></dt>
+ <dd> A base64 encoded platform certificate chain that includes the
platform
+ endorsement key (PEK), owners certificate authority (OCD), and chip
+ endorsement key (CEK).</dd>
+ <dt><code>cbitpos</code></dt>
+ <dd>When memory encryption is enabled, one of the physical address bits
+ (aka the C-bit) is utilized to mark if a memory page is protected. The
+ C-bit position is Hypervisor dependent.</dd>
+ <dt><code>reduced-phys-bits</code></dt>
+ <dd>When memory encryption is enabled, we lose certain bits in physical
+ address space. The number of bits we lose is hypervisor dependent.</dd>
+ </dl>
+
</body>
</html>
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 5913d711a3fe..26265645b82a 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -184,6 +184,9 @@
<interleave>
<ref name='gic'/>
<ref name='vmcoreinfo'/>
+ <optional>
+ <ref name='sev'/>
needs 1 more level of indent...
Noted.
+ </optional>
</interleave>
</element>
</define>
@@ -201,6 +204,23 @@
</element>
</define>
+ <define name='sev'>
+ <element name='sev'>
+ <element name='pdh'>
+ <data type='string'/>
+ </element>
+ <element name='cert-chain'>
+ <data type='string'/>
+ </element>
+ <element name='cbitpos'>
+ <data type='unsignedInt'/>
+ </element>
+ <element name='reduced-phys-bits'>
+ <data type='unsignedInt'/>
+ </element>
+ </element>
+ </define>
+
<define name='value'>
<zeroOrMore>
<element name='value'>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 6e2ab0a28796..3b767c45cbb3 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -542,6 +542,25 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,
FORMAT_EPILOGUE(gic);
}
+static void
+virDomainCapsFeatureSEVFormat(virBufferPtr buf,
+ virSEVCapabilityPtr const sev)
+{
+ if (!sev)
+ return;
+
+ virBufferAddLit(buf, "<sev supported='yes'>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
+ virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n",
+ sev->reduced_phys_bits);
+ virBufferEscapeString(buf, "<pdh>%s</pdh>\n", sev->pdh);
+ virBufferEscapeString(buf, "<cert-chain>%s</cert-chain>\n",
+ sev->cert_chain);
As I said, I have to agree with Dan here that reporting the 8k string in
capabilities is a good idea, so we can store it, we just wouldn't format it
^here...
Sure, if decide to go with new APIs then this code need to be reworked.
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</sev>\n");
+}
+
char *
virDomainCapsFormat(virDomainCapsPtr const caps)
@@ -585,6 +604,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
virDomainCapsFeatureGICFormat(&buf, &caps->gic);
virBufferAsprintf(&buf, "<vmcoreinfo supported='%s'/>\n",
caps->vmcoreinfo ? "yes" : "no");
+ virDomainCapsFeatureSEVFormat(&buf, caps->sev);
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</features>\n");
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index c1093234ceb8..e33bef525ef4 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -172,6 +172,7 @@ struct _virDomainCaps {
virDomainCapsFeatureGIC gic;
bool vmcoreinfo;
+ virSEVCapabilityPtr sev;
/* add new domain features here */
};
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 49b74f7e12c1..3345b09fa384 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4998,6 +4998,8 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
return -1;
+
+ domCaps->sev = qemuCaps->sevCapabilities;
domCaps is a one-time-use object needed when you format the qemuCaps into
domCaps which is returned to the user and is therefore cleaned up on the API
exit, so ^this shouldn't be considered safe and you should create
virQEMUCapsFillDomainFeatureSEVCaps where you allocate a fresh
domCaps->sev structure and then you'll need to put
virSEVCapabilityFree I mentioned in patch 1 to virDomainCapsDispose so as not
to leak any memory related to copying the SEV data.
I had something very similar in v2 or v3 but was asked in one of review
feedback to drop the function and simply save the pointers. I will
revisit and re-introduce the function.
-Brijesh
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
