Changeset
docs/formatdomain.html.in           | 120 ++++++++++++++++++++++++++++++++++++
docs/formatdomaincaps.html.in       |  40 ++++++++++++
docs/schemas/domaincaps.rng         |  20 ++++++
docs/schemas/domaincommon.rng       |  39 ++++++++++++
include/libvirt/libvirt-domain.h    |  17 +++++
src/conf/domain_capabilities.c      |  20 ++++++
src/conf/domain_capabilities.h      |  14 +++++
src/conf/domain_conf.c              | 111 +++++++++++++++++++++++++++++++++
src/conf/domain_conf.h              |  27 ++++++++
src/driver-hypervisor.h             |   7 +++
src/libvirt-domain.c                |  50 +++++++++++++++
src/libvirt_public.syms             |   5 ++
src/qemu/qemu_capabilities.c        |  45 ++++++++++++++
src/qemu/qemu_capabilities.h        |   1 +
src/qemu/qemu_capspriv.h            |   4 ++
src/qemu/qemu_command.c             |  33 ++++++++++
src/qemu/qemu_driver.c              |  72 ++++++++++++++++++++++
src/qemu/qemu_monitor.c             |  17 +++++
src/qemu/qemu_monitor.h             |   6 ++
src/qemu/qemu_monitor_json.c        | 105 +++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h        |   5 ++
src/qemu/qemu_process.c             |  91 +++++++++++++++++++++++++++
src/remote/remote_daemon_dispatch.c |  63 +++++++++++++++++++
src/remote/remote_driver.c          |  52 +++++++++++++++-
src/remote/remote_protocol.x        |  22 ++++++-
src/remote_protocol-structs         |  13 ++++
tests/genericxml2xmlindata/sev.xml  |  22 +++++++
tests/genericxml2xmloutdata/sev.xml |  22 +++++++
tests/genericxml2xmltest.c          |   2 +
tests/qemuxml2argvdata/sev.args     |  25 ++++++++
tests/qemuxml2argvdata/sev.xml      |  35 +++++++++++
tests/qemuxml2argvtest.c            |   2 +
tests/qemuxml2xmloutdata/sev.xml    |  39 ++++++++++++
tests/qemuxml2xmltest.c             |   2 +
tools/virsh-domain.c                |  93 ++++++++++++++++++++++++++++
35 files changed, 1239 insertions(+), 2 deletions(-)
create mode 100644 tests/genericxml2xmlindata/sev.xml
create mode 100644 tests/genericxml2xmloutdata/sev.xml
create mode 100644 tests/qemuxml2argvdata/sev.args
create mode 100644 tests/qemuxml2argvdata/sev.xml
create mode 100644 tests/qemuxml2xmloutdata/sev.xml
Git apply log
Switched to a new branch '20180308171208.54369-1-brijesh.singh@amd.com'
Applying: qemu: provide support to query the SEV capability
Applying: qemu: introduce SEV feature in hypervisor capabilities
Applying: conf: introduce launch-security element in domain
Applying: qemu: add support to launch SEV guest
Applying: libvirt: add new public API to get launch security info
Applying: remote: implement the remote protocol for launch security
Applying: qemu_driver: add support to launch security info
Applying: virsh: implement new command for launch security
Applying: tests: extend tests to include sev specific tag parsing
To https://github.com/patchew-project/libvirt
 + 605e473...d982c93 patchew/20180308171208.54369-1-brijesh.singh@amd.com -> patchew/20180308171208.54369-1-brijesh.singh@amd.com (forced update)
Test passed: syntax-check

loading

[libvirt] [PATCH v2 0/9] x86: Secure Encrypted Virtualization (AMD)
Posted by Brijesh Singh, 15 weeks ago
This patch series provides support for launching an encrypted guest using
AMD's new Secure Encrypted  Virtualization (SEV) feature.

SEV is an extension to the AMD-V architecture which supports running
multiple VMs under the control of a hypervisor. When enabled, SEV feature
allows the memory contents of a virtual machine (VM) to be transparently
encrypted with a key unique to the guest VM.

In order to launch SEV guest we need QEMU SEV patch [1].

[1] https://marc.info/?l=kvm&m=152051337301616&w=2

The patch series implements some of recommendation from Daniel [2]

[2] https://www.redhat.com/archives/libvir-list/2017-September/msg00197.html

At very high level the flow looks this:

1. mgmt tool calls virConnectGetDomainCapabilities. This returns an XML document
   that includes the following

   <feature>
    ...
    <sev supported='yes'>
     <cbitpos> </cbitpos>
     <reduced-phys-bits> </reduced-phys-bits>
     <pdh> </pdh>
     <cert-chain> </cert-chain>
   </feature>

  If <sev> is provided then we indicate that hypervisor is capable of launching
  SEV guest. 
  
2. (optional) mgmt tool can provide the PDH and Cert-chain to guest owner in case
   if guest owner wish to establish a secure connection with SEV firmware to
   negotiate a key used for validating the measurement.

3. mgmt tool requests to start a guest calling virCreateXML(), passing VIR_DOMAIN_START_PAUSED.
   The xml would include

   <launch-security type='sev'>
    <cbitpos> </cbitpos>  /* the value is same as what is obtained via virConnectGetDomainCapabilities()
    <reduced-phys-bits> </reduced-phys-bits>  /* the value is same as what is obtained via virConnectGetDomainCapabilities()
    <dh-cert> .. </dh> /* guest owners diffie-hellman key */ (optional)
    <session> ..</session> /* guest owners session blob */ (optional)
    <policy> ..</policy> /* guest policy */ (optional)

4. Libvirt generate the QEMU cli arg to enable the SEV feature, a typical
   args looks like this:

   # $QEMU ..
      -machine memory-encryption=sev0 \
      -object sev-guest,id=sev0,dh-cert-file=<file>....

5. Libvirt generates lifecycle VIR_DOMAIN_EVENT_SUSPENDED_PAUSED event

6. mgmt tool gets the VIR_DOMAIN_EVENT_SUSPENDED_PAUSED and calls virDomainGetLaunchSecretInfo()
   to retrieve the measurement of encrypted memory.

7. (optional) mgmt tool can provide the measurement value to guest owner, which can
   validate the measurement and gives GO/NO-GO answer. If mgmt tool gets GO then
   it resumes the guest otherwise it calls destroy() to kill the guest.

8. mgmt tool resumes the guest

TODO:
* SEV guest require to use DMA apis for the virtio devices. In order to use the DMA
  apis the virtio devices must have this tag

  <driver iommu=on ats=on>

  It is a bit unclear to me where these changes need to go. Do we need to
  modify the libvirt to automatically add these when SEV is enabled or
  we ask mgmt tool to make sure that it creates XML with right tag to enable
  the DMA APIs for virtio devices. I am looking for some suggestions.

Using these patches we have succesfully booted and tested a guest both with and
without SEV enabled.

SEV Firmware API spec is available at:
https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf

Changes since v1:
 * rename <sev> -> <launch-security> for domain
 * add more information about policy and other fields in domaincaps.html
 * split the domain_conf support in two patches
 * add virDomainGetLaunchInfo() to retrieve the SEV measurement
 * extend virsh command to show the domain's launch security information
 * add test cases to validate newly added <launch-security> element
 * fix issues reported with 'make check' and 'make syntax-check'

The complete git tree is available at:
https://github.com/codomania/libvirt/tree/v2

Brijesh Singh (8):
  qemu: provide support to query the SEV capability
  qemu: introduce SEV feature in hypervisor capabilities
  conf: introduce launch-security element in domain
  qemu: add support to launch SEV guest
  libvirt: add new public API to get launch security info
  remote: implement the remote protocol for launch security
  qemu_driver: add support to launch security info
  virsh: implement new command for launch security

Xiaogang Chen (1):
  tests: extend tests to include sev specific tag parsing

 docs/formatdomain.html.in           | 120 ++++++++++++++++++++++++++++++++++++
 docs/formatdomaincaps.html.in       |  40 ++++++++++++
 docs/schemas/domaincaps.rng         |  20 ++++++
 docs/schemas/domaincommon.rng       |  39 ++++++++++++
 include/libvirt/libvirt-domain.h    |  17 +++++
 src/conf/domain_capabilities.c      |  20 ++++++
 src/conf/domain_capabilities.h      |  14 +++++
 src/conf/domain_conf.c              | 111 +++++++++++++++++++++++++++++++++
 src/conf/domain_conf.h              |  27 ++++++++
 src/driver-hypervisor.h             |   7 +++
 src/libvirt-domain.c                |  50 +++++++++++++++
 src/libvirt_public.syms             |   5 ++
 src/qemu/qemu_capabilities.c        |  45 ++++++++++++++
 src/qemu/qemu_capabilities.h        |   1 +
 src/qemu/qemu_capspriv.h            |   4 ++
 src/qemu/qemu_command.c             |  33 ++++++++++
 src/qemu/qemu_driver.c              |  72 ++++++++++++++++++++++
 src/qemu/qemu_monitor.c             |  17 +++++
 src/qemu/qemu_monitor.h             |   6 ++
 src/qemu/qemu_monitor_json.c        | 105 +++++++++++++++++++++++++++++++
 src/qemu/qemu_monitor_json.h        |   5 ++
 src/qemu/qemu_process.c             |  91 +++++++++++++++++++++++++++
 src/remote/remote_daemon_dispatch.c |  63 +++++++++++++++++++
 src/remote/remote_driver.c          |  52 +++++++++++++++-
 src/remote/remote_protocol.x        |  22 ++++++-
 src/remote_protocol-structs         |  13 ++++
 tests/genericxml2xmlindata/sev.xml  |  22 +++++++
 tests/genericxml2xmloutdata/sev.xml |  22 +++++++
 tests/genericxml2xmltest.c          |   2 +
 tests/qemuxml2argvdata/sev.args     |  25 ++++++++
 tests/qemuxml2argvdata/sev.xml      |  35 +++++++++++
 tests/qemuxml2argvtest.c            |   2 +
 tests/qemuxml2xmloutdata/sev.xml    |  39 ++++++++++++
 tests/qemuxml2xmltest.c             |   2 +
 tools/virsh-domain.c                |  93 ++++++++++++++++++++++++++++
 35 files changed, 1239 insertions(+), 2 deletions(-)
 create mode 100644 tests/genericxml2xmlindata/sev.xml
 create mode 100644 tests/genericxml2xmloutdata/sev.xml
 create mode 100644 tests/qemuxml2argvdata/sev.args
 create mode 100644 tests/qemuxml2argvdata/sev.xml
 create mode 100644 tests/qemuxml2xmloutdata/sev.xml

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/9] qemu: provide support to query the SEV capability
Posted by Brijesh Singh, 15 weeks ago
QEMU version >= 2.12 provides support for launching an encrypted VMs on
AMD x86 platform using Secure Encrypted Virtualization (SEV) feature.
This patch adds support to query the SEV capability from the qemu.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 src/conf/domain_capabilities.h | 13 ++++++++
 src/qemu/qemu_capabilities.c   | 43 +++++++++++++++++++++++++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_capspriv.h       |  4 +++
 src/qemu/qemu_monitor.c        |  9 ++++++
 src/qemu/qemu_monitor.h        |  3 ++
 src/qemu/qemu_monitor_json.c   | 73 ++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_monitor_json.h   |  3 ++
 8 files changed, 149 insertions(+)

diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index fa4c1e442f57..83d04d4c8506 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -137,6 +137,19 @@ struct _virDomainCapsCPU {
     virDomainCapsCPUModelsPtr custom;
 };
 
+/*
+ * SEV capabilities
+ */
+typedef struct _virSEVCapability virSEVCapability;
+typedef virSEVCapability *virSEVCapabilityPtr;
+struct _virSEVCapability {
+    char *pdh;
+    char *cert_chain;
+    int cbitpos;
+    int reduced_phys_bits;
+};
+
+
 struct _virDomainCaps {
     virObjectLockable parent;
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b5eb8cf46a52..68e3622a3963 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
               "pl011",
               "machine.pseries.max-cpu-compat",
               "dump-completed",
+              "sev",
     );
 
 
@@ -525,6 +526,8 @@ struct _virQEMUCaps {
     size_t ngicCapabilities;
     virGICCapability *gicCapabilities;
 
+    virSEVCapability *sevCapabilities;
+
     virQEMUCapsHostCPUData kvmCPU;
     virQEMUCapsHostCPUData tcgCPU;
 };
@@ -2811,6 +2814,21 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,
     qemuCaps->ngicCapabilities = ncapabilities;
 }
 
+void
+virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps,
+                              virSEVCapability *capabilities)
+{
+    virSEVCapability *cap = qemuCaps->sevCapabilities;
+
+    if (cap) {
+        VIR_FREE(cap->pdh);
+        VIR_FREE(cap->cert_chain);
+    }
+
+    VIR_FREE(qemuCaps->sevCapabilities);
+
+    qemuCaps->sevCapabilities = capabilities;
+}
 
 static int
 virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps,
@@ -3318,6 +3336,19 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps,
     return 0;
 }
 
+static int
+virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps,
+                                   qemuMonitorPtr mon)
+{
+    virSEVCapability *caps = NULL;
+
+    if (qemuMonitorGetSEVCapabilities(mon, &caps) < 0)
+        return -1;
+
+    virQEMUCapsSetSEVCapabilities(qemuCaps, caps);
+
+    return 0;
+}
 
 bool
 virQEMUCapsCPUFilterFeatures(const char *name,
@@ -4896,6 +4927,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
         virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
     }
 
+    /* no way to query -object sev-guest */
+    if (ARCH_IS_X86(qemuCaps->arch) &&
+        qemuCaps->version >= 2012000) {
+        virQEMUCapsSet(qemuCaps, QEMU_CAPS_SEV);
+    }
+
     if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
         goto cleanup;
 
@@ -4951,6 +4988,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
         virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
         virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE);
 
+    /* Probe for SEV capabilities */
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) {
+        if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
+            virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV);
+    }
+
     ret = 0;
  cleanup:
     return ret;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c2ec2be19311..02acae491ab5 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -444,6 +444,7 @@ typedef enum {
     QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */
     QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */
     QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */
+    QEMU_CAPS_SEV, /* -object sev-guest,... */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index 222f3368e3b6..1fa85cc14f07 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -86,6 +86,10 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,
                               virGICCapability *capabilities,
                               size_t ncapabilities);
 
+void
+virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps,
+                              virSEVCapability *capabilities);
+
 int
 virQEMUCapsParseHelpStr(const char *qemu,
                         const char *str,
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ad5c572aeefb..195248c88ae1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4007,6 +4007,15 @@ qemuMonitorGetGICCapabilities(qemuMonitorPtr mon,
     return qemuMonitorJSONGetGICCapabilities(mon, capabilities);
 }
 
+int
+qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon,
+                              virSEVCapability **capabilities)
+{
+    QEMU_CHECK_MONITOR_JSON(mon);
+
+    return qemuMonitorJSONGetSEVCapabilities(mon, capabilities);
+}
+
 
 int
 qemuMonitorNBDServerStart(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 954ae88e4f64..1b2513650c58 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -755,6 +755,9 @@ int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon,
 int qemuMonitorGetGICCapabilities(qemuMonitorPtr mon,
                                   virGICCapability **capabilities);
 
+int qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon,
+                                  virSEVCapability **capabilities);
+
 typedef enum {
   QEMU_MONITOR_MIGRATE_BACKGROUND       = 1 << 0,
   QEMU_MONITOR_MIGRATE_NON_SHARED_DISK  = 1 << 1, /* migration with non-shared storage with full disk copy */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a09e93e464b3..94a1af1d3f75 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6362,6 +6362,79 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon,
     return ret;
 }
 
