Re: [PATCH RESEND v10 1/5] qemu: provide support to query the SGX capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/8/22 06:21, Haibin Huang wrote:
> QEMU version >= 6.2.0 provides support for creating enclave on
> SGX x86 platform using Software Guard Extensions (SGX) feature.
> This patch adds support to query the SGX capability from the qemu.
> 
> Signed-off-by: Haibin Huang <haibin.huang@xxxxxxxxx>
> ---
>  src/conf/domain_capabilities.c                |  10 ++
>  src/conf/domain_capabilities.h                |  13 ++
>  src/libvirt_private.syms                      |   1 +
>  src/qemu/qemu_capabilities.c                  | 113 ++++++++++++++++++
>  src/qemu/qemu_capabilities.h                  |   4 +
>  src/qemu/qemu_capspriv.h                      |   4 +
>  src/qemu/qemu_monitor.c                       |  10 ++
>  src/qemu/qemu_monitor.h                       |   3 +
>  src/qemu/qemu_monitor_json.c                  |  72 +++++++++++
>  src/qemu/qemu_monitor_json.h                  |   9 ++
>  .../caps_6.2.0.x86_64.replies                 |  22 +++-
>  .../caps_6.2.0.x86_64.xml                     |   5 +
>  .../caps_7.0.0.x86_64.replies                 |  22 +++-
>  .../caps_7.0.0.x86_64.xml                     |   5 +
>  14 files changed, 285 insertions(+), 8 deletions(-)
> 
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index c394a7a390..1170fd26df 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 1d2f4ac7a5..973d543ce8 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -191,6 +191,13 @@ struct _virSEVCapability {
>      unsigned int max_es_guests;
>  };
>  
> +typedef struct _virSGXCapability virSGXCapability;
> +typedef virSGXCapability *virSGXCapabilityPtr;
> +struct _virSGXCapability {
> +    bool flc;
> +    unsigned int epc_size;
> +};
> +
>  typedef enum {
>      VIR_DOMAIN_CAPS_FEATURE_IOTHREADS = 0,
>      VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO,
> @@ -227,6 +234,7 @@ struct _virDomainCaps {
>  
>      virDomainCapsFeatureGIC gic;
>      virSEVCapability *sev;
> +    virSGXCapability *sgx;
>      /* add new domain features here */
>  
>      virTristateBool features[VIR_DOMAIN_CAPS_FEATURE_LAST];
> @@ -275,3 +283,8 @@ void
>  virSEVCapabilitiesFree(virSEVCapability *capabilities);
>  
>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSEVCapability, virSEVCapabilitiesFree);
> +
> +void
> +virSGXCapabilitiesFree(virSGXCapability *capabilities);
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSGXCapability, virSGXCapabilitiesFree);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ba3462d849..0e74e4f20e 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 1b28c3f161..0e43dd2466 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -663,6 +663,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
>                "device.json+hotplug", /* QEMU_CAPS_DEVICE_JSON */
>                "hvf", /* QEMU_CAPS_HVF */
>                "virtio-mem-pci.prealloc", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_PREALLOC */
> +              "sgx-epc", /* QEMU_CAPS_SGX_EPC */
>      );
>  
>  
> @@ -744,6 +745,8 @@ struct _virQEMUCaps {
>  
>      virSEVCapability *sevCapabilities;
>  
> +    virSGXCapability *sgxCapabilities;
> +
>      /* Capabilities which may differ depending on the accelerator. */
>      virQEMUCapsAccel kvm;
>      virQEMUCapsAccel hvf;
> @@ -1398,6 +1401,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 },
>  };
>  
>  
> @@ -1962,6 +1966,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;
> +}
> +
> +
>  static void
>  virQEMUCapsAccelCopyMachineTypes(virQEMUCapsAccel *dst,
>                                   virQEMUCapsAccel *src)
> @@ -2043,6 +2063,12 @@ 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);
>  }
>  
> @@ -2606,6 +2632,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps)
>  }
>  
>  
> +virSGXCapabilityPtr
> +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps)
> +{
> +    return qemuCaps->sgxCapabilities;
> +}
> +
> +
>  static int
>  virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps,
>                              qemuMonitor *mon)
> @@ -3441,6 +3474,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
> @@ -4219,6 +4277,39 @@ virQEMUCapsParseSEVInfo(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt)
>  }
>  
>  
> +static int
> +virQEMUCapsParseSGXInfo(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt)

We like one argument per line.

