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.

Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 src/conf/domain_capabilities.h | 13 ++++++++
 src/qemu/qemu_capabilities.c   | 38 ++++++++++++++++++++++
 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, 144 insertions(+)

diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index fa4c1e442f57..72e9daf9120f 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;
+    unsigned int cbitpos;
+    unsigned int reduced_phys_bits;
+};
+
+
 struct _virDomainCaps {
     virObjectLockable parent;
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3eb5ed6d1a60..6da7cf7477c7 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;
 };
@@ -1694,6 +1697,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
     { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE },
     { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL },
     { "pl011", QEMU_CAPS_DEVICE_PL011 },
+    { "sev-guest", QEMU_CAPS_SEV },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
@@ -2770,6 +2774,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,
@@ -3273,6 +3292,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,
@@ -4906,6 +4938,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 1d67a97789e7..2820714b5c55 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 adfa87aba91b..aaa14f66fdfb 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -767,6 +767,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 08dfffdf6435..c51b98d2bda7 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6398,6 +6398,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

On 03/14/2018 11:44 AM, 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.
> 
> Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  src/conf/domain_capabilities.h | 13 ++++++++
>  src/qemu/qemu_capabilities.c   | 38 ++++++++++++++++++++++
>  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, 144 insertions(+)
> 

Now that the 2.12 capabilities were added, I started looking through
each of the upstream patch series that would make use of it - this is
just "next" on my list.

I tried applying just this patch, but kept getting:

29) caps_2.12.0(x86_64)
... libvirt: QEMU Driver error : internal error: query-cpu-definitions
reply data was not an array

a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was
returning NULL for @data after/when the "query-sev-capabilities".

I narrowed it down into the virQEMUCapsInitQMPMonitor when run during
qemucapabilitiestest (see [1])

> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index fa4c1e442f57..72e9daf9120f 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;
> +    unsigned int cbitpos;
> +    unsigned int reduced_phys_bits;
> +};
> +
> +
>  struct _virDomainCaps {
>      virObjectLockable parent;
>  
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 3eb5ed6d1a60..6da7cf7477c7 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",

This should also be "sev-guest" to be consistent with other
virQEMUCapsObjectTypes entries naming.

BTW: You can/should pull out this entry, the one in
virQEMUCapsObjectTypes and the qemu_capabilities.h change to add the
entry to virQEMUCapsFlags, then build and run:

  VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest

in order to get/see the changed
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml file.


>      );
>  
>  
> @@ -525,6 +526,8 @@ struct _virQEMUCaps {
>      size_t ngicCapabilities;
>      virGICCapability *gicCapabilities;
>  
> +    virSEVCapability *sevCapabilities;
> +
>      virQEMUCapsHostCPUData kvmCPU;
>      virQEMUCapsHostCPUData tcgCPU;
>  };
> @@ -1694,6 +1697,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>      { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE },
>      { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL },
>      { "pl011", QEMU_CAPS_DEVICE_PL011 },
> +    { "sev-guest", QEMU_CAPS_SEV },
>  };
>  
>  static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
> @@ -2770,6 +2774,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,
> @@ -3273,6 +3292,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,
> @@ -4906,6 +4938,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);

[1]
If I change the Clear to be a goto cleanup, then a slightly different
error appears:

libvirt: QEMU Driver error : internal error: 'cbitpos' field is missing

Checking the output of a

  LIBVIRT_DEBUG=1 VIR_TEST_DEBUG=1 VIR_TEST_RANGE=29 make -C tests check
TESTS=qemucapabilitiestest

gives a bit of a hint in the test-suite.log output:

2018-03-23 19:08:52.819+0000: 11727: debug :
qemuMonitorTestAddResponse:108 : Adding response to monitor command: '{
 "return": {   },   "id": "libvirt-1" }
...

So that says to me that we perhaps need some mocked output. Although I'm
not 100% sure - it doesn't seem previous adjustments needed those, but
there's lots that changes in this capabilities space and I'm not always
up to date on the mocking code.

BTW: resetting the last error in this case didn't fix the problem - it
just moved the cheese back to the original error. So there's some mocked
buffer somewhere that's being read - I'm hoping someone with a fresher
memory on how this works will be able to debug faster than I.

> +    }
> +
>      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,... */

I think this should be QEMU_CAPS_SEV_GUEST  (it's a consistency in
naming thing)

I'll defer to Dan and Peter on the rest...

John

>  
>      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 1d67a97789e7..2820714b5c55 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 adfa87aba91b..aaa14f66fdfb 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -767,6 +767,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 08dfffdf6435..c51b98d2bda7 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6398,6 +6398,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);
> 

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

Hi John,