+int
+qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon,
+                                  virSEVCapability **capabilities)
+{
+    int ret = -1;
+    virJSONValuePtr cmd;
+    virJSONValuePtr reply = NULL;
+    virJSONValuePtr caps;
+    virSEVCapability *capability = NULL;
+    const char *pdh = NULL, *cert_chain = NULL;
+    int cbitpos, reduced_phys_bits;
+
+    *capabilities = NULL;
+
+    if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-capabilities",
+                                           NULL)))
+        return -1;
+
+    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+        goto cleanup;
+
+
+    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+        goto cleanup;
+
+    caps = virJSONValueObjectGetObject(reply, "return");
+
+    if (virJSONValueObjectGetNumberInt(caps, "cbitpos", &cbitpos) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("'cbitpos' field is missing"));
+        goto cleanup;
+    }
+
+    if (virJSONValueObjectGetNumberInt(caps, "reduced-phys-bits",
+                                       &reduced_phys_bits) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("'reduced-phys-bits' field is missing"));
+        goto cleanup;
+    }
+
+    if (!(pdh = virJSONValueObjectGetString(caps, "pdh"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("'pdh' field is missing"));
+        goto cleanup;
+    }
+
+    if (!(cert_chain = virJSONValueObjectGetString(caps, "cert-chain"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("'cert-chain' field is missing"));
+        goto cleanup;
+    }
+
+    if (VIR_ALLOC(capability) < 0)
+        goto cleanup;
+
+    if (VIR_STRDUP(capability->pdh, pdh) < 0)
+        goto cleanup;
+
+    if (VIR_STRDUP(capability->cert_chain, cert_chain) < 0)
+        goto cleanup;
+
+    capability->cbitpos = cbitpos;
+    capability->reduced_phys_bits = reduced_phys_bits;
+    *capabilities = capability;
+    ret = 0;
+
+ cleanup:
+    virJSONValueFree(cmd);
+    virJSONValueFree(reply);
+
+    return ret;
+}
+
 static virJSONValuePtr
 qemuMonitorJSONBuildInetSocketAddress(const char *host,
                                       const char *port)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ec243becc4ae..305f789902e9 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -152,6 +152,9 @@ int qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon,
 int qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon,
                                       virGICCapability **capabilities);
 
+int qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon,
+                                      virSEVCapability **capabilities);
+
 int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
                            unsigned int flags,
                            const char *uri);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/9] qemu: provide support to query the SEV capability
Posted by Daniel P. Berrangé, 14 weeks ago
On Thu, Mar 08, 2018 at 11:12:00AM -0600, Brijesh Singh wrote:
> QEMU version >= 2.12 provides support for launching an encrypted VMs on
> AMD x86 platform using Secure Encrypted Virtualization (SEV) feature.
> This patch adds support to query the SEV capability from the qemu.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  src/conf/domain_capabilities.h | 13 ++++++++
>  src/qemu/qemu_capabilities.c   | 43 +++++++++++++++++++++++++
>  src/qemu/qemu_capabilities.h   |  1 +
>  src/qemu/qemu_capspriv.h       |  4 +++
>  src/qemu/qemu_monitor.c        |  9 ++++++
>  src/qemu/qemu_monitor.h        |  3 ++
>  src/qemu/qemu_monitor_json.c   | 73 ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h   |  3 ++
>  8 files changed, 149 insertions(+)
> 
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index fa4c1e442f57..83d04d4c8506 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -137,6 +137,19 @@ struct _virDomainCapsCPU {
>      virDomainCapsCPUModelsPtr custom;
>  };
>  
> +/*
> + * SEV capabilities
> + */
> +typedef struct _virSEVCapability virSEVCapability;
> +typedef virSEVCapability *virSEVCapabilityPtr;
> +struct _virSEVCapability {
> +    char *pdh;
> +    char *cert_chain;
> +    int cbitpos;
> +    int reduced_phys_bits;

If you have any reason to re-spin this patch series, lets make these
two be unsigned int, since IIUC -ve values are not possible.

> +};
> +
> +
>  struct _virDomainCaps {
>      virObjectLockable parent;
>  
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index b5eb8cf46a52..68e3622a3963 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "pl011",
>                "machine.pseries.max-cpu-compat",
>                "dump-completed",
> +              "sev",
>      );
>  
>  
> @@ -525,6 +526,8 @@ struct _virQEMUCaps {
>      size_t ngicCapabilities;
>      virGICCapability *gicCapabilities;
>  
> +    virSEVCapability *sevCapabilities;
> +
>      virQEMUCapsHostCPUData kvmCPU;
>      virQEMUCapsHostCPUData tcgCPU;
>  };
> @@ -2811,6 +2814,21 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,
>      qemuCaps->ngicCapabilities = ncapabilities;
>  }
>  
> +void
> +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps,
> +                              virSEVCapability *capabilities)
> +{
> +    virSEVCapability *cap = qemuCaps->sevCapabilities;
> +
> +    if (cap) {
> +        VIR_FREE(cap->pdh);
> +        VIR_FREE(cap->cert_chain);
> +    }
> +
> +    VIR_FREE(qemuCaps->sevCapabilities);
> +
> +    qemuCaps->sevCapabilities = capabilities;
> +}
>  
>  static int
>  virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps,
> @@ -3318,6 +3336,19 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps,
>      return 0;
>  }
>  
> +static int
> +virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps,
> +                                   qemuMonitorPtr mon)
> +{
> +    virSEVCapability *caps = NULL;
> +
> +    if (qemuMonitorGetSEVCapabilities(mon, &caps) < 0)
> +        return -1;
> +
> +    virQEMUCapsSetSEVCapabilities(qemuCaps, caps);
> +
> +    return 0;
> +}
>  
>  bool
>  virQEMUCapsCPUFilterFeatures(const char *name,
> @@ -4896,6 +4927,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
>      }
>  
> +    /* no way to query -object sev-guest */
> +    if (ARCH_IS_X86(qemuCaps->arch) &&
> +        qemuCaps->version >= 2012000) {
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_SEV);
> +    }

Sigh, we really need to fix introspection of -object types one day...

> +
>      if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
>          goto cleanup;
>  
> @@ -4951,6 +4988,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>          virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE);
>  
> +    /* Probe for SEV capabilities */
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) {
> +        if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
> +            virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV);
> +    }
> +
>      ret = 0;
>   cleanup:
>      return ret;
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index c2ec2be19311..02acae491ab5 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -444,6 +444,7 @@ typedef enum {
>      QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */
>      QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */
>      QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */
> +    QEMU_CAPS_SEV, /* -object sev-guest,... */
>  
>      QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> index 222f3368e3b6..1fa85cc14f07 100644
> --- a/src/qemu/qemu_capspriv.h
> +++ b/src/qemu/qemu_capspriv.h
> @@ -86,6 +86,10 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,
>                                virGICCapability *capabilities,
>                                size_t ncapabilities);
>  
> +void
> +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps,
> +                              virSEVCapability *capabilities);
> +
>  int
>  virQEMUCapsParseHelpStr(const char *qemu,
>                          const char *str,
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index ad5c572aeefb..195248c88ae1 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4007,6 +4007,15 @@ qemuMonitorGetGICCapabilities(qemuMonitorPtr mon,
>      return qemuMonitorJSONGetGICCapabilities(mon, capabilities);
>  }
>  
> +int
> +qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon,
> +                              virSEVCapability **capabilities)
> +{
> +    QEMU_CHECK_MONITOR_JSON(mon);
> +
> +    return qemuMonitorJSONGetSEVCapabilities(mon, capabilities);
> +}
> +
>  
>  int
>  qemuMonitorNBDServerStart(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 954ae88e4f64..1b2513650c58 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -755,6 +755,9 @@ int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon,
>  int qemuMonitorGetGICCapabilities(qemuMonitorPtr mon,
>                                    virGICCapability **capabilities);
>  
> +int qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon,
> +                                  virSEVCapability **capabilities);
> +
>  typedef enum {
>    QEMU_MONITOR_MIGRATE_BACKGROUND       = 1 << 0,
>    QEMU_MONITOR_MIGRATE_NON_SHARED_DISK  = 1 << 1, /* migration with non-shared storage with full disk copy */
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index a09e93e464b3..94a1af1d3f75 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6362,6 +6362,79 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon,
>      return ret;
>  }
>  
> +int
> +qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon,
> +                                  virSEVCapability **capabilities)
> +{
> +    int ret = -1;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +    virJSONValuePtr caps;
> +    virSEVCapability *capability = NULL;
> +    const char *pdh = NULL, *cert_chain = NULL;
> +    int cbitpos, reduced_phys_bits;
> +
> +    *capabilities = NULL;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-capabilities",
> +                                           NULL)))
> +        return -1;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        goto cleanup;
> +
> +
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        goto cleanup;
> +
> +    caps = virJSONValueObjectGetObject(reply, "return");
> +
> +    if (virJSONValueObjectGetNumberInt(caps, "cbitpos", &cbitpos) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("'cbitpos' field is missing"));
> +        goto cleanup;
> +    }
> +
> +    if (virJSONValueObjectGetNumberInt(caps, "reduced-phys-bits",
> +                                       &reduced_phys_bits) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("'reduced-phys-bits' field is missing"));
> +        goto cleanup;
> +    }
> +
> +    if (!(pdh = virJSONValueObjectGetString(caps, "pdh"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("'pdh' field is missing"));
> +        goto cleanup;
> +    }
> +
> +    if (!(cert_chain = virJSONValueObjectGetString(caps, "cert-chain"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("'cert-chain' field is missing"));
> +        goto cleanup;
> +    }
> +
> +    if (VIR_ALLOC(capability) < 0)
> +        goto cleanup;
> +
> +    if (VIR_STRDUP(capability->pdh, pdh) < 0)
> +        goto cleanup;
> +
> +    if (VIR_STRDUP(capability->cert_chain, cert_chain) < 0)
> +        goto cleanup;
> +
> +    capability->cbitpos = cbitpos;
> +    capability->reduced_phys_bits = reduced_phys_bits;
> +    *capabilities = capability;
> +    ret = 0;
> +
> + cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +
> +    return ret;
> +}
> +
>  static virJSONValuePtr
>  qemuMonitorJSONBuildInetSocketAddress(const char *host,
>                                        const char *port)
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index ec243becc4ae..305f789902e9 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -152,6 +152,9 @@ int qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon,
>  int qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon,
>                                        virGICCapability **capabilities);
>  
> +int qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon,
> +                                      virSEVCapability **capabilities);
> +
>  int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
>                             unsigned int flags,
>                             const char *uri);

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/9] qemu: provide support to query the SEV capability
Posted by Peter Krempa, 14 weeks ago
On Mon, Mar 12, 2018 at 13:31:23 +0000, Daniel Berrange wrote:
> On Thu, Mar 08, 2018 at 11:12:00AM -0600, Brijesh Singh wrote:
> > QEMU version >= 2.12 provides support for launching an encrypted VMs on
> > AMD x86 platform using Secure Encrypted Virtualization (SEV) feature.
> > This patch adds support to query the SEV capability from the qemu.
> > 
> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> > ---
> >  src/conf/domain_capabilities.h | 13 ++++++++
> >  src/qemu/qemu_capabilities.c   | 43 +++++++++++++++++++++++++
> >  src/qemu/qemu_capabilities.h   |  1 +
> >  src/qemu/qemu_capspriv.h       |  4 +++
> >  src/qemu/qemu_monitor.c        |  9 ++++++
> >  src/qemu/qemu_monitor.h        |  3 ++
> >  src/qemu/qemu_monitor_json.c   | 73 ++++++++++++++++++++++++++++++++++++++++++
> >  src/qemu/qemu_monitor_json.h   |  3 ++
> >  8 files changed, 149 insertions(+)
> > 

[...]

> > @@ -4896,6 +4927,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
> >          virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
> >      }
> >  
> > +    /* no way to query -object sev-guest */
> > +    if (ARCH_IS_X86(qemuCaps->arch) &&
> > +        qemuCaps->version >= 2012000) {
> > +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_SEV);
> > +    }
> 
> Sigh, we really need to fix introspection of -object types one day...

Quick grep-ing found that we are able to probe for 'memory-backend-ram'
or 'secret' objects so the 'sev' object should be possible to probe too.

You should add test data with the qemu patches applied so that we can
verify it.

Anyways, we should not push this until it's in upstream qemu.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/9] qemu: provide support to query the SEV capability
Posted by Brijesh Singh, 14 weeks ago

On 03/12/2018 08:52 AM, Peter Krempa wrote:
> On Mon, Mar 12, 2018 at 13:31:23 +0000, Daniel Berrange wrote:
>> On Thu, Mar 08, 2018 at 11:12:00AM -0600, Brijesh Singh wrote:
>>> QEMU version >= 2.12 provides support for launching an encrypted VMs on
>>> AMD x86 platform using Secure Encrypted Virtualization (SEV) feature.
>>> This patch adds support to query the SEV capability from the qemu.
>>>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>   src/conf/domain_capabilities.h | 13 ++++++++
>>>   src/qemu/qemu_capabilities.c   | 43 +++++++++++++++++++++++++
>>>   src/qemu/qemu_capabilities.h   |  1 +
>>>   src/qemu/qemu_capspriv.h       |  4 +++
>>>   src/qemu/qemu_monitor.c        |  9 ++++++
>>>   src/qemu/qemu_monitor.h        |  3 ++
>>>   src/qemu/qemu_monitor_json.c   | 73 ++++++++++++++++++++++++++++++++++++++++++
>>>   src/qemu/qemu_monitor_json.h   |  3 ++
>>>   8 files changed, 149 insertions(+)
>>>
> 
> [...]
> 
>>> @@ -4896,6 +4927,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>>>           virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
>>>       }
>>>   
>>> +    /* no way to query -object sev-guest */
>>> +    if (ARCH_IS_X86(qemuCaps->arch) &&
>>> +        qemuCaps->version >= 2012000) {
>>> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_SEV);
>>> +    }
>>
>> Sigh, we really need to fix introspection of -object types one day...
> 
> Quick grep-ing found that we are able to probe for 'memory-backend-ram'
> or 'secret' objects so the 'sev' object should be possible to probe too.
> 


thanks for suggestions I will investigate this.


> You should add test data with the qemu patches applied so that we can
> verify it.
> 

the patch [1] adds support to test the sev specific tags.

[1] https://www.redhat.com/archives/libvir-list/2018-March/msg00452.html


> Anyways, we should not push this until it's in upstream qemu.
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/9] qemu: provide support to query the SEV capability
Posted by Brijesh Singh, 14 weeks ago

On 03/12/2018 08:31 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 11:12:00AM -0600, Brijesh Singh wrote:
>> QEMU version >= 2.12 provides support for launching an encrypted VMs on
>> AMD x86 platform using Secure Encrypted Virtualization (SEV) feature.
>> This patch adds support to query the SEV capability from the qemu.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   src/conf/domain_capabilities.h | 13 ++++++++
>>   src/qemu/qemu_capabilities.c   | 43 +++++++++++++++++++++++++
>>   src/qemu/qemu_capabilities.h   |  1 +
>>   src/qemu/qemu_capspriv.h       |  4 +++
>>   src/qemu/qemu_monitor.c        |  9 ++++++
>>   src/qemu/qemu_monitor.h        |  3 ++
>>   src/qemu/qemu_monitor_json.c   | 73 ++++++++++++++++++++++++++++++++++++++++++
>>   src/qemu/qemu_monitor_json.h   |  3 ++
>>   8 files changed, 149 insertions(+)
>>
>> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
>> index fa4c1e442f57..83d04d4c8506 100644
>> --- a/src/conf/domain_capabilities.h
>> +++ b/src/conf/domain_capabilities.h
>> @@ -137,6 +137,19 @@ struct _virDomainCapsCPU {
>>       virDomainCapsCPUModelsPtr custom;
>>   };
>>   
>> +/*
>> + * SEV capabilities
>> + */
>> +typedef struct _virSEVCapability virSEVCapability;
>> +typedef virSEVCapability *virSEVCapabilityPtr;
>> +struct _virSEVCapability {
>> +    char *pdh;
>> +    char *cert_chain;
>> +    int cbitpos;
>> +    int reduced_phys_bits;
> 
> If you have any reason to re-spin this patch series, lets make these
> two be unsigned int, since IIUC -ve values are not possible.
> 

Will make the changes in next rev.

>>   
>> +    /* no way to query -object sev-guest */
>> +    if (ARCH_IS_X86(qemuCaps->arch) &&
>> +        qemuCaps->version >= 2012000) {
>> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_SEV);
>> +    }
> 
> Sigh, we really need to fix introspection of -object types one day...
> 

I will take Peter's suggestion and look at -secret object and see how 
its probed and will do similar thing for sev-guest.

Thanks

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/9] qemu: introduce SEV feature in hypervisor capabilities
Posted by Brijesh Singh, 15 weeks ago
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
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@amd.com>
---
 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 6bfcaf61caae..f38314166ac3 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -417,6 +417,12 @@
         &lt;value&gt;3&lt;/value&gt;
       &lt;/enum&gt;
     &lt;/gic&gt;
+    &lt;sev&gt;
+      &lt;pdh&gt; &lt;/pdh&gt;
+      &lt;cert-chain&gt; &lt;/cert-chain&gt;
+      &lt;cbitpos&gt; &lt;/cbitpos&gt;
+      &lt;reduced-phys-bits&gt; &lt;/reduced-phys-bits&gt;
+    &lt;/sev&gt;
   &lt;/features&gt;
 &lt;/domainCapabilities&gt;
 </pre>
@@ -441,5 +447,39 @@
       <code>gic</code> element.</dd>
     </dl>
 
+    <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.
+
+    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>Platform diffie-hellman key, which can be exported to remote entities
+      which wish to establish a secure transport context with the SEV platform
+      in order to transmit data securely. The key is encoded in base64</dd>
+      <dt><code>cert-chain</code></dt>
+      <dd> Platform certificate chain -- which includes platform endorsement key
+      (PEK), owners certificate authory (OCA) and chip endorsement key (CEK).
+      The certificate chain is encoded in base64.</dd>
+      <dt><code>cbitpos</code></dt>
+      <dd>When memory encryption is enabled, one of the physical address bit
+      (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 loose certain bits in physical
+      address space. The number of bits we loose is hypervisor dependent.</dd>
+    </dl>
+
   </body>
 </html>
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 39053181eb9a..53b33eb83c1f 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -173,6 +173,9 @@
     <element name='features'>
       <interleave>
         <ref name='gic'/>
+        <optional>
+        <ref name='sev'/>
+        </optional>
       </interleave>
     </element>
   </define>
@@ -184,6 +187,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 f7d9be50f82d..082065fb4733 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -549,6 +549,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);
+    virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh);
+    virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n",
+                      sev->cert_chain);
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</sev>\n");
+}
+
 
 char *
 virDomainCapsFormat(virDomainCapsPtr const caps)
@@ -587,6 +606,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
     virBufferAdjustIndent(&buf, 2);
 
     virDomainCapsFeatureGICFormat(&buf, &caps->gic);
+    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 83d04d4c8506..2239566105fd 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -170,6 +170,7 @@ struct _virDomainCaps {
     /* add new domain devices here */
 
     virDomainCapsFeatureGIC gic;
+    virSEVCapabilityPtr sev;
     /* add new domain features here */
 };
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 68e3622a3963..f56e88032c06 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5975,6 +5975,8 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
         virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
         virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
         return -1;
+
+    domCaps->sev = qemuCaps->sevCapabilities;
     return 0;
 }
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/9] qemu: introduce SEV feature in hypervisor capabilities
Posted by Daniel P. Berrangé, 14 weeks ago
On Thu, Mar 08, 2018 at 11:12:01AM -0600, 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
> 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@amd.com>
> ---
>  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 6bfcaf61caae..f38314166ac3 100644
> --- a/docs/formatdomaincaps.html.in
> +++ b/docs/formatdomaincaps.html.in
> @@ -417,6 +417,12 @@
>          &lt;value&gt;3&lt;/value&gt;
>        &lt;/enum&gt;
>      &lt;/gic&gt;
> +    &lt;sev&gt;
> +      &lt;pdh&gt; &lt;/pdh&gt;
> +      &lt;cert-chain&gt; &lt;/cert-chain&gt;
> +      &lt;cbitpos&gt; &lt;/cbitpos&gt;
> +      &lt;reduced-phys-bits&gt; &lt;/reduced-phys-bits&gt;
> +    &lt;/sev&gt;
>    &lt;/features&gt;
>  &lt;/domainCapabilities&gt;
>  </pre>
> @@ -441,5 +447,39 @@
>        <code>gic</code> element.</dd>
>      </dl>
>  
> +    <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.
> +
> +    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>Platform diffie-hellman key, which can be exported to remote entities
> +      which wish to establish a secure transport context with the SEV platform
> +      in order to transmit data securely. The key is encoded in base64</dd>
> +      <dt><code>cert-chain</code></dt>
> +      <dd> Platform certificate chain -- which includes platform endorsement key
> +      (PEK), owners certificate authory (OCA) and chip endorsement key (CEK).
> +      The certificate chain is encoded in base64.</dd>
> +      <dt><code>cbitpos</code></dt>
> +      <dd>When memory encryption is enabled, one of the physical address bit
> +      (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 loose certain bits in physical
> +      address space. The number of bits we loose is hypervisor dependent.</dd>
> +    </dl>
> +
>    </body>
>  </html>
> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
> index 39053181eb9a..53b33eb83c1f 100644
> --- a/docs/schemas/domaincaps.rng
> +++ b/docs/schemas/domaincaps.rng
> @@ -173,6 +173,9 @@
>      <element name='features'>
>        <interleave>
>          <ref name='gic'/>
> +        <optional>
> +        <ref name='sev'/>
> +        </optional>
>        </interleave>
>      </element>
>    </define>
> @@ -184,6 +187,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 f7d9be50f82d..082065fb4733 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -549,6 +549,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);
> +    virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh);
> +    virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n",
> +                      sev->cert_chain);

I wonder what length 'cert_chain' is going to be typically ? I hope we
don't cause ourselves any trouble by having the data inline in the main
capabilities XML ?

> +    virBufferAdjustIndent(buf, -2);
> +    virBufferAddLit(buf, "</sev>\n");
> +}
> +
>  
>  char *
>  virDomainCapsFormat(virDomainCapsPtr const caps)
> @@ -587,6 +606,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
>      virBufferAdjustIndent(&buf, 2);
>  
>      virDomainCapsFeatureGICFormat(&buf, &caps->gic);
> +    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 83d04d4c8506..2239566105fd 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -170,6 +170,7 @@ struct _virDomainCaps {
>      /* add new domain devices here */
>  
>      virDomainCapsFeatureGIC gic;
> +    virSEVCapabilityPtr sev;
>      /* add new domain features here */
>  };
>  
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 68e3622a3963..f56e88032c06 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5975,6 +5975,8 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
>          virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
>          virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
>          return -1;
> +
> +    domCaps->sev = qemuCaps->sevCapabilities;
>      return 0;
>  }

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/9] qemu: introduce SEV feature in hypervisor capabilities
Posted by Brijesh Singh, 14 weeks ago

On 03/12/2018 08:33 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 11:12:01AM -0600, 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
>> 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@amd.com>
>> ---
>>   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 6bfcaf61caae..f38314166ac3 100644
>> --- a/docs/formatdomaincaps.html.in
>> +++ b/docs/formatdomaincaps.html.in
>> @@ -417,6 +417,12 @@
>>           &lt;value&gt;3&lt;/value&gt;
>>         &lt;/enum&gt;
>>       &lt;/gic&gt;
>> +    &lt;sev&gt;
>> +      &lt;pdh&gt; &lt;/pdh&gt;
>> +      &lt;cert-chain&gt; &lt;/cert-chain&gt;
>> +      &lt;cbitpos&gt; &lt;/cbitpos&gt;
>> +      &lt;reduced-phys-bits&gt; &lt;/reduced-phys-bits&gt;
>> +    &lt;/sev&gt;
>>     &lt;/features&gt;
>>   &lt;/domainCapabilities&gt;
>>   </pre>
>> @@ -441,5 +447,39 @@
>>         <code>gic</code> element.</dd>
>>       </dl>
>>   
>> +    <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.
>> +
>> +    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>Platform diffie-hellman key, which can be exported to remote entities
>> +      which wish to establish a secure transport context with the SEV platform
>> +      in order to transmit data securely. The key is encoded in base64</dd>
>> +      <dt><code>cert-chain</code></dt>
>> +      <dd> Platform certificate chain -- which includes platform endorsement key
>> +      (PEK), owners certificate authory (OCA) and chip endorsement key (CEK).
>> +      The certificate chain is encoded in base64.</dd>
>> +      <dt><code>cbitpos</code></dt>
>> +      <dd>When memory encryption is enabled, one of the physical address bit
>> +      (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 loose certain bits in physical
>> +      address space. The number of bits we loose is hypervisor dependent.</dd>
>> +    </dl>
>> +
>>     </body>
>>   </html>
>> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
>> index 39053181eb9a..53b33eb83c1f 100644
>> --- a/docs/schemas/domaincaps.rng
>> +++ b/docs/schemas/domaincaps.rng
>> @@ -173,6 +173,9 @@
>>       <element name='features'>
>>         <interleave>
>>           <ref name='gic'/>
>> +        <optional>
>> +        <ref name='sev'/>
>> +        </optional>
>>         </interleave>
>>       </element>
>>     </define>
>> @@ -184,6 +187,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 f7d9be50f82d..082065fb4733 100644
>> --- a/src/conf/domain_capabilities.c
>> +++ b/src/conf/domain_capabilities.c
>> @@ -549,6 +549,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);
>> +    virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh);
>> +    virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n",
>> +                      sev->cert_chain);
> 
> I wonder what length 'cert_chain' is going to be typically ? I hope we
> don't cause ourselves any trouble by having the data inline in the main
> capabilities XML ?


The PDH is 2K and cert-chain length is 8K. A quick google indicates that 
a maximum data length in XML can be up to 64000 bytes.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/9] qemu: introduce SEV feature in hypervisor capabilities
Posted by Daniel P. Berrangé, 14 weeks ago
On Mon, Mar 12, 2018 at 10:25:44AM -0500, Brijesh Singh wrote:
> 
> 
> On 03/12/2018 08:33 AM, Daniel P. Berrangé wrote:
> > On Thu, Mar 08, 2018 at 11:12:01AM -0600, 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
> > > 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@amd.com>
> > > ---
> > >   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 6bfcaf61caae..f38314166ac3 100644
> > > --- a/docs/formatdomaincaps.html.in
> > > +++ b/docs/formatdomaincaps.html.in
> > > @@ -417,6 +417,12 @@
> > >           &lt;value&gt;3&lt;/value&gt;
> > >         &lt;/enum&gt;
> > >       &lt;/gic&gt;
> > > +    &lt;sev&gt;
> > > +      &lt;pdh&gt; &lt;/pdh&gt;
> > > +      &lt;cert-chain&gt; &lt;/cert-chain&gt;
> > > +      &lt;cbitpos&gt; &lt;/cbitpos&gt;
> > > +      &lt;reduced-phys-bits&gt; &lt;/reduced-phys-bits&gt;
> > > +    &lt;/sev&gt;
> > >     &lt;/features&gt;
> > >   &lt;/domainCapabilities&gt;
> > >   </pre>
> > > @@ -441,5 +447,39 @@
> > >         <code>gic</code> element.</dd>
> > >       </dl>
> > > +    <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.
> > > +
> > > +    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>Platform diffie-hellman key, which can be exported to remote entities
> > > +      which wish to establish a secure transport context with the SEV platform
> > > +      in order to transmit data securely. The key is encoded in base64</dd>
> > > +      <dt><code>cert-chain</code></dt>
> > > +      <dd> Platform certificate chain -- which includes platform endorsement key
> > > +      (PEK), owners certificate authory (OCA) and chip endorsement key (CEK).
> > > +      The certificate chain is encoded in base64.</dd>
> > > +      <dt><code>cbitpos</code></dt>
> > > +      <dd>When memory encryption is enabled, one of the physical address bit
> > > +      (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 loose certain bits in physical
> > > +      address space. The number of bits we loose is hypervisor dependent.</dd>
> > > +    </dl>
> > > +
> > >     </body>
> > >   </html>
> > > diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
> > > index 39053181eb9a..53b33eb83c1f 100644
> > > --- a/docs/schemas/domaincaps.rng
> > > +++ b/docs/schemas/domaincaps.rng
> > > @@ -173,6 +173,9 @@
> > >       <element name='features'>
> > >         <interleave>
> > >           <ref name='gic'/>
> > > +        <optional>
> > > +        <ref name='sev'/>
> > > +        </optional>
> > >         </interleave>
> > >       </element>
> > >     </define>
> > > @@ -184,6 +187,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 f7d9be50f82d..082065fb4733 100644
> > > --- a/src/conf/domain_capabilities.c
> > > +++ b/src/conf/domain_capabilities.c
> > > @@ -549,6 +549,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);
> > > +    virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh);
> > > +    virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n",
> > > +                      sev->cert_chain);
> > 
> > I wonder what length 'cert_chain' is going to be typically ? I hope we
> > don't cause ourselves any trouble by having the data inline in the main
> > capabilities XML ?
> 
> 
> The PDH is 2K and cert-chain length is 8K. A quick google indicates that a
> maximum data length in XML can be up to 64000 bytes.

64kb is a fairly arbitrary cap we place on it in the RPC protocol, but we
have freedom to extend it.

I wonder if putting 8k of certs into the domain capabilities XML is good
idea though, even if possible.

Does anyone else have an opinion about that, or should we perhaps have a
separate API to fetch the PDH / Cert data, and use the domain capabilities
merely to indicate existance of the SEV feature (and the bit field stuff)

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/9] conf: introduce launch-security element in domain
Posted by Brijesh Singh, 15 weeks ago
The launch-security element can be used to define the security
model to use when launching a domain. Currently we support 'sev'.

