[libvirt] [PATCH v6 2/9] qemu: introduce SEV feature in hypervisor capabilities

Mon May 28 10:28:26 UTC 2018

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...

> +        </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...

> +    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.

Erik