[libvirt] [PATCH v6 2/9] qemu: introduce SEV feature in hypervisor capabilities
Brijesh Singh
brijesh.singh at amd.com
Tue May 29 15:39:49 UTC 2018
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.singh at 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
More information about the libvir-list
mailing list