When 'sev' is used, the VM will be launched with AMD SEV feature enabled.
SEV feature supports running encrypted VM under the control of KVM.
Encrypted VMs have their pages (code and data) secured such that only the
guest itself has access to the unencrypted version. Each encrypted VM is
associated with a unique encryption key; if its data is accessed to a
different entity using a different key the encrypted guests data will be
incorrectly decrypted, leading to unintelligible data.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 docs/formatdomain.html.in     | 120 ++++++++++++++++++++++++++++++++++++++++++
 docs/schemas/domaincommon.rng |  39 ++++++++++++++
 src/conf/domain_conf.c        | 111 ++++++++++++++++++++++++++++++++++++++
 src/conf/domain_conf.h        |  27 ++++++++++
 4 files changed, 297 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6fd2189cd2f4..830d2a3c59be 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -8195,6 +8195,126 @@ qemu-kvm -net nic,model=? /dev/null
 
     <p>Note: DEA/TDEA is synonymous with DES/TDES.</p>
 
+    <h3><a id="sev">Secure Encrypted Virtualization (SEV)</a></h3>
+
+    <p>
+       The contents of the <code>&lt;launch-security type='sev'&gt;</code> element
+       is used to provide the guest owners input used for creating an encrypted
+       VM using the AMD SEV feature.
+
+       SEV is an extension to the AMD-V architecture which supports running
+       encrypted virtual machine (VMs) under the control of KVM. Encrypted
+       VMs have their pages (code and data) secured such that only the guest
+       itself has access to the unencrypted version. Each encrypted VM is
+       associated with a unique encryption key; if its data is accessed to a
+       different entity using a different key the encrypted guests data will
+       be incorrectly decrypted, leading to unintelligible data.
+
+       For more information see various input parameters and its format see SEV API spec
+       <a href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf"> https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf </a>
+       <span class="since">Since 4.2.0</span>
+       </p>
+    <pre>
+&lt;domain&gt;
+  ...
+  &lt;launch-security type='sev'&gt;
+    &lt;policy&gt; 0 &lt;/policy&gt;
+    &lt;cbitpos&gt; 47 &lt;/cbitpos&gt;
+    &lt;reduced-phys-bits&gt; 5 &lt;/reduced-phys-bits&gt;
+    &lt;session&gt; ... &lt;/session&gt;
+    &lt;dh-cert&gt; ... &lt;/dh&gt;
+  &lt;/sev&gt;
+  ...
+&lt;/domain&gt;
+</pre>
+
+    <p>
+    A least <code>cbitpos</code> and <code>reduced-phys-bits</code> must be
+    nested within the <code>launch-security</code> element.
+    </p>
+    <dl>
+      <dt><code>cbitpos</code></dt>
+      <dd>The <code>cbitpos</code> element provides the C-bit (aka encryption bit)
+      location in guest page table entry. The value of <code>cbitpos</code> is
+      hypervisor dependent and can be obtained through the <code>sev</code> element
+      from domaincapabilities.
+      </dd>
+      <dt><code>reduced-phys-bits</code></dt>
+      <dd>The <code>reduced-phys-bits</code> element provides the physical
+      address bit reducation. Similar to <code>cbitpos</code> the value of <code>
+      reduced-phys-bit</code> is hypervisor dependent and can be obtained
+      through the <code>sev</code> element from domaincapabilities.
+      </dd>
+      <dt><code>policy</code></dt>
+      <dd>The optional <code>policy</code> element provides the guest policy
+      which must be maintained by the SEV firmware. This policy is enforced by
+      the firmware and restricts what configuration and operational commands
+      can be performed on this guest by the hypervisor. The guest policy
+      provided during guest launch is bound to the guest and cannot be changed
+      throughout the lifetime of the guest. The policy is also transmitted
+      during snapshot and migration flows and enforced on the destination platform.
+
+      The guest policy is a 4-byte structure with the fields shown in Table:
+
+      <table class="top_table">
+        <tr>
+          <th> Bit(s) </th>
+          <th> Description </th>
+        </tr>
+        <tr>
+          <td> 0 </td>
+          <td> Debugging of the guest is disallowed when set </td>
+        </tr>
+        <tr>
+          <td> 1 </td>
+          <td> Sharing keys with other guests is disallowed when set </td>
+        </tr>
+        <tr>
+          <td> 2 </td>
+          <td> SEV-ES is required when set</td>
+        </tr>
+        <tr>
+          <td> 3 </td>
+          <td> Sending the guest to another platform is disallowed when set</td>
+        </tr>
+        <tr>
+          <td> 4 </td>
+          <td> The guest must not be transmitted to another platform that is
+               not in the domain when set. </td>
+        </tr>
+        <tr>
+          <td> 5 </td>
+          <td> The guest must not be transmitted to another platform that is
+               not SEV capable when set. </td>
+        </tr>
+        <tr>
+          <td> 15:6 </td>
+          <td> reserved </td>
+        </tr>
+        <tr>
+          <td> 16:32 </td>
+          <td> The guest must not be transmitted to another platform with a
+               lower firmware version. </td>
+        </tr>
+      </table>
+      Default value is 0x1
+
+      </dd>
+      <dt><code>dh-cert</code></dt>
+      <dd>The optional <code>dh-cert</code> element provides the guest owners public
+      Diffie-Hellman (DH) key. The key is used to negotiate a master secret
+      key between the SEV firmware and guest owner. This master secret key is
+      then used to establish a trusted channel between SEV firmware and guest
+      owner. The value must be encoded in base64.
+      </dd>
+      <dt><code>session</code></dt>
+      <dd>The optional <code>session</code> element provides the guest owners
+      session blob defined in SEV API spec. The value must be encoded in base64.
+
+      See SEV spec LAUNCH_START section for session blob format.
+      </dd>
+    </dl>
+
     <h2><a id="examples">Example configs</a></h2>
 
     <p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8165e699d67e..e3dcc69067c2 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -77,6 +77,9 @@
         <optional>
           <ref name='keywrap'/>
         </optional>
+        <optional>
+          <ref name='launch-security'/>
+        </optional>
       </interleave>
     </element>
   </define>
@@ -436,6 +439,42 @@
     </element>
   </define>
 
+  <define name="launch-security">
+    <element name="launch-security">
+      <attribute name="type">
+        <value>sev</value>
+      </attribute>
+      <interleave>
+        <element name="cbitpos">
+          <data type='unsignedInt'/>
+        </element>
+        <element name="reduced-phys-bits">
+          <data type='unsignedInt'/>
+        </element>
+        <optional>
+          <element name="policy">
+            <ref name='hexuint'/>
+          </element>
+        </optional>
+        <optional>
+          <element name="handle">
+            <ref name='unsignedInt'/>
+          </element>
+        </optional>
+        <optional>
+          <element name="dh-cert">
+            <data type="string"/>
+          </element>
+        </optional>
+        <optional>
+          <element name="session">
+            <data type="string"/>
+          </element>
+        </optional>
+      </interleave>
+    </element>
+  </define>
+
   <!--
       Enable or disable perf events for the domain. For each
       of the events the following rules apply:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fcafc8b2fafe..52d34de69d45 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -929,6 +929,10 @@ VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
               "ivshmem-plain",
               "ivshmem-doorbell")
 
+VIR_ENUM_IMPL(virDomainLaunchSecurity, VIR_DOMAIN_LAUNCH_SECURITY_LAST,
+              "",
+              "sev")
+
 static virClassPtr virDomainObjClass;
 static virClassPtr virDomainXMLOptionClass;
 static void virDomainObjDispose(void *obj);
@@ -2897,6 +2901,14 @@ virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune)
     VIR_FREE(cachetune);
 }
 
+static void
+virDomainSevDefFree(virDomainSevDefPtr def)
+{
+    VIR_FREE(def->dh_cert);
+    VIR_FREE(def->session);
+
+    VIR_FREE(def);
+}
 
 void virDomainDefFree(virDomainDefPtr def)
 {
@@ -3079,6 +3091,9 @@ void virDomainDefFree(virDomainDefPtr def)
     if (def->namespaceData && def->ns.free)
         (def->ns.free)(def->namespaceData);
 
+    if (def->sev)
+        virDomainSevDefFree(def->sev);
+
     xmlFreeNode(def->metadata);
 
     VIR_FREE(def);
@@ -15539,6 +15554,71 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
     return ret;
 }
 
+static virDomainSevDefPtr
+virDomainSevDefParseXML(xmlNodePtr sevNode,
+                        xmlXPathContextPtr ctxt)
+{
+    char *tmp = NULL, *type = NULL;
+    xmlNodePtr save = ctxt->node;
+    virDomainSevDefPtr def;
+    unsigned long policy;
+
+    ctxt->node = sevNode;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    if (!(type = virXMLPropString(sevNode, "type"))) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing launch-security type"));
+        goto error;
+    }
+
+    if (virDomainLaunchSecurityTypeFromString(type) !=
+        VIR_DOMAIN_LAUNCH_SECURITY_SEV) {
+        goto error;
+    }
+
+    if (virXPathInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0) {
+        virReportError(VIR_ERR_XML_ERROR, "%s", _("failed to get cbitpos"));
+        goto error;
+    }
+
+    if (virXPathInt("string(./reduced-phys-bits)", ctxt,
+                    &def->reduced_phys_bits) < 0) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("failed to get reduced-phys-bits"));
+        goto error;
+    }
+
+    if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0)
+        policy = 0x1;
+
+    def->policy = policy;
+
+    if ((tmp = virXPathString("string(./dh-cert)", ctxt))) {
+        if (VIR_STRDUP(def->dh_cert, tmp) < 0)
+            goto error;
+
+        VIR_FREE(tmp);
+    }
+
+    if ((tmp = virXPathString("string(./session)", ctxt))) {
+        if (VIR_STRDUP(def->session, tmp) < 0)
+            goto error;
+
+        VIR_FREE(tmp);
+    }
+
+    ctxt->node = save;
+    return def;
+
+ error:
+    VIR_FREE(tmp);
+    virDomainSevDefFree(def);
+    ctxt->node = save;
+    return NULL;
+}
 
 static virDomainMemoryDefPtr
 virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
@@ -20212,6 +20292,13 @@ virDomainDefParseXML(xmlDocPtr xml,
     ctxt->node = node;
     VIR_FREE(nodes);
 
+    /* Check for SEV feature */
+    if ((node = virXPathNode("./launch-security", ctxt)) != NULL) {
+        def->sev = virDomainSevDefParseXML(node, ctxt);
+        if (!def->sev)
+            goto error;
+    }
+
     /* analysis of memory devices */
     if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0)
         goto error;
@@ -26102,6 +26189,27 @@ virDomainKeyWrapDefFormat(virBufferPtr buf, virDomainKeyWrapDefPtr keywrap)
     virBufferAddLit(buf, "</keywrap>\n");
 }
 