> +{
> +    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"));
> +        return -1;
> +    }
> +
> +    sgx = g_new0(virSGXCapability, 1);
> +
> +    if (virXPathBoolean("boolean(./sgx/flc)", ctxt) == 0) {

This only checks whether <flc/> element exists and not its value, ...

> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("missing SGX platform flc 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 in QEMU capabilities cache"));
> +        return -1;
> +    }
> +
> +    qemuCaps->sgxCapabilities = g_steal_pointer(&sgx);
> +    return 0;
> +}
> +
> +
>  static int
>  virQEMUCapsParseFlags(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt)
>  {
> @@ -4521,6 +4612,9 @@ virQEMUCapsLoadCache(virArch hostArch,
>      if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0)
>          return -1;
>  
> +    if (virQEMUCapsParseSGXInfo(qemuCaps, ctxt) < 0)
> +        return -1;
> +
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
>          virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
> @@ -4701,6 +4795,20 @@ virQEMUCapsFormatSEVInfo(virQEMUCaps *qemuCaps, virBuffer *buf)
>  }
>  
>  
> +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");

... which is in contradiction with the way we format it.

> +    virBufferAsprintf(buf, "<epc_size>%u</epc_size>\n", sgx->epc_size);
> +    virBufferAdjustIndent(buf, -2);
> +    virBufferAddLit(buf, "</sgx>\n");
> +}
> +
> +
>  char *
>  virQEMUCapsFormatCache(virQEMUCaps *qemuCaps)
>  {
> @@ -4782,6 +4890,9 @@ virQEMUCapsFormatCache(virQEMUCaps *qemuCaps)
>      if (qemuCaps->sevCapabilities)
>          virQEMUCapsFormatSEVInfo(qemuCaps, &buf);
>  
> +    if (qemuCaps->sgxCapabilities)
> +        virQEMUCapsFormatSGXInfo(qemuCaps, &buf);
> +
>      if (qemuCaps->kvmSupportsNesting)
>          virBufferAddLit(&buf, "<kvmSupportsNesting/>\n");
>  
> @@ -5466,6 +5577,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps,
>          return -1;
>      if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
>          return -1;
> +    if (virQEMUCapsProbeQMPSGXCapabilities(qemuCaps, mon) < 0)
> +        return -1;
>  
>      virQEMUCapsInitProcessCaps(qemuCaps);
>  
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 6ff0b7a78b..a630676d6c 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -638,6 +638,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
>      QEMU_CAPS_DEVICE_JSON, /* -device accepts JSON (and works with hot-unplug) */
>      QEMU_CAPS_HVF, /* Whether Hypervisor.framework is available */
>      QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_PREALLOC, /* -device virtio-mem-pci.prealloc= */
> +    QEMU_CAPS_SGX_EPC, /* -object sgx-epc,... */
>  
>      QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> @@ -831,6 +832,9 @@ virQEMUCapsCPUFeatureFromQEMU(virQEMUCaps *qemuCaps,
>  virSEVCapability *
>  virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps);
>  
> +virSGXCapabilityPtr
> +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps);
> +
>  bool
>  virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) G_GNUC_NO_INLINE;
>  
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> index f4f4a99d32..c632647a74 100644
> --- a/src/qemu/qemu_capspriv.h
> +++ b/src/qemu/qemu_capspriv.h
> @@ -101,6 +101,10 @@ void
>  virQEMUCapsSetSEVCapabilities(virQEMUCaps *qemuCaps,
>                                virSEVCapability *capabilities);
>  
> +void
> +virQEMUCapsSetSGXCapabilities(virQEMUCaps *qemuCaps,
> +                              virSGXCapability *capabilities);
> +
>  int
>  virQEMUCapsProbeCPUDefinitionsTest(virQEMUCaps *qemuCaps,
>                                     qemuMonitor *mon);
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index babf9e62fb..c54cca2406 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3719,6 +3719,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 9b2e4e1421..4e85dcb548 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -920,6 +920,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 b0b513683b..811db233c4 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6493,6 +6493,78 @@ qemuMonitorJSONGetSEVCapabilities(qemuMonitor *mon,
>      return 1;
>  }
>  
> +/**
> + * 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;

This variable is pretty useles, because ...

> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) reply = NULL;
> +    virJSONValue *caps;
> +    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, "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:
> +
> +    return ret;


... this label is pretty much useless. s/goto cleanup/return -1/ for the
whole function, except for that one case where return 0 is needed.

> +}
> +

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux