[libvirt][PATCH v9 1/5] Get SGX Capabilities from QEMU

Huang, Haibin haibin.huang at intel.com
Thu Jan 20 01:59:46 UTC 2022


Hi Michael,

Thank you for your comments.

> There's too much going on in this patch. You are querying qemu for SGX
> support and filling domain caps. At least the domain caps should go into the
> next patch.

Ok , we can put domain_capabilities.c in to the next patch, but virSGXCapability is define domain_capabilities.h, qemu_monitor_json.c will use it.
If we also put it to next patch, then this patch will not work.

So, I think we can just put domain_capabilities.c in to the next patch, ok?

> -----Original Message-----
> From: Michal Prívozník <mprivozn at redhat.com>
> Sent: Friday, January 7, 2022 11:06 PM
> To: Huang, Haibin <haibin.huang at intel.com>; libvir-list at redhat.com; Ding,
> Jian-feng <jian-feng.ding at intel.com>; Yang, Lin A <lin.a.yang at intel.com>; Lu,
> Lianhao <lianhao.lu at intel.com>; Zhong, Yang <yang.zhong at intel.com>
> Subject: Re: [libvirt][PATCH v9 1/5] Get SGX Capabilities from QEMU
> 
> On 12/15/21 04:40, Haibin Huang wrote:
> > The Qemu QMP provide the command "query-sgx-capabilities"
> > libvirt call the command to get sgx capabilities
> >
> > {"execute":"query-sgx-capabilities"}
> > {"return":
> >   {"sgx": true, "sgx1": true, "sgx2": false, "section-size": 0, \
> >    "flc": false}}
> >
> > Signed-off-by: Haibin Huang <haibin.huang at intel.com>
> > ---
> >  src/conf/domain_capabilities.c                |  10 ++
> >  src/conf/domain_capabilities.h                |  13 ++
> >  src/libvirt_private.syms                      |   1 +
> >  src/qemu/qemu_capabilities.c                  | 143 +++++++++++++++++-
> >  src/qemu/qemu_capabilities.h                  |   4 +
> >  src/qemu/qemu_monitor.c                       |  10 ++
> >  src/qemu/qemu_monitor.h                       |   3 +
> >  src/qemu/qemu_monitor_json.c                  |  83 ++++++++++
> >  src/qemu/qemu_monitor_json.h                  |   3 +
> >  .../caps_6.2.0.x86_64.replies                 |  22 ++-
> >  .../caps_6.2.0.x86_64.xml                     |   5 +
> >  11 files changed, 292 insertions(+), 5 deletions(-)
> 
> 
> There's too much going on in this patch. You are querying qemu for SGX
> support and filling domain caps. At least the domain caps should go into the
> next patch.

Ok , we can put domain_capabilities.c in to the next patch, but virSGXCapability is define domain_capabilities.h, qemu_monitor_json.c will use it.
If we also put it to next patch, then this patch will not work.
> 
> Secondly, you are using SEV functions as an placeholder. I mean, where you
> see SEV you put corresponding SGX function. There is nothing wrong with
> that, but either put pick a placement (after/before SEV code) and stick to it.
> 
> More comments below.

Ok , I have change it