+static void
+virDomainSevDefFormat(virBufferPtr buf, virDomainSevDefPtr sev)
+{
+    virBufferAddLit(buf, "<launch-security type='sev'>\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);
+    virBufferAsprintf(buf, "<policy>%d</policy>\n", sev->policy);
+    if (sev->dh_cert)
+        virBufferAsprintf(buf, "<dh_cert>%s</dh_cert>\n", sev->dh_cert);
+
+    if (sev->session)
+        virBufferAsprintf(buf, "<session>%s</session>\n", sev->session);
+
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</launch-security>\n");
+}
+
+
 static void
 virDomainPerfDefFormat(virBufferPtr buf, virDomainPerfDefPtr perf)
 {
@@ -27274,6 +27382,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     if (def->keywrap)
         virDomainKeyWrapDefFormat(buf, def->keywrap);
 
+    if (def->sev)
+        virDomainSevDefFormat(buf, def->sev);
+
     virBufferAdjustIndent(buf, -2);
     virBufferAddLit(buf, "</domain>\n");
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 368f16f3fbf9..ff625a0aff1b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -142,6 +142,9 @@ typedef virDomainPanicDef *virDomainPanicDefPtr;
 typedef struct _virDomainMemoryDef virDomainMemoryDef;
 typedef virDomainMemoryDef *virDomainMemoryDefPtr;
 
+typedef struct _virDomainSevDef virDomainSevDef;
+typedef virDomainSevDef *virDomainSevDefPtr;
+
 /* forward declarations virDomainChrSourceDef, required by
  * virDomainNetDef
  */
@@ -2289,6 +2292,26 @@ struct _virDomainKeyWrapDef {
     int dea; /* enum virTristateSwitch */
 };
 
+typedef enum {
+    VIR_DOMAIN_LAUNCH_SECURITY_NONE,
+    VIR_DOMAIN_LAUNCH_SECURITY_SEV,
+
+    VIR_DOMAIN_LAUNCH_SECURITY_LAST,
+} virDomainLaunchSecurity;
+
+typedef struct _virDomainSevDef virDomainSevDef;
+typedef virDomainSevDef *virDomainSevDefPtr;
+
+struct _virDomainSevDef {
+    char *dh_cert;
+    char *session;
+    char *configDir;
+    int policy;
+    int cbitpos;
+    int reduced_phys_bits;
+};
+
+
 typedef enum {
     VIR_DOMAIN_IOMMU_MODEL_INTEL,
 
@@ -2454,6 +2477,9 @@ struct _virDomainDef {
 
     virDomainKeyWrapDefPtr keywrap;
 
+    /* SEV-specific domain */
+    virDomainSevDefPtr sev;
+
     /* Application-specific custom metadata */
     xmlNodePtr metadata;
 
@@ -3344,6 +3370,7 @@ VIR_ENUM_DECL(virDomainMemorySource)
 VIR_ENUM_DECL(virDomainMemoryAllocation)
 VIR_ENUM_DECL(virDomainIOMMUModel)
 VIR_ENUM_DECL(virDomainShmemModel)
+VIR_ENUM_DECL(virDomainLaunchSecurity)
 /* from libvirt.h */
 VIR_ENUM_DECL(virDomainState)
 VIR_ENUM_DECL(virDomainNostateReason)
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/9] conf: introduce launch-security element in domain
Posted by Daniel P. Berrangé, 14 weeks ago
On Thu, Mar 08, 2018 at 11:12:02AM -0600, Brijesh Singh wrote:
> The launch-security element can be used to define the security
> model to use when launching a domain. Currently we support 'sev'.
> 
> When 'sev' is used, the VM will be launched with AMD SEV feature enabled.
> SEV feature supports running encrypted VM under the control of KVM.
> Encrypted VMs have their pages (code and data) secured such that only the
> guest itself has access to the unencrypted version. Each encrypted VM is
> associated with a unique encryption key; if its data is accessed to a
> different entity using a different key the encrypted guests data will be
> incorrectly decrypted, leading to unintelligible data.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  docs/formatdomain.html.in     | 120 ++++++++++++++++++++++++++++++++++++++++++
>  docs/schemas/domaincommon.rng |  39 ++++++++++++++
>  src/conf/domain_conf.c        | 111 ++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h        |  27 ++++++++++
>  4 files changed, 297 insertions(+)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6fd2189cd2f4..830d2a3c59be 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -8195,6 +8195,126 @@ qemu-kvm -net nic,model=? /dev/null
>  
>      <p>Note: DEA/TDEA is synonymous with DES/TDES.</p>
>  
> +    <h3><a id="sev">Secure Encrypted Virtualization (SEV)</a></h3>
> +
> +    <p>
> +       The contents of the <code>&lt;launch-security type='sev'&gt;</code> element
> +       is used to provide the guest owners input used for creating an encrypted
> +       VM using the AMD SEV feature.
> +
> +       SEV is an extension to the AMD-V architecture which supports running
> +       encrypted virtual machine (VMs) under the control of KVM. Encrypted
> +       VMs have their pages (code and data) secured such that only the guest
> +       itself has access to the unencrypted version. Each encrypted VM is
> +       associated with a unique encryption key; if its data is accessed to a
> +       different entity using a different key the encrypted guests data will
> +       be incorrectly decrypted, leading to unintelligible data.
> +
> +       For more information see various input parameters and its format see SEV API spec
> +       <a href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf"> https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf </a>
> +       <span class="since">Since 4.2.0</span>
> +       </p>
> +    <pre>
> +&lt;domain&gt;
> +  ...
> +  &lt;launch-security type='sev'&gt;
> +    &lt;policy&gt; 0 &lt;/policy&gt;
> +    &lt;cbitpos&gt; 47 &lt;/cbitpos&gt;
> +    &lt;reduced-phys-bits&gt; 5 &lt;/reduced-phys-bits&gt;
> +    &lt;session&gt; ... &lt;/session&gt;
> +    &lt;dh-cert&gt; ... &lt;/dh&gt;
> +  &lt;/sev&gt;
> +  ...
> +&lt;/domain&gt;
> +</pre>
> +
> +    <p>
> +    A least <code>cbitpos</code> and <code>reduced-phys-bits</code> must be
> +    nested within the <code>launch-security</code> element.
> +    </p>
> +    <dl>
> +      <dt><code>cbitpos</code></dt>
> +      <dd>The <code>cbitpos</code> element provides the C-bit (aka encryption bit)
> +      location in guest page table entry. The value of <code>cbitpos</code> is
> +      hypervisor dependent and can be obtained through the <code>sev</code> element
> +      from domaincapabilities.
> +      </dd>
> +      <dt><code>reduced-phys-bits</code></dt>
> +      <dd>The <code>reduced-phys-bits</code> element provides the physical
> +      address bit reducation. Similar to <code>cbitpos</code> the value of <code>
> +      reduced-phys-bit</code> is hypervisor dependent and can be obtained
> +      through the <code>sev</code> element from domaincapabilities.
> +      </dd>
> +      <dt><code>policy</code></dt>
> +      <dd>The optional <code>policy</code> element provides the guest policy
> +      which must be maintained by the SEV firmware. This policy is enforced by
> +      the firmware and restricts what configuration and operational commands
> +      can be performed on this guest by the hypervisor. The guest policy
> +      provided during guest launch is bound to the guest and cannot be changed
> +      throughout the lifetime of the guest. The policy is also transmitted
> +      during snapshot and migration flows and enforced on the destination platform.
> +
> +      The guest policy is a 4-byte structure with the fields shown in Table:
> +
> +      <table class="top_table">
> +        <tr>
> +          <th> Bit(s) </th>
> +          <th> Description </th>
> +        </tr>
> +        <tr>
> +          <td> 0 </td>
> +          <td> Debugging of the guest is disallowed when set </td>
> +        </tr>
> +        <tr>
> +          <td> 1 </td>
> +          <td> Sharing keys with other guests is disallowed when set </td>
> +        </tr>
> +        <tr>
> +          <td> 2 </td>
> +          <td> SEV-ES is required when set</td>
> +        </tr>
> +        <tr>
> +          <td> 3 </td>
> +          <td> Sending the guest to another platform is disallowed when set</td>
> +        </tr>
> +        <tr>
> +          <td> 4 </td>
> +          <td> The guest must not be transmitted to another platform that is
> +               not in the domain when set. </td>
> +        </tr>
> +        <tr>
> +          <td> 5 </td>
> +          <td> The guest must not be transmitted to another platform that is
> +               not SEV capable when set. </td>
> +        </tr>
> +        <tr>
> +          <td> 15:6 </td>
> +          <td> reserved </td>
> +        </tr>
> +        <tr>
> +          <td> 16:32 </td>
> +          <td> The guest must not be transmitted to another platform with a
> +               lower firmware version. </td>
> +        </tr>
> +      </table>
> +      Default value is 0x1
> +
> +      </dd>
> +      <dt><code>dh-cert</code></dt>
> +      <dd>The optional <code>dh-cert</code> element provides the guest owners public
> +      Diffie-Hellman (DH) key. The key is used to negotiate a master secret
> +      key between the SEV firmware and guest owner. This master secret key is
> +      then used to establish a trusted channel between SEV firmware and guest
> +      owner. The value must be encoded in base64.
> +      </dd>
> +      <dt><code>session</code></dt>
> +      <dd>The optional <code>session</code> element provides the guest owners
> +      session blob defined in SEV API spec. The value must be encoded in base64.
> +
> +      See SEV spec LAUNCH_START section for session blob format.
> +      </dd>
> +    </dl>
> +
>      <h2><a id="examples">Example configs</a></h2>
>  
>      <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8165e699d67e..e3dcc69067c2 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -77,6 +77,9 @@
>          <optional>
>            <ref name='keywrap'/>
>          </optional>
> +        <optional>
> +          <ref name='launch-security'/>
> +        </optional>
>        </interleave>
>      </element>
>    </define>
> @@ -436,6 +439,42 @@
>      </element>
>    </define>
>  
> +  <define name="launch-security">
> +    <element name="launch-security">
> +      <attribute name="type">
> +        <value>sev</value>
> +      </attribute>
> +      <interleave>
> +        <element name="cbitpos">
> +          <data type='unsignedInt'/>
> +        </element>
> +        <element name="reduced-phys-bits">
> +          <data type='unsignedInt'/>
> +        </element>
> +        <optional>
> +          <element name="policy">
> +            <ref name='hexuint'/>
> +          </element>
> +        </optional>
> +        <optional>
> +          <element name="handle">
> +            <ref name='unsignedInt'/>
> +          </element>
> +        </optional>
> +        <optional>
> +          <element name="dh-cert">
> +            <data type="string"/>
> +          </element>
> +        </optional>
> +        <optional>
> +          <element name="session">
> +            <data type="string"/>
> +          </element>
> +        </optional>
> +      </interleave>
> +    </element>
> +  </define>
> +
>    <!--
>        Enable or disable perf events for the domain. For each
>        of the events the following rules apply:
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index fcafc8b2fafe..52d34de69d45 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -929,6 +929,10 @@ VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
>                "ivshmem-plain",
>                "ivshmem-doorbell")
>  
> +VIR_ENUM_IMPL(virDomainLaunchSecurity, VIR_DOMAIN_LAUNCH_SECURITY_LAST,
> +              "",
> +              "sev")
> +
>  static virClassPtr virDomainObjClass;
>  static virClassPtr virDomainXMLOptionClass;
>  static void virDomainObjDispose(void *obj);
> @@ -2897,6 +2901,14 @@ virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune)
>      VIR_FREE(cachetune);
>  }
>  
> +static void
> +virDomainSevDefFree(virDomainSevDefPtr def)
> +{
> +    VIR_FREE(def->dh_cert);
> +    VIR_FREE(def->session);
> +
> +    VIR_FREE(def);
> +}
>  
>  void virDomainDefFree(virDomainDefPtr def)
>  {
> @@ -3079,6 +3091,9 @@ void virDomainDefFree(virDomainDefPtr def)
>      if (def->namespaceData && def->ns.free)
>          (def->ns.free)(def->namespaceData);
>  
> +    if (def->sev)
> +        virDomainSevDefFree(def->sev);
> +
>      xmlFreeNode(def->metadata);
>  
>      VIR_FREE(def);
> @@ -15539,6 +15554,71 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
>      return ret;
>  }
>  
> +static virDomainSevDefPtr
> +virDomainSevDefParseXML(xmlNodePtr sevNode,
> +                        xmlXPathContextPtr ctxt)
> +{
> +    char *tmp = NULL, *type = NULL;
> +    xmlNodePtr save = ctxt->node;
> +    virDomainSevDefPtr def;
> +    unsigned long policy;
> +
> +    ctxt->node = sevNode;
> +
> +    if (VIR_ALLOC(def) < 0)
> +        return NULL;
> +
> +    if (!(type = virXMLPropString(sevNode, "type"))) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("missing launch-security type"));
> +        goto error;
> +    }
> +
> +    if (virDomainLaunchSecurityTypeFromString(type) !=
> +        VIR_DOMAIN_LAUNCH_SECURITY_SEV) {
> +        goto error;
> +    }
> +
> +    if (virXPathInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s", _("failed to get cbitpos"));
> +        goto error;
> +    }
> +
> +    if (virXPathInt("string(./reduced-phys-bits)", ctxt,
> +                    &def->reduced_phys_bits) < 0) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("failed to get reduced-phys-bits"));
> +        goto error;
> +    }
> +
> +    if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0)
> +        policy = 0x1;
> +
> +    def->policy = policy;
> +
> +    if ((tmp = virXPathString("string(./dh-cert)", ctxt))) {
> +        if (VIR_STRDUP(def->dh_cert, tmp) < 0)
> +            goto error;
> +
> +        VIR_FREE(tmp);
> +    }
> +
> +    if ((tmp = virXPathString("string(./session)", ctxt))) {
> +        if (VIR_STRDUP(def->session, tmp) < 0)
> +            goto error;
> +
> +        VIR_FREE(tmp);
> +    }
> +
> +    ctxt->node = save;
> +    return def;
> +
> + error:
> +    VIR_FREE(tmp);
> +    virDomainSevDefFree(def);
> +    ctxt->node = save;
> +    return NULL;
> +}
>  
>  static virDomainMemoryDefPtr
>  virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
> @@ -20212,6 +20292,13 @@ virDomainDefParseXML(xmlDocPtr xml,
>      ctxt->node = node;
>      VIR_FREE(nodes);
>  
> +    /* Check for SEV feature */
> +    if ((node = virXPathNode("./launch-security", ctxt)) != NULL) {
> +        def->sev = virDomainSevDefParseXML(node, ctxt);
> +        if (!def->sev)
> +            goto error;
> +    }
> +
>      /* analysis of memory devices */
>      if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0)
>          goto error;
> @@ -26102,6 +26189,27 @@ virDomainKeyWrapDefFormat(virBufferPtr buf, virDomainKeyWrapDefPtr keywrap)
>      virBufferAddLit(buf, "</keywrap>\n");
>  }
>  
> +static void
> +virDomainSevDefFormat(virBufferPtr buf, virDomainSevDefPtr sev)
> +{
> +    virBufferAddLit(buf, "<launch-security type='sev'>\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);
> +    virBufferAsprintf(buf, "<policy>%d</policy>\n", sev->policy);
> +    if (sev->dh_cert)
> +        virBufferAsprintf(buf, "<dh_cert>%s</dh_cert>\n", sev->dh_cert);
> +
> +    if (sev->session)
> +        virBufferAsprintf(buf, "<session>%s</session>\n", sev->session);
> +
> +    virBufferAdjustIndent(buf, -2);
> +    virBufferAddLit(buf, "</launch-security>\n");
> +}
> +
> +
>  static void
>  virDomainPerfDefFormat(virBufferPtr buf, virDomainPerfDefPtr perf)
>  {
> @@ -27274,6 +27382,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>      if (def->keywrap)
>          virDomainKeyWrapDefFormat(buf, def->keywrap);
>  
> +    if (def->sev)
> +        virDomainSevDefFormat(buf, def->sev);
> +
>      virBufferAdjustIndent(buf, -2);
>      virBufferAddLit(buf, "</domain>\n");
>  
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 368f16f3fbf9..ff625a0aff1b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -142,6 +142,9 @@ typedef virDomainPanicDef *virDomainPanicDefPtr;
>  typedef struct _virDomainMemoryDef virDomainMemoryDef;
>  typedef virDomainMemoryDef *virDomainMemoryDefPtr;
>  
> +typedef struct _virDomainSevDef virDomainSevDef;
> +typedef virDomainSevDef *virDomainSevDefPtr;
> +
>  /* forward declarations virDomainChrSourceDef, required by
>   * virDomainNetDef
>   */
> @@ -2289,6 +2292,26 @@ struct _virDomainKeyWrapDef {
>      int dea; /* enum virTristateSwitch */
>  };
>  
> +typedef enum {
> +    VIR_DOMAIN_LAUNCH_SECURITY_NONE,
> +    VIR_DOMAIN_LAUNCH_SECURITY_SEV,
> +
> +    VIR_DOMAIN_LAUNCH_SECURITY_LAST,
> +} virDomainLaunchSecurity;
> +
> +typedef struct _virDomainSevDef virDomainSevDef;
> +typedef virDomainSevDef *virDomainSevDefPtr;
> +
> +struct _virDomainSevDef {
> +    char *dh_cert;
> +    char *session;
> +    char *configDir;
> +    int policy;
> +    int cbitpos;
> +    int reduced_phys_bits;
> +};

I'd suggested 'unsigned int' for these 3 variables, assuming they
don't ever need -ve values.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/9] qemu: add support to launch SEV guest
Posted by Brijesh Singh, 15 weeks ago
QEMU >= 2.12 provides 'sev-guest' object which is used to launch encrypted
VMs on AMD platform using SEV feature. The various inputs required to
launch SEV guest is provided through the <launch-security> tag. A typical
SEV guest launch command line looks like this:

# $QEMU ...\
  -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 ...\
  -machine memory-encryption=sev0 \

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 src/qemu/qemu_command.c | 33 ++++++++++++++++++
 src/qemu/qemu_process.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fa0aa5d5c3d4..39f136a389cb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9663,6 +9663,36 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
     return 0;
 }
 
+static void
+qemuBuildSevCommandLine(virCommandPtr cmd,
+                        virDomainSevDefPtr sev)
+{
+    virBuffer obj = VIR_BUFFER_INITIALIZER;
+    char *path = NULL;
+
+    VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
+              sev->policy, sev->cbitpos, sev->reduced_phys_bits);
+
+    virCommandAddArgList(cmd, "-machine", "memory-encryption=sev0", NULL);
+
+    virBufferAsprintf(&obj, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos);
+    virBufferAsprintf(&obj, ",reduced-phys-bits=%d", sev->reduced_phys_bits);
+    virBufferAsprintf(&obj, ",policy=0x%x", sev->policy);
+
+    if (sev->dh_cert) {
+        ignore_value(virAsprintf(&path, "%s/dh_cert.base64", sev->configDir));
+        virBufferAsprintf(&obj, ",dh-cert-file=%s", path);
+        VIR_FREE(path);
+    }
+
+    if (sev->session) {
+        ignore_value(virAsprintf(&path, "%s/session.base64", sev->configDir));
+        virBufferAsprintf(&obj, ",session-file=%s", path);
+        VIR_FREE(path);
+    }
+
+    virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL);
+}
 
 static int
 qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
@@ -10108,6 +10138,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
     if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
         goto error;
 
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev)
+        qemuBuildSevCommandLine(cmd, def->sev);
+
     if (snapshot)
         virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 57c06c7c1550..349e12b6dc12 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3457,6 +3457,16 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
 }
 
 
+static void
+qemuProcessDestroySevPaths(virDomainSevDefPtr sev)
+{
+    if (!sev)
+        return;
+
+    virFileDeleteTree(sev->configDir);
+    VIR_FREE(sev->configDir);
+}
+
 int
 qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
                                    virDomainObjPtr vm,
@@ -5741,6 +5751,83 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
     return ret;
 }
 