On 03/23/2018 02:18 PM, John Ferlan wrote:
> 
> 
> On 03/14/2018 11:44 AM, 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.
>>
>> Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   src/conf/domain_capabilities.h | 13 ++++++++
>>   src/qemu/qemu_capabilities.c   | 38 ++++++++++++++++++++++
>>   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, 144 insertions(+)
>>
> 
> Now that the 2.12 capabilities were added, I started looking through
> each of the upstream patch series that would make use of it - this is
> just "next" on my list.
> 
> I tried applying just this patch, but kept getting:
> 
> 29) caps_2.12.0(x86_64)
> ... libvirt: QEMU Driver error : internal error: query-cpu-definitions
> reply data was not an array
> 
> a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was
> returning NULL for @data after/when the "query-sev-capabilities".
> 
> I narrowed it down into the virQEMUCapsInitQMPMonitor when run during
> qemucapabilitiestest (see [1])

I have not tried latest libvirt yet, I will try this today and debug to 
see what we are missing. I did the 'make check' before submitting the 
patch but at that time QEMU 2.12 was not available and we did not had 
updated caps_2.12.0.x86_64.xml and caps_2.12.0.x86_64.replies.

I will investigate a bit more and update with my findings.

thanks

> 
>> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
>> index fa4c1e442f57..72e9daf9120f 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;
>> +    unsigned int cbitpos;
>> +    unsigned int reduced_phys_bits;
>> +};
>> +
>> +
>>   struct _virDomainCaps {
>>       virObjectLockable parent;
>>   
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 3eb5ed6d1a60..6da7cf7477c7 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",
> 
> This should also be "sev-guest" to be consistent with other
> virQEMUCapsObjectTypes entries naming.
> 
> BTW: You can/should pull out this entry, the one in
> virQEMUCapsObjectTypes and the qemu_capabilities.h change to add the
> entry to virQEMUCapsFlags, then build and run:
> 
>    VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest
> 
> in order to get/see the changed
> tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml file.
> 
> 
>>       );
>>   
>>   
>> @@ -525,6 +526,8 @@ struct _virQEMUCaps {
>>       size_t ngicCapabilities;
>>       virGICCapability *gicCapabilities;
>>   
>> +    virSEVCapability *sevCapabilities;
>> +
>>       virQEMUCapsHostCPUData kvmCPU;
>>       virQEMUCapsHostCPUData tcgCPU;
>>   };
>> @@ -1694,6 +1697,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>>       { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE },
>>       { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL },
>>       { "pl011", QEMU_CAPS_DEVICE_PL011 },
>> +    { "sev-guest", QEMU_CAPS_SEV },
>>   };
>>   
>>   static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
>> @@ -2770,6 +2774,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,
>> @@ -3273,6 +3292,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,
>> @@ -4906,6 +4938,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);
> 
> [1]
> If I change the Clear to be a goto cleanup, then a slightly different
> error appears:
> 
> libvirt: QEMU Driver error : internal error: 'cbitpos' field is missing
> 
> Checking the output of a
> 
>    LIBVIRT_DEBUG=1 VIR_TEST_DEBUG=1 VIR_TEST_RANGE=29 make -C tests check
> TESTS=qemucapabilitiestest
> 
> gives a bit of a hint in the test-suite.log output:
> 
> 2018-03-23 19:08:52.819+0000: 11727: debug :
> qemuMonitorTestAddResponse:108 : Adding response to monitor command: '{
>   "return": {   },   "id": "libvirt-1" }
> ...
> 
> So that says to me that we perhaps need some mocked output. Although I'm
> not 100% sure - it doesn't seem previous adjustments needed those, but
> there's lots that changes in this capabilities space and I'm not always
> up to date on the mocking code.
> 
> BTW: resetting the last error in this case didn't fix the problem - it
> just moved the cheese back to the original error. So there's some mocked
> buffer somewhere that's being read - I'm hoping someone with a fresher
> memory on how this works will be able to debug faster than I.
> 
>> +    }
>> +
>>       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,... */
> 
> I think this should be QEMU_CAPS_SEV_GUEST  (it's a consistency in
> naming thing)
> 
> I'll defer to Dan and Peter on the rest...
> 
> John
> 
>>   
>>       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 1d67a97789e7..2820714b5c55 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 adfa87aba91b..aaa14f66fdfb 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -767,6 +767,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 08dfffdf6435..c51b98d2bda7 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -6398,6 +6398,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);
>>

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

On Mon, 2018-03-26 at 08:52 -0500, Brijesh Singh wrote:
> > I tried applying just this patch, but kept getting:
> > 
> > 29) caps_2.12.0(x86_64)
> > ... libvirt: QEMU Driver error : internal error: query-cpu-definitions
> > reply data was not an array
> > 
> > a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was
> > returning NULL for @data after/when the "query-sev-capabilities".
> > 
> > I narrowed it down into the virQEMUCapsInitQMPMonitor when run during
> > qemucapabilitiestest (see [1])
> 
> I have not tried latest libvirt yet, I will try this today and debug to 
> see what we are missing. I did the 'make check' before submitting the 
> patch but at that time QEMU 2.12 was not available and we did not had 
> updated caps_2.12.0.x86_64.xml and caps_2.12.0.x86_64.replies.

I thought the lack of churn in tests/qemucapabilitiesdata/ was
weird, but thanks to your explanation it makes perfect sense now.