> >
> > diff --git a/src/conf/domain_capabilities.c
> > b/src/conf/domain_capabilities.c index 22f0963326..d39be55f6a 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -78,6 +78,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap)  }
> >
> >
> > +void
> > +virSGXCapabilitiesFree(virSGXCapability *cap) {
> > +    if (!cap)
> > +        return;
> > +
> > +    VIR_FREE(cap);
> > +}
> > +
> > +
> >  static void
> >  virDomainCapsDispose(void *obj)
> >  {
> > diff --git a/src/conf/domain_capabilities.h
> > b/src/conf/domain_capabilities.h index d44acdcd01..b647ff8107 100644
> > --- a/src/conf/domain_capabilities.h
> > +++ b/src/conf/domain_capabilities.h
> > @@ -172,6 +172,13 @@ struct _virDomainCapsCPU {
> >      virDomainCapsCPUModels *custom;
> >  };
> >
> > +typedef struct _virSGXCapability virSGXCapability; typedef
> > +virSGXCapability *virSGXCapabilityPtr;
> 
> Even in 7.8.0 which you implemented your patches on top of had virXXXPtr
> typedefs dropped. Please do not introduce new ones.

Ok, I have changed it.

> 
> > +struct _virSGXCapability {
> > +    bool flc;
> > +    unsigned int epc_size;
> > +};
> > +
> >  typedef struct _virSEVCapability virSEVCapability;  struct
> > _virSEVCapability {
> >      char *pdh;
> > @@ -215,6 +222,7 @@ struct _virDomainCaps {
> >
> >      virDomainCapsFeatureGIC gic;
> >      virSEVCapability *sev;
> > +    virSGXCapability *sgx;
> >      /* add new domain features here */
> >
> >      virTristateBool features[VIR_DOMAIN_CAPS_FEATURE_LAST];
> > @@ -262,4 +270,9 @@ char * virDomainCapsFormat(const virDomainCaps
> > *caps);  void  virSEVCapabilitiesFree(virSEVCapability *capabilities);
> >
> > +void
> > +virSGXCapabilitiesFree(virSGXCapability *capabilities);
> > +
> >  G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSEVCapability,
> > virSEVCapabilitiesFree);
> > +
> > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSGXCapability,
> > +virSGXCapabilitiesFree);
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index
> > c5d788285e..d90d4ee6e1 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -219,6 +219,7 @@ virDomainCapsEnumSet;  virDomainCapsFormat;
> > virDomainCapsNew;  virSEVCapabilitiesFree;
> > +virSGXCapabilitiesFree;
> >
> >
> >  # conf/domain_conf.h
> > diff --git a/src/qemu/qemu_capabilities.c
> > b/src/qemu/qemu_capabilities.c index a607f5ea5f..8ce184ce35 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -651,6 +651,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> >                "chardev.json", /* QEMU_CAPS_CHARDEV_JSON */
> >                "device.json", /* QEMU_CAPS_DEVICE_JSON */
> >                "query-dirty-rate", /* QEMU_CAPS_QUERY_DIRTY_RATE */
> > +              "sgx-epc", /* QEMU_CAPS_SGX_EPC */
> >      );
> >
> >
> > @@ -731,11 +732,14 @@ struct _virQEMUCaps {
> >
> >      virSEVCapability *sevCapabilities;
> >
> > +    virSGXCapability *sgxCapabilities;
> > +
> >      /* Capabilities which may differ depending on the accelerator. */
> >      virQEMUCapsAccel kvm;
> >      virQEMUCapsAccel tcg;
> >  };
> >
> > +
> >  struct virQEMUCapsSearchData {
> >      virArch arch;
> >      const char *binaryFilter;
> > @@ -1367,6 +1371,7 @@ struct virQEMUCapsStringFlags
> virQEMUCapsObjectTypes[] = {
> >      { "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL },
> >      { "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
> >      { "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI },
> > +    { "sgx-epc", QEMU_CAPS_SGX_EPC },
> >  };
> >
> >
> > @@ -1918,6 +1923,22 @@ virQEMUCapsSEVInfoCopy(virSEVCapability
> **dst,
> > }
> >
> >
> > +static int
> > +virQEMUCapsSGXInfoCopy(virSGXCapabilityPtr *dst,
> > +                       virSGXCapabilityPtr src) {
> > +    g_autoptr(virSGXCapability) tmp = NULL;
> > +
> > +    tmp = g_new0(virSGXCapability, 1);
> > +
> > +    tmp->flc = src->flc;
> > +    tmp->epc_size = src->epc_size;
> > +
> > +    *dst = g_steal_pointer(&tmp);
> > +    return 0;
> 
> I know you followed the example of virQEMUCapsSEVInfoCopy() but both
> functions should be void. They never return anything else than 0.

Ok, change it.

> 
> > +}
> > +
> > +
> >  static void
> >  virQEMUCapsAccelCopyMachineTypes(virQEMUCapsAccel *dst,
> >                                   virQEMUCapsAccel *src) @@ -1997,6
> > +2018,11 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps
> *qemuCaps)
> >                                 qemuCaps->sevCapabilities) < 0)
> >          return NULL;
> >
> > +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC) &&
> > +        virQEMUCapsSGXInfoCopy(&ret->sgxCapabilities,
> > +                               qemuCaps->sgxCapabilities) < 0)
> > +        return NULL;
> > +
> >      return g_steal_pointer(&ret);
> >  }
> >
> > @@ -2033,6 +2059,7 @@ void virQEMUCapsDispose(void *obj)
> >      g_free(qemuCaps->gicCapabilities);
> >
> >      virSEVCapabilitiesFree(qemuCaps->sevCapabilities);
> > +    virSGXCapabilitiesFree(qemuCaps->sgxCapabilities);
> >
> >      virQEMUCapsAccelClear(&qemuCaps->kvm);
> >      virQEMUCapsAccelClear(&qemuCaps->tcg);
> > @@ -2553,6 +2580,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCaps
> > *qemuCaps)  }
> >
> >
> > +virSGXCapabilityPtr
> > +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps) {
> > +    return qemuCaps->sgxCapabilities; }
> > +
> > +
> >  static int
> >  virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps,
> >                              qemuMonitor *mon) @@ -3327,6 +3361,31 @@
> > virQEMUCapsProbeQMPSEVCapabilities(virQEMUCaps *qemuCaps,  }
> >
> >
> > +static int
> > +virQEMUCapsProbeQMPSGXCapabilities(virQEMUCaps *qemuCaps,
> > +                                   qemuMonitor *mon) {
> > +    int rc = -1;
> > +    virSGXCapability *caps = NULL;
> > +
> > +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC))
> > +        return 0;
> > +
> > +    if ((rc = qemuMonitorGetSGXCapabilities(mon, &caps)) < 0)
> > +        return -1;
> > +
> > +    /* SGX isn't actually supported */
> > +    if (rc == 0) {
> > +        virQEMUCapsClear(qemuCaps, QEMU_CAPS_SGX_EPC);
> > +        return 0;
> > +    }
> > +
> > +    virSGXCapabilitiesFree(qemuCaps->sgxCapabilities);
> > +    qemuCaps->sgxCapabilities = caps;
> > +    return 0;
> > +}
> > +
> > +
> >  /*
> >   * Filter for features which should never be passed to QEMU. Either
> because
> >   * QEMU never supported them or they were dropped as they never did
> > anything @@ -4110,6 +4169,41 @@
> virQEMUCapsParseSEVInfo(virQEMUCaps *qemuCaps, xmlXPathContextPtr
> ctxt)
> >      return 0;
> >  }
> >
> > +static int
> > +virQEMUCapsParseSGXInfo(virQEMUCaps *qemuCaps,
> xmlXPathContextPtr
> > +ctxt)
> 
> Here and everywhere else, please one argument per line.

Ok, changed it

> 
> > +{
> > +    g_autoptr(virSGXCapability) sgx = NULL;
> > +
> > +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC))
> > +        return 0;
> > +
> > +    if (virXPathBoolean("boolean(./sgx)", ctxt) == 0) {
> > +        virReportError(VIR_ERR_XML_ERROR, "%s",
> > +                       _("missing SGX platform data in QEMU "
> > +                       "capabilities cache"));
> 
> I know you wanted to fit under 80 characters, but error messages are
> extempt from the rule. In fact, we want error messages to be on one line
> because it's easier to git-grep them.
> 
> > +        return -1;
> > +    }
> > +
> > +    sgx = g_new0(virSGXCapability, 1);
> > +
> > +    if (virXPathBoolean("boolean(./sgx/flc)", ctxt) == 0) {
> > +        virReportError(VIR_ERR_XML_ERROR, "%s",
> > +                       _("missing SGX platform flc data in QEMU "
> > +                       "capabilities cache"));
> > +        return -1;
> > +    }
> > +
> > +    if (virXPathUInt("string(./sgx/epc_size)", ctxt, &sgx->epc_size) < 0) {
> > +        virReportError(VIR_ERR_XML_ERROR, "%s",
> > +                       _("missing or malformed SGX platform epc_size information "
> > +                       "in QEMU capabilities cache"));
> > +        return -1;
> > +    }
> > +
> > +    qemuCaps->sgxCapabilities = g_steal_pointer(&sgx);
> > +    return 0;
> > +}
> > +
> >
> >  /*
> >   * Parsing a doc that looks like
> > @@ -4226,7 +4320,7 @@ virQEMUCapsLoadCache(virArch hostArch,
> >          flag = virQEMUCapsTypeFromString(str);
> >          if (flag < 0) {
> >              virReportError(VIR_ERR_INTERNAL_ERROR,
> > -                           _("Unknown qemu capabilities flag %s"), str);
> > +                           _("Haibin Unknown qemu capabilities flag
> > + %s"), str);
> 
> Huh? Probably some leftover from debugging?

Sorry, miss it, change it.

> 
> >              goto cleanup;
> >          }
> >          VIR_FREE(str);
> > @@ -4351,6 +4445,9 @@ virQEMUCapsLoadCache(virArch hostArch,
> >      if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0)
> >          goto cleanup;
> >
> > +    if (virQEMUCapsParseSGXInfo(qemuCaps, ctxt) < 0)
> > +        goto cleanup;
> > +
> >      virQEMUCapsInitHostCPUModel(qemuCaps, hostArch,
> VIR_DOMAIN_VIRT_KVM);
> >      virQEMUCapsInitHostCPUModel(qemuCaps, hostArch,
> > VIR_DOMAIN_VIRT_QEMU);
> >
> > @@ -4531,6 +4628,19 @@ virQEMUCapsFormatSEVInfo(virQEMUCaps
> *qemuCaps, virBuffer *buf)
> >      virBufferAddLit(buf, "</sev>\n");  }
> >
> > +static void
> > +virQEMUCapsFormatSGXInfo(virQEMUCaps *qemuCaps, virBuffer *buf) {
> > +    virSGXCapabilityPtr sgx =
> > +virQEMUCapsGetSGXCapabilities(qemuCaps);
> > +
> > +    virBufferAddLit(buf, "<sgx>\n");
> > +    virBufferAdjustIndent(buf, 2);
> > +    virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no");
> > +    virBufferAsprintf(buf, "<epc_size>%u</epc_size>\n", sgx->epc_size);
> > +    virBufferAdjustIndent(buf, -2);
> > +    virBufferAddLit(buf, "</sgx>\n"); }
> > +
> >
> >  char *
> >  virQEMUCapsFormatCache(virQEMUCaps *qemuCaps) @@ -4605,6
> +4715,9 @@
> > virQEMUCapsFormatCache(virQEMUCaps *qemuCaps)
> >      if (qemuCaps->sevCapabilities)
> >          virQEMUCapsFormatSEVInfo(qemuCaps, &buf);
> >
> > +    if (qemuCaps->sgxCapabilities)
> > +        virQEMUCapsFormatSGXInfo(qemuCaps, &buf);
> > +
> >      if (qemuCaps->kvmSupportsNesting)
> >          virBufferAddLit(&buf, "<kvmSupportsNesting/>\n");
> >
> > @@ -5276,6 +5389,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCaps
> *qemuCaps,
> >          return -1;
> >      if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
> >          return -1;
> > +    if (virQEMUCapsProbeQMPSGXCapabilities(qemuCaps, mon) < 0)
> > +        return -1;
> >
> >      virQEMUCapsInitProcessCaps(qemuCaps);
> >
> > @@ -6248,6 +6363,31 @@
> virQEMUCapsFillDomainFeatureGICCaps(virQEMUCaps
> > *qemuCaps,  }
> >
> >
> > +/**
> > + * virQEMUCapsFillDomainFeatureiSGXCaps:
> > + * @qemuCaps: QEMU capabilities
> > + * @domCaps: domain capabilities
> > + *
> > + * Take the information about SGX capabilities that has been obtained
> > + * using the 'query-sgx-capabilities' QMP command and stored in
> > + at qemuCaps
> > + * and convert it to a form suitable for @domCaps.
> > + */
> > +static void
> > +virQEMUCapsFillDomainFeatureSGXCaps(virQEMUCaps *qemuCaps,
> > +                                    virDomainCaps *domCaps) {
> > +    virSGXCapability *cap = qemuCaps->sgxCapabilities;
> > +
> > +    if (!cap)
> > +        return;
> > +
> > +    domCaps->sgx = g_new0(virSGXCapability, 1);
> > +
> > +    domCaps->sgx->flc = cap->flc;
> > +    domCaps->sgx->epc_size = cap->epc_size; }
> > +
> > +
> >  /**
> >   * virQEMUCapsFillDomainFeatureSEVCaps:
> >   * @qemuCaps: QEMU capabilities
> > @@ -6339,6 +6479,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps
> *qemuCaps,
> >      virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps);
> >      virQEMUCapsFillDomainFeatureSEVCaps(qemuCaps, domCaps);
> >      virQEMUCapsFillDomainFeatureS390PVCaps(qemuCaps, domCaps);
> > +    virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
> >
> >      return 0;
> >  }
> > diff --git a/src/qemu/qemu_capabilities.h
> > b/src/qemu/qemu_capabilities.h index bb53d9ae46..e8c052b27d 100644
> > --- a/src/qemu/qemu_capabilities.h
> > +++ b/src/qemu/qemu_capabilities.h
> > @@ -631,6 +631,7 @@ typedef enum { /* virQEMUCapsFlags grouping
> marker for syntax-check */
> >      QEMU_CAPS_CHARDEV_JSON, /* -chardev accepts JSON */
> >      QEMU_CAPS_DEVICE_JSON, /* -device accepts JSON */
> >      QEMU_CAPS_QUERY_DIRTY_RATE, /* accepts query-dirty-rate */
> > +    QEMU_CAPS_SGX_EPC, /* -object sgx-epc,... */
> >
> >      QEMU_CAPS_LAST /* this must always be the last item */  }
> > virQEMUCapsFlags; @@ -824,5 +825,8 @@
> > virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps);  bool
> > virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps)
> > G_GNUC_NO_INLINE;
> >
> > +virSGXCapabilityPtr
> > +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps);
> > +
> >  virArch virQEMUCapsArchFromString(const char *arch);  const char
> > *virQEMUCapsArchToString(virArch arch); diff --git
> > a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index
> > e8accaf2b0..dca3e94ed2 100644
> > --- a/src/qemu/qemu_monitor.c
> > +++ b/src/qemu/qemu_monitor.c
> > @@ -3787,6 +3787,16 @@ qemuMonitorGetSEVCapabilities(qemuMonitor
> *mon,
> > }
> >
> >
> > +int
> > +qemuMonitorGetSGXCapabilities(qemuMonitor *mon,
> > +                              virSGXCapability **capabilities) {
> > +    QEMU_CHECK_MONITOR(mon);
> > +
> > +    return qemuMonitorJSONGetSGXCapabilities(mon, capabilities); }
> > +
> > +
> >  int
> >  qemuMonitorNBDServerStart(qemuMonitor *mon,
> >                            const virStorageNetHostDef *server, diff
> > --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index
> > 52ff34d316..e08fc1bfe3 100644
> > --- a/src/qemu/qemu_monitor.h
> > +++ b/src/qemu/qemu_monitor.h
> > @@ -915,6 +915,9 @@ int qemuMonitorGetGICCapabilities(qemuMonitor
> > *mon,  int qemuMonitorGetSEVCapabilities(qemuMonitor *mon,
> >                                    virSEVCapability **capabilities);
> >
> > +int qemuMonitorGetSGXCapabilities(qemuMonitor *mon,
> > +                                  virSGXCapability **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
> > 579d986e02..b2c5042a22 100644
> > --- a/src/qemu/qemu_monitor_json.c
> > +++ b/src/qemu/qemu_monitor_json.c
> > @@ -6723,6 +6723,89 @@
> qemuMonitorJSONGetGICCapabilities(qemuMonitor
> > *mon,  }
> >
> >
> > +/**
> > + * qemuMonitorJSONGetSGXCapabilities:
> > + * @mon: qemu monitor object
> > + * @capabilities: pointer to pointer to a SGX capability structure to
> > +be filled
> > + *
> > + * This function queries and fills in INTEL's SGX platform-specific data.
> > + * Note that from QEMU's POV both -object sgx-epc and
> > +query-sgx-capabilities
> > + * can be present even if SGX is not available, which basically
> > +leaves us with
> > + * checking for JSON "GenericError" in order to differentiate between
> > +compiled-in
> > + * support and actual SGX support on the platform.
> > + *
> > + * Returns -1 on error, 0 if SGX is not supported, and 1 if SGX is
> > +supported on
> > + * the platform.
> > + */
> > +int
> > +qemuMonitorJSONGetSGXCapabilities(qemuMonitor *mon,
> > +                                  virSGXCapability **capabilities) {
> > +    int ret = -1;
> > +    virJSONValue *cmd;
> > +    virJSONValue *reply = NULL;
> > +    virJSONValue *caps;
> 
> Here, both cmd and reply should be declared with g_autoptr(), like this:
> 
>     g_autoptr(virJSONValue) cmd = NULL;
>     g_autoptr(virJSONValue) reply = NULL;
> 
> That way they don't have to be freed explicitly and whole cleanup label can
> be dropped.
> 

Ok, I have changed it.

> 
> > +    bool sgx = false;
> > +    bool flc = false;
> > +    unsigned int section_size = 0;
> > +    g_autoptr(virSGXCapability) capability = NULL;
> > +
> > +    *capabilities = NULL;
> > +
> > +    if (!(cmd = qemuMonitorJSONMakeCommand("query-sgx-capabilities",
> NULL)))
> > +        return -1;
> > +
> > +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> > +        goto cleanup;
> > +
> > +    /* QEMU has only compiled-in support of SGX */
> > +    if (qemuMonitorJSONHasError(reply, "GenericError")) {
> > +        ret = 0;
> > +        goto cleanup;
> > +    }
> > +
> > +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> > +        goto cleanup;
> > +
> > +    caps = virJSONValueObjectGetObject(reply, "return");
> > +
> > +    if (virJSONValueObjectGetBoolean(caps, "sgx", &sgx) < 0) {
> 
> This boolean is never used. What's its purpose?

Ok, for if exist sgx,  I have deleted it because if epc size is not zero mean sgx exist. 

> 
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                       _("query-sgx reply was missing"
> > +                         " 'sgx' field"));
> > +        goto cleanup;
> > +    }
> > +
> > +    if (virJSONValueObjectGetBoolean(caps, "flc", &flc) < 0) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                       _("query-sgx-capabilities reply was missing"
> > +                         " 'flc' field"));
> > +        goto cleanup;
> > +    }
> > +
> > +    if (virJSONValueObjectGetNumberUint(caps, "section-size",
> &section_size) < 0) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                       _("query-sgx-capabilities reply was missing"
> > +                         " 'section-size' field"));
> > +        goto cleanup;
> > +    }
> > +
> > +    capability = g_new0(virSGXCapability, 1);
> > +
> > +    capability->flc = flc;
> > +
> > +    capability->epc_size = section_size/1024;
> > +    *capabilities = g_steal_pointer(&capability);
> > +    ret = 1;
> > +
> > + cleanup:
> > +    virJSONValueFree(cmd);
> > +    virJSONValueFree(reply);
> > +
> > +    return ret;
> > +}
> > +
> > +
> >  /**
> >   * qemuMonitorJSONGetSEVCapabilities:
> >   * @mon: qemu monitor object
> > diff --git a/src/qemu/qemu_monitor_json.h
> > b/src/qemu/qemu_monitor_json.h index c841de0a03..bfb405ed04 100644
> > --- a/src/qemu/qemu_monitor_json.h
> > +++ b/src/qemu/qemu_monitor_json.h
> > @@ -157,6 +157,9 @@ int
> qemuMonitorJSONGetGICCapabilities(qemuMonitor
> > *mon,  int qemuMonitorJSONGetSEVCapabilities(qemuMonitor *mon,
> >                                        virSEVCapability
> > **capabilities);
> >
> > +int qemuMonitorJSONGetSGXCapabilities(qemuMonitor *mon,
> > +                                      virSGXCapability
> > +**capabilities);
> > +
> >  int qemuMonitorJSONMigrate(qemuMonitor *mon,
> >                             unsigned int flags,
> >                             const char *uri); diff --git
> > a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> > b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> > index aa7a779a68..1092eb7c31 100644
> > --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> > +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> > @@ -29479,6 +29479,20 @@
> >    }
> >  }
> >
> > +{
> > +  "execute": "query-sgx-capabilities",
> > +  "id": "libvirt-49"
> > +}
> > +
> > +{
> > +  "id": "libvirt-49",
> > +  "return": {
> > +    "sgx": true,
> > +    "section-size": 1024,
> > +    "flc": false
> > +  }
> 
> I don't have SGX enabled machine, but I believe that 'id' is the last item in the
> reply. Also, according to QEMU's documentation
> (qapi/misc-target.json) returns more fields. Have you actually generated this
> reply using QEMU or was it handcrafted? Nothing wrong with that, but we
> should get as close to actual reply as possible.

Yes, this is actually reply, I will adjust the id to end.
> 
> > +}
> > +
> 
> Michal





More information about the libvir-list mailing list