+static int
+qemuBuildSevCreateFile(const char *configDir, const char *name,
+                       const char *data)
+{
+    char *configFile;
+
+    if (!(configFile = virFileBuildPath(configDir, name, ".base64")))
+        return -1;
+
+    if (virFileRewriteStr(configFile, S_IRUSR | S_IWUSR, data) < 0) {
+        virReportSystemError(errno, _("failed to write data to config '%s'"),
+                             configFile);
+        goto error;
+    }
+
+    VIR_FREE(configFile);
+    return 0;
+
+ error:
+    VIR_FREE(configFile);
+    return -1;
+}
+
+static int
+qemuProcessPrepareSevGuestInput(virQEMUDriverPtr driver,
+                                virDomainObjPtr vm)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    virDomainDefPtr def = vm->def;
+    virQEMUCapsPtr qemuCaps = priv->qemuCaps;
+    virDomainSevDefPtr sev = def->sev;
+    char *configDir = NULL;
+    char *domPath = virDomainDefGetShortName(def);
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+    if (!sev)
+        return 0;
+
+    VIR_DEBUG("Prepare SEV guest");
+
+    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                        _("Domain %s asked for 'sev' launch but "
+                          "QEMU does not support SEV feature"), vm->def->name);
+        return -1;
+    }
+
+    if (virAsprintf(&configDir, "%s/sev/%s", cfg->configDir, domPath) < 0)
+        goto error;
+
+    if (virFileMakePathWithMode(configDir, S_IRWXU) < 0) {
+        virReportSystemError(errno, _("cannot create config directory '%s'"),
+                             configDir);
+        goto error;
+    }
+
+    if (sev->dh_cert) {
+        if (qemuBuildSevCreateFile(configDir, "dh_cert", sev->dh_cert) < 0)
+            goto error1;
+    }
+
+    if (sev->session) {
+        if (qemuBuildSevCreateFile(configDir, "session", sev->session) < 0)
+            goto error1;
+    }
+
+    VIR_FREE(domPath);
+    sev->configDir = configDir;
+    return 0;
+
+ error1:
+    virFileDeleteTree(configDir);
+ error:
+    VIR_FREE(configDir);
+    VIR_FREE(domPath);
+    return -1;
+}
 
 static int
 qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
@@ -5866,6 +5953,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
     if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
         goto cleanup;
 
+    if (qemuProcessPrepareSevGuestInput(driver, vm) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     virObjectUnref(cfg);
@@ -6535,6 +6625,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     }
 
     qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
+    qemuProcessDestroySevPaths(vm->def->sev);
 
     vm->def->id = -1;
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/9] qemu: add support to launch SEV guest
Posted by Daniel P. Berrangé, 14 weeks ago
On Thu, Mar 08, 2018 at 11:12:03AM -0600, Brijesh Singh wrote:
> QEMU >= 2.12 provides 'sev-guest' object which is used to launch encrypted
> VMs on AMD platform using SEV feature. The various inputs required to
> launch SEV guest is provided through the <launch-security> tag. A typical
> SEV guest launch command line looks like this:
> 
> # $QEMU ...\
>   -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 ...\
>   -machine memory-encryption=sev0 \
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  src/qemu/qemu_command.c | 33 ++++++++++++++++++
>  src/qemu/qemu_process.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 124 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index fa0aa5d5c3d4..39f136a389cb 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9663,6 +9663,36 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
>      return 0;
>  }
>  
> +static void
> +qemuBuildSevCommandLine(virCommandPtr cmd,
> +                        virDomainSevDefPtr sev)
> +{
> +    virBuffer obj = VIR_BUFFER_INITIALIZER;
> +    char *path = NULL;
> +
> +    VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
> +              sev->policy, sev->cbitpos, sev->reduced_phys_bits);
> +
> +    virCommandAddArgList(cmd, "-machine", "memory-encryption=sev0", NULL);


Hmm, so this will result in us passing  '-machine' many times, and relying
on QEMU to acculate the args. It should work, but IME it could be a little
surprising to people debuggnig qemu to see repeated args for -machine.

Could we just call this method from the place that  constructs the
current -machine arg so we append to that.

> +
> +    virBufferAsprintf(&obj, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos);
> +    virBufferAsprintf(&obj, ",reduced-phys-bits=%d", sev->reduced_phys_bits);
> +    virBufferAsprintf(&obj, ",policy=0x%x", sev->policy);
> +
> +    if (sev->dh_cert) {
> +        ignore_value(virAsprintf(&path, "%s/dh_cert.base64", sev->configDir));
> +        virBufferAsprintf(&obj, ",dh-cert-file=%s", path);
> +        VIR_FREE(path);
> +    }
> +
> +    if (sev->session) {
> +        ignore_value(virAsprintf(&path, "%s/session.base64", sev->configDir));
> +        virBufferAsprintf(&obj, ",session-file=%s", path);
> +        VIR_FREE(path);
> +    }
> +
> +    virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL);
> +}
>  
>  static int
>  qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
> @@ -10108,6 +10138,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>      if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
>          goto error;
>  
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev)
> +        qemuBuildSevCommandLine(cmd, def->sev);
> +
>      if (snapshot)
>          virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 57c06c7c1550..349e12b6dc12 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3457,6 +3457,16 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
>  }
>  
>  
> +static void
> +qemuProcessDestroySevPaths(virDomainSevDefPtr sev)
> +{
> +    if (!sev)
> +        return;
> +
> +    virFileDeleteTree(sev->configDir);

Just delete the individual files we created, no recursive delete please.

> +    VIR_FREE(sev->configDir);
> +}
> +
>  int
>  qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
>                                     virDomainObjPtr vm,
> @@ -5741,6 +5751,83 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
>      return ret;
>  }
>  
> +static int
> +qemuBuildSevCreateFile(const char *configDir, const char *name,
> +                       const char *data)
> +{
> +    char *configFile;
> +
> +    if (!(configFile = virFileBuildPath(configDir, name, ".base64")))
> +        return -1;
> +
> +    if (virFileRewriteStr(configFile, S_IRUSR | S_IWUSR, data) < 0) {
> +        virReportSystemError(errno, _("failed to write data to config '%s'"),
> +                             configFile);
> +        goto error;
> +    }
> +
> +    VIR_FREE(configFile);
> +    return 0;
> +
> + error:
> +    VIR_FREE(configFile);
> +    return -1;
> +}
> +
> +static int
> +qemuProcessPrepareSevGuestInput(virQEMUDriverPtr driver,
> +                                virDomainObjPtr vm)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virDomainDefPtr def = vm->def;
> +    virQEMUCapsPtr qemuCaps = priv->qemuCaps;
> +    virDomainSevDefPtr sev = def->sev;
> +    char *configDir = NULL;
> +    char *domPath = virDomainDefGetShortName(def);
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +
> +    if (!sev)
> +        return 0;
> +
> +    VIR_DEBUG("Prepare SEV guest");
> +
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                        _("Domain %s asked for 'sev' launch but "
> +                          "QEMU does not support SEV feature"), vm->def->name);
> +        return -1;
> +    }
> +
> +    if (virAsprintf(&configDir, "%s/sev/%s", cfg->configDir, domPath) < 0)
> +        goto error;

We already have a directory where we put transient things like the QEMU
monitor socket, pid file, etc. This is defineed by 'priv->libDir', so
just use that directory and don't create extra sub-dirs.



> +    if (virFileMakePathWithMode(configDir, S_IRWXU) < 0) {
> +        virReportSystemError(errno, _("cannot create config directory '%s'"),
> +                             configDir);
> +        goto error;
> +    }
> +
> +    if (sev->dh_cert) {
> +        if (qemuBuildSevCreateFile(configDir, "dh_cert", sev->dh_cert) < 0)
> +            goto error1;
> +    }
> +
> +    if (sev->session) {
> +        if (qemuBuildSevCreateFile(configDir, "session", sev->session) < 0)
> +            goto error1;
> +    }
> +
> +    VIR_FREE(domPath);
> +    sev->configDir = configDir;
> +    return 0;
> +
> + error1:
> +    virFileDeleteTree(configDir);
> + error:
> +    VIR_FREE(configDir);
> +    VIR_FREE(domPath);
> +    return -1;
> +}
>  
>  static int
>  qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
> @@ -5866,6 +5953,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
>      if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
>          goto cleanup;
>  
> +    if (qemuProcessPrepareSevGuestInput(driver, vm) < 0)
> +        goto cleanup;
> +
>      ret = 0;
>   cleanup:
>      virObjectUnref(cfg);
> @@ -6535,6 +6625,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>      }
>  
>      qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
> +    qemuProcessDestroySevPaths(vm->def->sev);
>  
>      vm->def->id = -1;
>  
> -- 
> 2.14.3
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/9] qemu: add support to launch SEV guest
Posted by Brijesh Singh, 14 weeks ago

On 03/12/2018 08:41 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 11:12:03AM -0600, Brijesh Singh wrote:
>> QEMU >= 2.12 provides 'sev-guest' object which is used to launch encrypted
>> VMs on AMD platform using SEV feature. The various inputs required to
>> launch SEV guest is provided through the <launch-security> tag. A typical
>> SEV guest launch command line looks like this:
>>
>> # $QEMU ...\
>>    -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 ...\
>>    -machine memory-encryption=sev0 \
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   src/qemu/qemu_command.c | 33 ++++++++++++++++++
>>   src/qemu/qemu_process.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 124 insertions(+)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index fa0aa5d5c3d4..39f136a389cb 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -9663,6 +9663,36 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
>>       return 0;
>>   }
>>   
>> +static void
>> +qemuBuildSevCommandLine(virCommandPtr cmd,
>> +                        virDomainSevDefPtr sev)
>> +{
>> +    virBuffer obj = VIR_BUFFER_INITIALIZER;
>> +    char *path = NULL;
>> +
>> +    VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
>> +              sev->policy, sev->cbitpos, sev->reduced_phys_bits);
>> +
>> +    virCommandAddArgList(cmd, "-machine", "memory-encryption=sev0", NULL);
> 
> 
> Hmm, so this will result in us passing  '-machine' many times, and relying
> on QEMU to acculate the args. It should work, but IME it could be a little
> surprising to people debuggnig qemu to see repeated args for -machine.
> 
> Could we just call this method from the place that  constructs the
> current -machine arg so we append to that.


OK, I will take a look and try to avoid duplicate "-machine".

> 
>> +
>> +    virBufferAsprintf(&obj, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos);
>> +    virBufferAsprintf(&obj, ",reduced-phys-bits=%d", sev->reduced_phys_bits);
>> +    virBufferAsprintf(&obj, ",policy=0x%x", sev->policy);
>> +
>> +    if (sev->dh_cert) {
>> +        ignore_value(virAsprintf(&path, "%s/dh_cert.base64", sev->configDir));
>> +        virBufferAsprintf(&obj, ",dh-cert-file=%s", path);
>> +        VIR_FREE(path);
>> +    }
>> +
>> +    if (sev->session) {
>> +        ignore_value(virAsprintf(&path, "%s/session.base64", sev->configDir));
>> +        virBufferAsprintf(&obj, ",session-file=%s", path);
>> +        VIR_FREE(path);
>> +    }
>> +
>> +    virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL);
>> +}
>>   
>>   static int
>>   qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
>> @@ -10108,6 +10138,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>>       if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
>>           goto error;
>>   
>> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev)
>> +        qemuBuildSevCommandLine(cmd, def->sev);
>> +
>>       if (snapshot)
>>           virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
>>   
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 57c06c7c1550..349e12b6dc12 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -3457,6 +3457,16 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> +static void
>> +qemuProcessDestroySevPaths(virDomainSevDefPtr sev)
>> +{
>> +    if (!sev)
>> +        return;
>> +
>> +    virFileDeleteTree(sev->configDir);
> 
> Just delete the individual files we created, no recursive delete please.
> 

Will do.


>> +    VIR_FREE(sev->configDir);
>> +}
>> +
>>   int
>>   qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
>>                                      virDomainObjPtr vm,
>> @@ -5741,6 +5751,83 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
>>       return ret;
>>   }
>>   
>> +static int
>> +qemuBuildSevCreateFile(const char *configDir, const char *name,
>> +                       const char *data)
>> +{
>> +    char *configFile;
>> +
>> +    if (!(configFile = virFileBuildPath(configDir, name, ".base64")))
>> +        return -1;
>> +
>> +    if (virFileRewriteStr(configFile, S_IRUSR | S_IWUSR, data) < 0) {
>> +        virReportSystemError(errno, _("failed to write data to config '%s'"),
>> +                             configFile);
>> +        goto error;
>> +    }
>> +
>> +    VIR_FREE(configFile);
>> +    return 0;
>> +
>> + error:
>> +    VIR_FREE(configFile);
>> +    return -1;
>> +}
>> +
>> +static int
>> +qemuProcessPrepareSevGuestInput(virQEMUDriverPtr driver,
>> +                                virDomainObjPtr vm)
>> +{
>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>> +    virDomainDefPtr def = vm->def;
>> +    virQEMUCapsPtr qemuCaps = priv->qemuCaps;
>> +    virDomainSevDefPtr sev = def->sev;
>> +    char *configDir = NULL;
>> +    char *domPath = virDomainDefGetShortName(def);
>> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> +
>> +    if (!sev)
>> +        return 0;
>> +
>> +    VIR_DEBUG("Prepare SEV guest");
>> +
>> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                        _("Domain %s asked for 'sev' launch but "
>> +                          "QEMU does not support SEV feature"), vm->def->name);
>> +        return -1;
>> +    }
>> +
>> +    if (virAsprintf(&configDir, "%s/sev/%s", cfg->configDir, domPath) < 0)
>> +        goto error;
> 
> We already have a directory where we put transient things like the QEMU
> monitor socket, pid file, etc. This is defineed by 'priv->libDir', so
> just use that directory and don't create extra sub-dirs.
> 
> 

Will use priv->libDir in next rev. thanks


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info
Posted by Brijesh Singh, 15 weeks ago
The API can be used outside the libvirt to get the launch security
information. When SEV is enabled, the API can be used to get the
measurement of the launch process.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 include/libvirt/libvirt-domain.h | 17 ++++++++++++++
 src/driver-hypervisor.h          |  7 ++++++
 src/libvirt-domain.c             | 50 ++++++++++++++++++++++++++++++++++++++++
 src/libvirt_public.syms          |  5 ++++
 4 files changed, 79 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 4048acf38aaf..11c3fec92bfa 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4756,4 +4756,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
                                 unsigned int action,
                                 unsigned int flags);
 
+/**
+ * Launch Security API
+ */
+
+/**
+ * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
+ *
+ * Macro represents the launch measurement of the SEV guest,
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
+
+int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
+                                   virTypedParameterPtr params,
+                                   int *nparams,
+                                   unsigned int flags);
+
 #endif /* __VIR_LIBVIRT_DOMAIN_H__ */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index ce0e2b252552..dc4873a8ad1c 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1283,6 +1283,12 @@ typedef int
                                   unsigned int action,
                                   unsigned int flags);
 
+typedef int
+(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
+                                     virTypedParameterPtr params,
+                                     int *nparams,
+                                     unsigned int flags);
+
 
 typedef struct _virHypervisorDriver virHypervisorDriver;
 typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1528,6 +1534,7 @@ struct _virHypervisorDriver {
     virDrvDomainSetVcpu domainSetVcpu;
     virDrvDomainSetBlockThreshold domainSetBlockThreshold;
     virDrvDomainSetLifecycleAction domainSetLifecycleAction;
+    virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
 };
 
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index eaec0979ad49..21356bb92894 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12095,3 +12095,53 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
     virDispatchError(domain->conn);
     return -1;
 }