Your code adds a call to query-sev-capabilities, but the replies
file doesn't contain the corresponding return data, which makes
the parser go out of sync.

You're going to have to either fetch capabilities from your own
QEMU 2.12 binary or hack it up by adding the return data in the
right spot and call tests/qemucapsfixreplies to re-align the ids.

I think you can get away with the latter, as we're going to want
to refresh the replies files once 2.12 is released anyway.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

On 03/26/2018 09:32 AM, Andrea Bolognani wrote:
> On Mon, 2018-03-26 at 08:52 -0500, Brijesh Singh wrote:
>>> I tried applying just this patch, but kept getting:
>>>
>>> 29) caps_2.12.0(x86_64)
>>> ... libvirt: QEMU Driver error : internal error: query-cpu-definitions
>>> reply data was not an array
>>>
>>> a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was
>>> returning NULL for @data after/when the "query-sev-capabilities".
>>>
>>> I narrowed it down into the virQEMUCapsInitQMPMonitor when run during
>>> qemucapabilitiestest (see [1])
>>
>> I have not tried latest libvirt yet, I will try this today and debug to
>> see what we are missing. I did the 'make check' before submitting the
>> patch but at that time QEMU 2.12 was not available and we did not had
>> updated caps_2.12.0.x86_64.xml and caps_2.12.0.x86_64.replies.
> 
> I thought the lack of churn in tests/qemucapabilitiesdata/ was
> weird, but thanks to your explanation it makes perfect sense now.
> 
> Your code adds a call to query-sev-capabilities, but the replies
> file doesn't contain the corresponding return data, which makes
> the parser go out of sync.
> 
> You're going to have to either fetch capabilities from your own
> QEMU 2.12 binary or hack it up by adding the return data in the
> right spot and call tests/qemucapsfixreplies to re-align the ids.
> 

Right, running the tests/qemucapsprobe on my system I see that 
query-sev-capabilities command returns a valid data. In my local branch, 
I updated the caps_2.12.0.x86_64.replies with response. With those 
changes I no longer get the internal error but 
tests/qemucapabilitiestest still fails for me with below error message:

29) caps_2.12.0(x86_64)                                               ...
  In 
'/home/amd/workdir/upstream/libvirt/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml':
  Offset 7143
  Expect [060]
  Actual [306]

It is basically pointing to microcode version change, do I need to 
update the cap with new version ?



> I think you can get away with the latter, as we're going to want
> to refresh the replies files once 2.12 is released anyway.
> 

I am not able to follow this comment, let me explain the situation.
The QEMU_CAPS_SEV flag was set to indicate QEMU supports the 
'query-sev-capabilities' QMP command and sev-guest object. That merely 
indicates that command exist but does not means that command will always 
execute successfully. e.g If hypervisor does not support the SEV feature 
then query-sev-capabilities will return error. That means if 
tests/qemucapsprobe is used to generate the replies on non-SEV capable 
system then .replies will not contain output of query-sev-capabilities 
command. Will this be an issue ?

-Brijesh

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

On Tue, 2018-03-27 at 09:26 -0500, Brijesh Singh wrote:
> > You're going to have to either fetch capabilities from your own
> > QEMU 2.12 binary or hack it up by adding the return data in the
> > right spot and call tests/qemucapsfixreplies to re-align the ids.
> 
> Right, running the tests/qemucapsprobe on my system I see that 
> query-sev-capabilities command returns a valid data. In my local branch, 
> I updated the caps_2.12.0.x86_64.replies with response. With those 
> changes I no longer get the internal error but 
> tests/qemucapabilitiestest still fails for me with below error message:
> 
> 29) caps_2.12.0(x86_64)                                               ...
>   In 
> '/home/amd/workdir/upstream/libvirt/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml':
>   Offset 7143
>   Expect [060]
>   Actual [306]
> 
> It is basically pointing to microcode version change, do I need to 
> update the cap with new version ?

After you've tweaked the .replies file, you can just run

  $ VIR_TEST_REGENERATE_OUTPUT=1 make check

and the .xml file will be refreshed, microcode version and all.

> > I think you can get away with the latter, as we're going to want
> > to refresh the replies files once 2.12 is released anyway.
> 
> I am not able to follow this comment, let me explain the situation.
> The QEMU_CAPS_SEV flag was set to indicate QEMU supports the 
> 'query-sev-capabilities' QMP command and sev-guest object. That merely 
> indicates that command exist but does not means that command will always 
> execute successfully. e.g If hypervisor does not support the SEV feature 
> then query-sev-capabilities will return error. That means if 
> tests/qemucapsprobe is used to generate the replies on non-SEV capable 
> system then .replies will not contain output of query-sev-capabilities 
> command. Will this be an issue ?

Not at all. That's already the case for a lot of features.

My comment was merely pointing out that the capabilities data we
have in the tree right now is for QEMU 2.12.0-rc0, and once QEMU
2.12 is officially released we will want to collect it again.
It's not something you need to worry about, really :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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