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@intel.com>
---
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)
+{
+ 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) {
+ 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");
+ 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;
+ 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", §ion_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;
+}
+
static virJSONValue *
qemuMonitorJSONBuildInetSocketAddress(const char *host,
const char *port)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 64d9ebdaa3..82e6f29b7a 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -255,6 +255,15 @@ qemuMonitorJSONAddFileHandleToSet(qemuMonitor *mon,
const char *opaque,
qemuMonitorAddFdInfo *info);
+int qemuMonitorJSONGetSGXCapabilities(qemuMonitor *mon,
+ virSGXCapability **capabilities);
+
+int qemuMonitorJSONMigrate(qemuMonitor *mon,
+ unsigned int flags,
+ const char *uri);
+int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitor *mon,
+ bool *spice_migrated);
+
int
qemuMonitorJSONRemoveFdset(qemuMonitor *mon,
int fdset);
diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
index 8a574c893b..0719194e24 100644
--- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
@@ -32688,6 +32688,20 @@
}
}
+{
+ "execute": "query-sgx-capabilities",
+ "id": "libvirt-50"
+}
+
+{
+ "return": {
+ "sgx": true,
+ "section-size": 1024,
+ "flc": false
+ },
+ "id": "libvirt-50"
+}
+
{
"execute": "query-cpu-model-expansion",
"arguments": {
@@ -32696,7 +32710,7 @@
"name": "host"
}
},
- "id": "libvirt-50"
+ "id": "libvirt-51"
}
{
@@ -33029,7 +33043,7 @@
}
}
},
- "id": "libvirt-50"
+ "id": "libvirt-51"
}
{
@@ -33043,7 +33057,7 @@
}
}
},
- "id": "libvirt-51"
+ "id": "libvirt-52"
}
{
@@ -33376,7 +33390,7 @@
}
}
},
- "id": "libvirt-51"
+ "id": "libvirt-52"
}
{
diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
index 103d00fddd..1a4895fc42 100644
--- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
@@ -239,6 +239,7 @@
<flag name='rbd-encryption'/>
<flag name='sev-guest-kernel-hashes'/>
<flag name='sev-inject-launch-secret'/>
+ <flag name='sgx-epc'/>
<version>6002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100244</microcodeVersion>
@@ -3707,4 +3708,8 @@
<machine type='tcg' name='pc-q35-2.5' hotplugCpus='yes' maxCpus='255' defaultCPU='qemu64-x86_64-cpu' numaMemSupported='yes' defaultRAMid='pc.ram'/>
<machine type='tcg' name='pc-i440fx-3.0' hotplugCpus='yes' maxCpus='255' defaultCPU='qemu64-x86_64-cpu' numaMemSupported='yes' defaultRAMid='pc.ram'/>
<machine type='tcg' name='pc-q35-2.11' hotplugCpus='yes' maxCpus='288' defaultCPU='qemu64-x86_64-cpu' numaMemSupported='yes' defaultRAMid='pc.ram'/>
+ <sgx>
+ <flc>no</flc>
+ <epc_size>1</epc_size>
+ </sgx>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies
index 3d92303f79..6ed8968f04 100644
--- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies
@@ -32842,6 +32842,20 @@
}
}
+{
+ "execute": "query-sgx-capabilities",
+ "id": "libvirt-50"
+}
+
+{
+ "return": {
+ "sgx": true,
+ "section-size": 1024,
+ "flc": false
+ },
+ "id": "libvirt-50"
+}
+
{
"execute": "query-cpu-model-expansion",
"arguments": {
@@ -32850,7 +32864,7 @@
"name": "host"
}
},
- "id": "libvirt-50"
+ "id": "libvirt-51"
}
{
@@ -33183,7 +33197,7 @@
}
}
},
- "id": "libvirt-50"
+ "id": "libvirt-51"
}
{
@@ -33197,7 +33211,7 @@
}
}
},
- "id": "libvirt-51"
+ "id": "libvirt-52"
}
{
@@ -33530,7 +33544,7 @@
}
}
},
- "id": "libvirt-51"
+ "id": "libvirt-52"
}
{
diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml
index 07d49a89e1..5ef35c1e25 100644
--- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml
@@ -241,6 +241,7 @@
<flag name='sev-inject-launch-secret'/>
<flag name='device.json+hotplug'/>
<flag name='virtio-mem-pci.prealloc'/>
+ <flag name='sgx-epc'/>
<version>6002050</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100243</microcodeVersion>
@@ -3713,4 +3714,8 @@
<machine type='tcg' name='pc-q35-2.5' hotplugCpus='yes' maxCpus='255' defaultCPU='qemu64-x86_64-cpu' numaMemSupported='yes' defaultRAMid='pc.ram'/>
<machine type='tcg' name='pc-i440fx-3.0' hotplugCpus='yes' maxCpus='255' defaultCPU='qemu64-x86_64-cpu' numaMemSupported='yes' defaultRAMid='pc.ram'/>
<machine type='tcg' name='pc-q35-2.11' hotplugCpus='yes' maxCpus='288' defaultCPU='qemu64-x86_64-cpu' numaMemSupported='yes' defaultRAMid='pc.ram'/>
+ <sgx>
+ <flc>no</flc>
+ <epc_size>1</epc_size>
+ </sgx>
</qemuCaps>
--
2.17.1
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@intel.com>
> ---
> 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", §ion_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
I accept all comments. But I didn't your point for this comment .
> > +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.
[Haibin] sorry, I don't get your point, can you give me the details
Does you mean that change virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no"); to virBufferAsprintf(buf, "<flc>%u</flc>\n", sgx->flc);
If yes, but I see the cpu->maximum is also like this.
virBufferAsprintf(buf, "<mode name='%s' supported='%s'",
virCPUModeTypeToString(VIR_CPU_MODE_MAXIMUM),
cpu->maximum ? "yes" : "no");
> -----Original Message-----
> From: Michal Prívozník <mprivozn@redhat.com>
> Sent: Wednesday, February 16, 2022 6:26 PM
> To: Huang, Haibin <haibin.huang@intel.com>; libvir-list@redhat.com;
> berrange@redhat.com; Ding, Jian-feng <jian-feng.ding@intel.com>; Yang,
> Lin A <lin.a.yang@intel.com>; Lu, Lianhao <lianhao.lu@intel.com>
> Subject: Re: [libvirt][PATCH RESEND v10 1/5] qemu: provide support to query
> the SGX capability
>
> 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@intel.com>
> > ---
> > 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.
[Haibin] Ok, I will fix 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"));
> > + 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, ...
[Haibin] ok, I will check sgx
>
> > + 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.
[Haibin] sorry, I don't get your point, can you give me the details
>
> > + 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",
> §ion_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.
[Haibin] ok
>
> > +}
> > +
>
> Michal
On 2/23/22 04:41, Huang, Haibin wrote:
> I accept all comments. But I didn't your point for this comment .
>>> +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.
> [Haibin] sorry, I don't get your point, can you give me the details
>
> Does you mean that change virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no"); to virBufferAsprintf(buf, "<flc>%u</flc>\n", sgx->flc);
No. You can keep this particual virBufferAsprintf() as is. What I have
problem with is the parsing side. So if this virQEMUCapsFormatSGXInfo()
function is called, if produces the following XML:
<sgx>
<flc>no</flc>
<epc_size>1024</epc_size>
</sgx>
(I've made the values up)
This corresponds to: sgx->flc = false; sgx->epc_size = 1024;
But when the XML is parsed, in virQEMUCapsParseSGXInfo(), it would se
the following values:
sgx->flc = true; sgx->epc_size = 2014;
Notice the change in ->flc? This is because
virXPathBoolean("boolean(./sgx/flc)") does not evaluate the value of
<flc/> element, but only whether the xpath exists. Therefore, even if
flc's element value is "no", it does exist and as such is evaluated to true.
Hope this cleans things up.
Michal
I understand and thank you very much for your meticulous explanation.
> -----Original Message-----
> From: Michal Prívozník <mprivozn@redhat.com>
> Sent: Wednesday, February 23, 2022 8:16 PM
> To: Huang, Haibin <haibin.huang@intel.com>; libvir-list@redhat.com;
> berrange@redhat.com; Ding, Jian-feng <jian-feng.ding@intel.com>; Yang, Lin
> A <lin.a.yang@intel.com>; Lu, Lianhao <lianhao.lu@intel.com>
> Subject: Re: [libvirt][PATCH RESEND v10 1/5] qemu: provide support to query
> the SGX capability
>
> On 2/23/22 04:41, Huang, Haibin wrote:
> > I accept all comments. But I didn't your point for this comment .
> >>> +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.
> > [Haibin] sorry, I don't get your point, can you give me the details
> >
> > Does you mean that change virBufferAsprintf(buf, "<flc>%s</flc>\n",
> > sgx->flc ? "yes" : "no"); to virBufferAsprintf(buf, "<flc>%u</flc>\n",
> > sgx->flc);
>
> No. You can keep this particual virBufferAsprintf() as is. What I have problem
> with is the parsing side. So if this virQEMUCapsFormatSGXInfo() function is
> called, if produces the following XML:
>
> <sgx>
> <flc>no</flc>
> <epc_size>1024</epc_size>
> </sgx>
>
> (I've made the values up)
>
> This corresponds to: sgx->flc = false; sgx->epc_size = 1024;
>
> But when the XML is parsed, in virQEMUCapsParseSGXInfo(), it would se the
> following values:
>
> sgx->flc = true; sgx->epc_size = 2014;
>
> Notice the change in ->flc? This is because
> virXPathBoolean("boolean(./sgx/flc)") does not evaluate the value of <flc/>
> element, but only whether the xpath exists. Therefore, even if flc's element
> value is "no", it does exist and as such is evaluated to true.
>
> Hope this cleans things up.
>
> Michal
© 2016 - 2026 Red Hat, Inc.