+
+/**
+ * virDomainGetLaunchSecurityInfo:
+ * @domain: a domain object
+ * @params: where to store security info
+ * @nparams: number of items in @params
+ * @flags: bitwise-OR of virDomainModificationImpact
+ *
+ * Get the launch security info. In case of the SEV guest, this will
+ * return the launch measurement.
+ *
+ * Returns -1 in case of failure, 0 in case of success.
+ */
+int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
+                                   virTypedParameterPtr params,
+                                   int *nparams,
+                                   unsigned int flags)
+{
+    virConnectPtr conn;
+
+    VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
+                     params, nparams, flags);
+
+    virResetLastError();
+
+    virCheckDomainReturn(domain, -1);
+    virCheckNonNegativeArgGoto(*nparams, error);
+    if (*nparams != 0)
+        virCheckNonNullArgGoto(params, error);
+
+    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+        flags |= VIR_TYPED_PARAM_STRING_OKAY;
+
+    conn = domain->conn;
+
+    if (conn->driver->domainGetLaunchSecurityInfo) {
+        int ret;
+        ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
+                                                        nparams, flags);
+        if (ret < 0)
+            goto error;
+        return ret;
+    }
+    virReportUnsupportedError();
+
+ error:
+    virDispatchError(domain->conn);
+    return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 95df3a0dbc7b..caba2862d371 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
         virStoragePoolLookupByTargetPath;
 } LIBVIRT_3.9.0;
 
+LIBVIRT_4.2.0 {
+    global:
+        virDomainGetLaunchSecurityInfo;
+} LIBVIRT_4.1.0;
+
 # .... define new API here using predicted next version number ....
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info
Posted by Daniel P. Berrangé, 14 weeks ago
On Thu, Mar 08, 2018 at 11:12:04AM -0600, Brijesh Singh wrote:
> The API can be used outside the libvirt to get the launch security
> information. When SEV is enabled, the API can be used to get the
> measurement of the launch process.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  include/libvirt/libvirt-domain.h | 17 ++++++++++++++
>  src/driver-hypervisor.h          |  7 ++++++
>  src/libvirt-domain.c             | 50 ++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  5 ++++
>  4 files changed, 79 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 4048acf38aaf..11c3fec92bfa 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4756,4 +4756,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>                                  unsigned int action,
>                                  unsigned int flags);
>  
> +/**
> + * Launch Security API
> + */
> +
> +/**
> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
> + *
> + * Macro represents the launch measurement of the SEV guest,
> + * as VIR_TYPED_PARAM_STRING.
> + */
> +#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
> +
> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
> +                                   virTypedParameterPtr params,
> +                                   int *nparams,
> +                                   unsigned int flags);

These days we prefer new APIs to use

       virTypedParameterPtr *params,

and have the API implementation allocate the right number of
elements for the array, so the caller doesn't have to allocate
anything itself - only free the returned memory.

See virDomainGetJobStats for an example.

> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index ce0e2b252552..dc4873a8ad1c 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1283,6 +1283,12 @@ typedef int
>                                    unsigned int action,
>                                    unsigned int flags);
>  
> +typedef int
> +(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
> +                                     virTypedParameterPtr params,
> +                                     int *nparams,
> +                                     unsigned int flags);
> +
>  
>  typedef struct _virHypervisorDriver virHypervisorDriver;
>  typedef virHypervisorDriver *virHypervisorDriverPtr;
> @@ -1528,6 +1534,7 @@ struct _virHypervisorDriver {
>      virDrvDomainSetVcpu domainSetVcpu;
>      virDrvDomainSetBlockThreshold domainSetBlockThreshold;
>      virDrvDomainSetLifecycleAction domainSetLifecycleAction;
> +    virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
>  };
>  
>  
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index eaec0979ad49..21356bb92894 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -12095,3 +12095,53 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>      virDispatchError(domain->conn);
>      return -1;
>  }
> +
> +/**
> + * virDomainGetLaunchSecurityInfo:
> + * @domain: a domain object
> + * @params: where to store security info
> + * @nparams: number of items in @params
> + * @flags: bitwise-OR of virDomainModificationImpact

This API doesn't use virDomainModificationImpact. So just say

   @flags currently used, set to 0

> + *
> + * Get the launch security info. In case of the SEV guest, this will
> + * return the launch measurement.
> + *
> + * Returns -1 in case of failure, 0 in case of success.
> + */
> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
> +                                   virTypedParameterPtr params,
> +                                   int *nparams,
> +                                   unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
> +                     params, nparams, flags);
> +
> +    virResetLastError();
> +
> +    virCheckDomainReturn(domain, -1);
> +    virCheckNonNegativeArgGoto(*nparams, error);
> +    if (*nparams != 0)
> +        virCheckNonNullArgGoto(params, error);

We should require both args to be non-null, when we do allocation
ourselves.

I think we probably want to forbid this method for read-only
users, so add

  virCheckReadOnlyGoto(conn->flags, error);



> +
> +    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> +                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
> +        flags |= VIR_TYPED_PARAM_STRING_OKAY;
> +
> +    conn = domain->conn;
> +
> +    if (conn->driver->domainGetLaunchSecurityInfo) {
> +        int ret;
> +        ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
> +                                                        nparams, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +    virReportUnsupportedError();
> +
> + error:
> +    virDispatchError(domain->conn);
> +    return -1;
> +}
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0dbc7b..caba2862d371 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
>          virStoragePoolLookupByTargetPath;
>  } LIBVIRT_3.9.0;
>  
> +LIBVIRT_4.2.0 {
> +    global:
> +        virDomainGetLaunchSecurityInfo;
> +} LIBVIRT_4.1.0;
> +
>  # .... define new API here using predicted next version number ....
> -- 
> 2.14.3
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info
Posted by Brijesh Singh, 14 weeks ago

On 03/12/2018 07:01 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 11:12:04AM -0600, Brijesh Singh wrote:
>> The API can be used outside the libvirt to get the launch security
>> information. When SEV is enabled, the API can be used to get the
>> measurement of the launch process.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   include/libvirt/libvirt-domain.h | 17 ++++++++++++++
>>   src/driver-hypervisor.h          |  7 ++++++
>>   src/libvirt-domain.c             | 50 ++++++++++++++++++++++++++++++++++++++++
>>   src/libvirt_public.syms          |  5 ++++
>>   4 files changed, 79 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>> index 4048acf38aaf..11c3fec92bfa 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -4756,4 +4756,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>>                                   unsigned int action,
>>                                   unsigned int flags);
>>   
>> +/**
>> + * Launch Security API
>> + */
>> +
>> +/**
>> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
>> + *
>> + * Macro represents the launch measurement of the SEV guest,
>> + * as VIR_TYPED_PARAM_STRING.
>> + */
>> +#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
>> +
>> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
>> +                                   virTypedParameterPtr params,
>> +                                   int *nparams,
>> +                                   unsigned int flags);
> 
> These days we prefer new APIs to use
> 
>         virTypedParameterPtr *params,
> 
> and have the API implementation allocate the right number of
> elements for the array, so the caller doesn't have to allocate
> anything itself - only free the returned memory.
> 
> See virDomainGetJobStats for an example.
> 

Ah okay, I will take a look and change the API to allocate the array 
elements instead of forcing caller to allocate the arrays.



>> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
>> index ce0e2b252552..dc4873a8ad1c 100644
>> --- a/src/driver-hypervisor.h
>> +++ b/src/driver-hypervisor.h
>> @@ -1283,6 +1283,12 @@ typedef int
>>                                     unsigned int action,
>>                                     unsigned int flags);
>>   
>> +typedef int
>> +(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
>> +                                     virTypedParameterPtr params,
>> +                                     int *nparams,
>> +                                     unsigned int flags);
>> +
>>   
>>   typedef struct _virHypervisorDriver virHypervisorDriver;
>>   typedef virHypervisorDriver *virHypervisorDriverPtr;
>> @@ -1528,6 +1534,7 @@ struct _virHypervisorDriver {
>>       virDrvDomainSetVcpu domainSetVcpu;
>>       virDrvDomainSetBlockThreshold domainSetBlockThreshold;
>>       virDrvDomainSetLifecycleAction domainSetLifecycleAction;
>> +    virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
>>   };
>>   
>>   
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index eaec0979ad49..21356bb92894 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -12095,3 +12095,53 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>>       virDispatchError(domain->conn);
>>       return -1;
>>   }
>> +
>> +/**
>> + * virDomainGetLaunchSecurityInfo:
>> + * @domain: a domain object
>> + * @params: where to store security info
>> + * @nparams: number of items in @params
>> + * @flags: bitwise-OR of virDomainModificationImpact
> 
> This API doesn't use virDomainModificationImpact. So just say
> 
>     @flags currently used, set to 0
> 

Noted. thanks


>> + *
>> + * Get the launch security info. In case of the SEV guest, this will
>> + * return the launch measurement.
>> + *
>> + * Returns -1 in case of failure, 0 in case of success.
>> + */
>> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
>> +                                   virTypedParameterPtr params,
>> +                                   int *nparams,
>> +                                   unsigned int flags)
>> +{
>> +    virConnectPtr conn;
>> +
>> +    VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
>> +                     params, nparams, flags);
>> +
>> +    virResetLastError();
>> +
>> +    virCheckDomainReturn(domain, -1);
>> +    virCheckNonNegativeArgGoto(*nparams, error);
>> +    if (*nparams != 0)
>> +        virCheckNonNullArgGoto(params, error);
> 
> We should require both args to be non-null, when we do allocation
> ourselves.
> 
> I think we probably want to forbid this method for read-only
> users, so add
> 
>    virCheckReadOnlyGoto(conn->flags, error);
> 
> 
> 

Okay, got it.


>> +
>> +    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
>> +                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
>> +        flags |= VIR_TYPED_PARAM_STRING_OKAY;
>> +
>> +    conn = domain->conn;
>> +
>> +    if (conn->driver->domainGetLaunchSecurityInfo) {
>> +        int ret;
>> +        ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
>> +                                                        nparams, flags);
>> +        if (ret < 0)
>> +            goto error;
>> +        return ret;
>> +    }
>> +    virReportUnsupportedError();
>> +
>> + error:
>> +    virDispatchError(domain->conn);
>> +    return -1;
>> +}
>> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
>> index 95df3a0dbc7b..caba2862d371 100644
>> --- a/src/libvirt_public.syms
>> +++ b/src/libvirt_public.syms
>> @@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
>>           virStoragePoolLookupByTargetPath;
>>   } LIBVIRT_3.9.0;
>>   
>> +LIBVIRT_4.2.0 {
>> +    global:
>> +        virDomainGetLaunchSecurityInfo;
>> +} LIBVIRT_4.1.0;
>> +
>>   # .... define new API here using predicted next version number ....
>> -- 
>> 2.14.3
>>
> 
> Regards,
> Daniel
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/9] remote: implement the remote protocol for launch security
Posted by Brijesh Singh, 15 weeks ago
Add remote support for launch security info.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 src/remote/remote_daemon_dispatch.c | 63 +++++++++++++++++++++++++++++++++++++
 src/remote/remote_driver.c          | 52 +++++++++++++++++++++++++++++-
 src/remote/remote_protocol.x        | 22 ++++++++++++-
 src/remote_protocol-structs         | 13 ++++++++
 4 files changed, 148 insertions(+), 2 deletions(-)

diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index ea67cb7bc018..d3343c9ec972 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -3087,6 +3087,69 @@ remoteDispatchNodeGetMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED,
     return rv;
 }
 
+static int
+remoteDispatchDomainGetLaunchSecurityInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+                                          virNetServerClientPtr client ATTRIBUTE_UNUSED,
+                                          virNetMessagePtr msg ATTRIBUTE_UNUSED,
+                                          virNetMessageErrorPtr rerr,
+                                          remote_domain_get_launch_security_info_args *args,
+                                          remote_domain_get_launch_security_info_ret *ret)
+{
+    virDomainPtr dom = NULL;
+    virTypedParameterPtr params = NULL;
+    int nparams = 0;
+    int rv = -1;
+    unsigned int flags;
+    struct daemonClientPrivate *priv =
+        virNetServerClientGetPrivateData(client);
+
+    if (!priv->conn) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+        goto cleanup;
+    }
+
+    flags = args->flags;
+
+    if (args->nparams > REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
+        goto cleanup;
+    }
+
+    if (args->nparams && VIR_ALLOC_N(params, args->nparams) < 0)
+        goto cleanup;
+    nparams = args->nparams;
+
+    if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+        goto cleanup;
+
+    if (virDomainGetLaunchSecurityInfo(dom, params, &nparams, args->flags) < 0)
+        goto cleanup;
+
+    /* In this case, we need to send back the number of parameters
+     * supported
+     */
+    if (args->nparams == 0) {
+        ret->nparams = nparams;
+        goto success;
+    }
+
+    if (virTypedParamsSerialize(params, nparams,
+                                (virTypedParameterRemotePtr *) &ret->params.params_val,
+                                &ret->params.params_len,
+                                flags) < 0)
+        goto cleanup;
+
+ success:
+    rv = 0;
+
+ cleanup:
+    if (rv < 0)
+        virNetMessageSaveError(rerr);
+    virTypedParamsFree(params, nparams);
+    virObjectUnref(dom);
+    return rv;
+}
+
 static int
 remoteDispatchDomainGetPerfEvents(virNetServerPtr server ATTRIBUTE_UNUSED,
                                   virNetServerClientPtr client ATTRIBUTE_UNUSED,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9ea726dc45c0..695ec629c5cd 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1951,6 +1951,55 @@ remoteDomainGetNumaParameters(virDomainPtr domain,
     return rv;
 }
 
+static int
+remoteDomainGetLaunchSecurityInfo(virDomainPtr domain,
+                                  virTypedParameterPtr params,
+                                  int *nparams,
+                                  unsigned int flags)
+{
+    int rv = -1;
+    remote_domain_get_launch_security_info_args args;
+    remote_domain_get_launch_security_info_ret ret;
+    struct private_data *priv = domain->conn->privateData;
+
+    remoteDriverLock(priv);
+
+    make_nonnull_domain(&args.dom, domain);
+    args.flags = flags;
+    args.nparams = *nparams;
+
+    memset(&ret, 0, sizeof(ret));
+    if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_LAUNCH_SECURITY_INFO,
+             (xdrproc_t) xdr_remote_domain_get_launch_security_info_args, (char *) &args,
+             (xdrproc_t) xdr_remote_domain_get_launch_security_info_ret, (char *) &ret) == -1)
+        goto done;
+
+    /* Handle the case when the caller does not know the number of parameters
+     * and is asking for the number of parameters supported
+     */
+    if (*nparams == 0) {
+        *nparams = ret.nparams;
+        rv = 0;
+        goto cleanup;
+    }
+
+    if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val,
+                                  ret.params.params_len,
+                                  REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX,
+                                  &params,
+                                  nparams) < 0)
+        goto cleanup;
+
+    rv = 0;
+
+ cleanup:
+    xdr_free((xdrproc_t) xdr_remote_domain_get_launch_security_info_ret,
+             (char *) &ret);
+ done:
+    remoteDriverUnlock(priv);
+    return rv;
+}
+
 static int
 remoteDomainGetPerfEvents(virDomainPtr domain,
                           virTypedParameterPtr *params,
@@ -8497,7 +8546,8 @@ static virHypervisorDriver hypervisor_driver = {
     .domainSetGuestVcpus = remoteDomainSetGuestVcpus, /* 2.0.0 */
     .domainSetVcpu = remoteDomainSetVcpu, /* 3.1.0 */
     .domainSetBlockThreshold = remoteDomainSetBlockThreshold, /* 3.2.0 */
-    .domainSetLifecycleAction = remoteDomainSetLifecycleAction /* 3.9.0 */
+    .domainSetLifecycleAction = remoteDomainSetLifecycleAction, /* 3.9.0 */
+    .domainGetLaunchSecurityInfo = remoteDomainGetLaunchSecurityInfo /* 4.2.0 */
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 9dbd497b2fff..de31c997358e 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -253,6 +253,9 @@ const REMOTE_DOMAIN_IP_ADDR_MAX = 2048;
 /* Upper limit on number of guest vcpu information entries */
 const REMOTE_DOMAIN_GUEST_VCPU_PARAMS_MAX = 64;
 
+/* Upper limit on number of launch security information entries */
+const REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX = 64;
+
 /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
 typedef opaque remote_uuid[VIR_UUID_BUFLEN];
 
@@ -3448,6 +3451,17 @@ struct remote_domain_set_lifecycle_action_args {
     unsigned int flags;
 };
 
+struct remote_domain_get_launch_security_info_args {
+    remote_nonnull_domain dom;
+    int nparams;
+    unsigned int flags;
+};
+
+struct remote_domain_get_launch_security_info_ret {
+    remote_typed_param params<REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX>;
+    int nparams;
+};
+
 /*----- Protocol. -----*/
 
 /* Define the program number, protocol version and procedure numbers here. */
@@ -6135,5 +6149,11 @@ enum remote_procedure {
      * @priority: high
      * @acl: storage_pool:getattr
      */
-    REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391
+    REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391,
+
+    /**
+     * @generate: none
+     * @acl: domain:read
+     */
+    REMOTE_PROC_DOMAIN_GET_LAUNCH_SECURITY_INFO = 392
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index f45aba27a202..4974e619f7f0 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2877,6 +2877,18 @@ struct remote_domain_set_lifecycle_action_args {
         u_int                      action;
         u_int                      flags;
 };
+struct remote_domain_get_launch_security_info_args {
+        remote_nonnull_domain      dom;
+        int                        nparams;
+        u_int                      flags;
+};
+struct remote_domain_get_launch_security_info_ret {
+        struct {
+                u_int              params_len;
+                remote_typed_param * params_val;
+        } params;
+        int                        nparams;
+};
 enum remote_procedure {
         REMOTE_PROC_CONNECT_OPEN = 1,
         REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -3269,4 +3281,5 @@ enum remote_procedure {
         REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 389,
         REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390,
         REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391,
+        REMOTE_PROC_DOMAIN_GET_LAUNCH_SECURITY_INFO = 392,
 };
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 7/9] qemu_driver: add support to launch security info
Posted by Brijesh Singh, 15 weeks ago
This patch implement the internal driver API for launch event into
qemu driver. When SEV is enabled, execute 'query-sev-launch-measurement'
to get the measurement of memory encrypted through launch sequence.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 src/qemu/qemu_driver.c       | 72 ++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_monitor.c      |  8 +++++
 src/qemu/qemu_monitor.h      |  3 ++
 src/qemu/qemu_monitor_json.c | 32 ++++++++++++++++++++
 src/qemu/qemu_monitor_json.h |  2 ++
 5 files changed, 117 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 96454c17c03d..bcd539b6aff3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21254,6 +21254,77 @@ qemuDomainSetLifecycleAction(virDomainPtr dom,
     return ret;
 }
 
+static int qemuDomainGetSevMeasurement(virQEMUDriverPtr driver,
+                                       virDomainObjPtr vm,
+                                       virTypedParameterPtr params,
+                                       int *nparams)
+{
+    int ret = -1;
+    char *tmp;
+    virTypedParameterPtr p;
+
+    if ((*nparams) == 0) {
+        *nparams = 1;
+        return 0;
+    }
+
+    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+        return -1;
+
+    if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+        goto endjob;
+
+    tmp = qemuMonitorGetSevMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon);
+    if (tmp == NULL)
+        goto endjob;
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        goto endjob;
+
+    p = &params[0];
+    if (virTypedParameterAssign(p, VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT,
+                            VIR_TYPED_PARAM_STRING, tmp) < 0)
+        goto endjob;
+
+    ret = 0;
+
+ endjob:
+    qemuDomainObjEndJob(driver, vm);
+    return ret;
+}
+
+
+static int
+qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
+                                virTypedParameterPtr params,
+                                int *nparams,
+                                unsigned int flags)
+{
+    virQEMUDriverPtr driver = domain->conn->privateData;
+    virDomainObjPtr vm;
+    int ret = -1;
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG |
+                  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+    if (!(vm = qemuDomObjFromDomain(domain)))
+        goto cleanup;
+
+    if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0)
+        goto cleanup;
+
+    if (vm->def->sev) {
+        if (qemuDomainGetSevMeasurement(driver, vm, params, nparams) < 0)
+            goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
 
 static virHypervisorDriver qemuHypervisorDriver = {
     .name = QEMU_DRIVER_NAME,
@@ -21474,6 +21545,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
     .domainSetVcpu = qemuDomainSetVcpu, /* 3.1.0 */
     .domainSetBlockThreshold = qemuDomainSetBlockThreshold, /* 3.2.0 */
     .domainSetLifecycleAction = qemuDomainSetLifecycleAction, /* 3.9.0 */
+    .domainGetLaunchSecurityInfo = qemuDomainGetLaunchSecurityInfo, /* 4.2.0 */
 };
 
 
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 195248c88ae1..e3dd078e4e73 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4400,3 +4400,11 @@ qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
 
     return qemuMonitorJSONSetWatchdogAction(mon, action);
 }
+
+char *
+qemuMonitorGetSevMeasurement(qemuMonitorPtr mon)
+{
+    QEMU_CHECK_MONITOR_NULL(mon);
+
+    return qemuMonitorJSONGetSevMeasurement(mon);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 1b2513650c58..dd0821178c47 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1176,4 +1176,7 @@ virJSONValuePtr qemuMonitorQueryNamedBlockNodes(qemuMonitorPtr mon);
 
 int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
                                  const char *action);
+char *
+qemuMonitorGetSevMeasurement(qemuMonitorPtr mon);
+
 #endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 94a1af1d3f75..d652da0c4db2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7955,3 +7955,35 @@ qemuMonitorJSONSetWatchdogAction(qemuMonitorPtr mon,
     virJSONValueFree(reply);
     return ret;
 }
+
+char *
+qemuMonitorJSONGetSevMeasurement(qemuMonitorPtr mon)
+{
+    const char *tmp;
+    char *measurement = NULL;
+    virJSONValuePtr cmd;
+    virJSONValuePtr reply = NULL;
+    virJSONValuePtr data;
+
+    if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-launch-measure", NULL)))
+         return NULL;
+
+    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+        goto cleanup;
+
+    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+        goto cleanup;
+
+    data = virJSONValueObjectGetObject(reply, "return");
+
+    if (!(tmp = virJSONValueObjectGetString(data, "data")))
+        goto cleanup;
+
+    if (VIR_STRDUP(measurement, tmp) < 0)
+        goto cleanup;
+
+ cleanup:
+    virJSONValueFree(cmd);
+    virJSONValueFree(reply);
+    return measurement;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 305f789902e9..b83160a20e00 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -342,6 +342,8 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon,
 
 int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon);
 
+char *qemuMonitorJSONGetSevMeasurement(qemuMonitorPtr mon);
+
 int qemuMonitorJSONGetVersion(qemuMonitorPtr mon,
                               int *major,
                               int *minor,
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 8/9] virsh: implement new command for launch security
Posted by Brijesh Singh, 15 weeks ago
Add new 'launch-security' command, the command can be used to get or set
the launch security information when booting encrypted VMs.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 tools/virsh-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 29bc8e6db18b..556dbca8e877 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13872,6 +13872,93 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
     return ret >= 0;
 }
 
+/*
+ * "launch-security" command
+ */
+static const vshCmdInfo info_launch_security[] = {
+    {.name = "help",
+        .data = N_("Get or set launch-security information")
+    },
+    {.name = "desc",
+        .data = N_("Get or set the current launch-security information for a guest"
+                   " domain.\n"
+                   "    To get the launch-security information use following command: \n\n"
+                   "    virsh # launch-security <domain>")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_launch_security[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "get",
+     .type = VSH_OT_STRING,
+     .help = N_("Show the launch-security info")
+    },
+    VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+    VIRSH_COMMON_OPT_DOMAIN_LIVE,
+    VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+    {.name = NULL}
+};
+
+static void
+virshPrintLaunchSecurityInfo(vshControl *ctl, virTypedParameterPtr params,
+                             int nparams)
+{
+    size_t i;
+
+    for (i = 0; i < nparams; i++) {
+        if (params[i].type == VIR_TYPED_PARAM_STRING)
+            vshPrintExtra(ctl, "%-15s: %s\n", params[i].field, params[i].value.s);
+    }
+}
+
+static bool
+cmdLaunchSecurity(vshControl *ctl, const vshCmd *cmd)
+{
+    virDomainPtr dom;
+    int nparams = 0;
+    virTypedParameterPtr params = NULL;
+    bool ret = false;
+    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+    bool current = vshCommandOptBool(cmd, "current");
+    bool config = vshCommandOptBool(cmd, "config");
+    bool live = vshCommandOptBool(cmd, "live");
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+    if (config)
+        flags |= VIR_DOMAIN_AFFECT_CONFIG;
+    if (live)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
+
+    /* get the number of parameters */
+    if (virDomainGetLaunchSecurityInfo(dom, NULL, &nparams, flags) != 0) {
+        vshError(ctl, "%s", _("Unable to get launch security info"));
+        goto cleanup;
+    }
+
+    /* now go get all the launch-security parameters */
+    params = vshCalloc(ctl, nparams, sizeof(*params));
+
+    if (virDomainGetLaunchSecurityInfo(dom, params, &nparams, flags) != 0) {
+        vshError(ctl, "%s", _("Unable to get launch security info"));
+        goto cleanup;
+    }
+
+    virshPrintLaunchSecurityInfo(ctl, params, nparams);
+
+    ret = true;
+ cleanup:
+    virTypedParamsFree(params, nparams);
+    virshDomainFree(dom);
+    return ret;
+}
+
+
 const vshCmdDef domManagementCmds[] = {
     {.name = "attach-device",
      .handler = cmdAttachDevice,
@@ -14487,5 +14574,11 @@ const vshCmdDef domManagementCmds[] = {
      .info = info_domblkthreshold,
      .flags = 0
     },
+    {.name = "launch-security",
+     .handler = cmdLaunchSecurity,
+     .opts = opts_launch_security,
+     .info = info_launch_security,
+     .flags = 0
+    },
     {.name = NULL}
 };
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 9/9] tests: extend tests to include sev specific tag parsing
Posted by Brijesh Singh, 15 weeks ago
From: Xiaogang Chen <Xiaogang.Chen@amd.com>

Update qemuxml2xmltest, genericxml2xmltest and qemuxml2argvtest to include
sev specific tag, a typical SEV specific tag looks like

<launch-security type='sev>
  <cbitpos>47</cbitpos>
  <reduced-phys-bits>1</reduced-phys-bits>
  <policy>1</policy>
</launch-security>

Signed-off-by: Xiaogang Chen <Xiaogang.Chen@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 tests/genericxml2xmlindata/sev.xml  | 22 +++++++++++++++++++++
 tests/genericxml2xmloutdata/sev.xml | 22 +++++++++++++++++++++
 tests/genericxml2xmltest.c          |  2 ++
 tests/qemuxml2argvdata/sev.args     | 25 ++++++++++++++++++++++++
 tests/qemuxml2argvdata/sev.xml      | 35 +++++++++++++++++++++++++++++++++
 tests/qemuxml2argvtest.c            |  2 ++
 tests/qemuxml2xmloutdata/sev.xml    | 39 +++++++++++++++++++++++++++++++++++++
 tests/qemuxml2xmltest.c             |  2 ++
 8 files changed, 149 insertions(+)
 create mode 100644 tests/genericxml2xmlindata/sev.xml
 create mode 100644 tests/genericxml2xmloutdata/sev.xml
 create mode 100644 tests/qemuxml2argvdata/sev.args
 create mode 100644 tests/qemuxml2argvdata/sev.xml
 create mode 100644 tests/qemuxml2xmloutdata/sev.xml

diff --git a/tests/genericxml2xmlindata/sev.xml b/tests/genericxml2xmlindata/sev.xml
new file mode 100644
index 000000000000..6f9a09d3a133
--- /dev/null
+++ b/tests/genericxml2xmlindata/sev.xml
@@ -0,0 +1,22 @@
+<domain type='qemu'>
+  <name>foobar</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>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>
+  </devices>
+  <launch-security type='sev'>
+    <cbitpos>47</cbitpos>
+    <reduced-phys-bits>1</reduced-phys-bits>
+    <policy>1</policy>
+  </launch-security>
+</domain>
diff --git a/tests/genericxml2xmloutdata/sev.xml b/tests/genericxml2xmloutdata/sev.xml
new file mode 100644
index 000000000000..6f9a09d3a133
--- /dev/null
+++ b/tests/genericxml2xmloutdata/sev.xml
@@ -0,0 +1,22 @@
+<domain type='qemu'>
+  <name>foobar</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>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>
+  </devices>
+  <launch-security type='sev'>
+    <cbitpos>47</cbitpos>
+    <reduced-phys-bits>1</reduced-phys-bits>
+    <policy>1</policy>
+  </launch-security>
+</domain>
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index c33fce192237..0ab95c000322 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -141,6 +141,8 @@ mymain(void)
     DO_TEST_FULL("cachetune-colliding-types", false, true,
                  TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
 
+    DO_TEST_DIFFERENT("sev");
+
     virObjectUnref(caps);
     virObjectUnref(xmlopt);
 
diff --git a/tests/qemuxml2argvdata/sev.args b/tests/qemuxml2argvdata/sev.args
new file mode 100644
index 000000000000..67f5505bf91d
--- /dev/null
+++ b/tests/qemuxml2argvdata/sev.args
@@ -0,0 +1,25 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-machine memory-encryption=sev0 \
+-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x1
diff --git a/tests/qemuxml2argvdata/sev.xml b/tests/qemuxml2argvdata/sev.xml
new file mode 100644
index 000000000000..267ea0cf4806
--- /dev/null
+++ b/tests/qemuxml2argvdata/sev.xml
@@ -0,0 +1,35 @@
+<domain type='qemu'>
+  <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='i686' machine='pc'>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-i686</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>
+  <launch-security type='sev'>
+    <cbitpos>47</cbitpos>
+    <reduced-phys-bits>1</reduced-phys-bits>
+    <policy>1</policy>
+  </launch-security>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 688846b9b464..382fa1a5f07d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2966,6 +2966,8 @@ mymain(void)
             QEMU_CAPS_HDA_DUPLEX);
     DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI);
 
+    DO_TEST("sev", QEMU_CAPS_SEV);
+
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakerootdir);
 
diff --git a/tests/qemuxml2xmloutdata/sev.xml b/tests/qemuxml2xmloutdata/sev.xml
new file mode 100644
index 000000000000..e603326799c1
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/sev.xml
@@ -0,0 +1,39 @@
+<domain type='qemu'>
+  <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='i686' machine='pc'>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-i686</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'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+  <launch-security type='sev'>
+    <cbitpos>47</cbitpos>
+    <reduced-phys-bits>1</reduced-phys-bits>
+    <policy>1</policy>
+  </launch-security>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0eb9e6c77a77..709405683652 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1344,6 +1344,8 @@ mymain(void)
 
     DO_TEST("user-aliases", NONE);
 
+    DO_TEST("sev", NONE);
+
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakerootdir);
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list