On Wed, Sep 08, 2021 at 09:15:57AM +0800, Haibin Huang wrote:
1.Add SGX feature in domain capabilities
2.Get sgx capabilities by query-sgx-capabilities
I think we'd generally prefer it if the domain capabilities additions
are separate from the QEMU Driver capabilities probe.
So I think I'd suggest you move the probing of QEMU capabilities
into a separate patch - this probing will also need to update
the QEMU capabilities tests at the same time - we need bisectability
such that
git rebase -i master -x 'ninja -C build test'
will succeeed at each step.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 090ac80691..7ae4d52e3f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -218,6 +218,7 @@ virDomainCapsEnumSet;
virDomainCapsFormat;
virDomainCapsNew;
virSEVCapabilitiesFree;
+virSGXCapabilitiesFree;
# conf/domain_conf.h
@@ -1843,7 +1844,6 @@ virBitmapToDataBuf;
virBitmapToString;
virBitmapUnion;
-
Stay line removal crept in here by mistake
# util/virbpf.h
virBPFAttachProg;
virBPFCreateMap;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f6f4ee28fb..ccc0a4392d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -637,6 +637,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"confidential-guest-support", /*
QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT */
"query-display-options", /* QEMU_CAPS_QUERY_DISPLAY_OPTIONS */
"s390-pv-guest", /* QEMU_CAPS_S390_PV_GUEST */
+ "sgx-epc", /* QEMU_CAPS_SGX_EPC */
);
@@ -717,11 +718,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;
@@ -1357,6 +1361,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "virtio-gpu-gl-pci", QEMU_CAPS_VIRTIO_GPU_GL_PCI },
{ "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL },
{ "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
+ { "sgx-epc", QEMU_CAPS_SGX_EPC },
};
@@ -1917,6 +1922,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)
@@ -1996,6 +2017,11 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps)
qemuCaps->sevCapabilities) < 0)
goto error;
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC) &&
+ virQEMUCapsSGXInfoCopy(&ret->sgxCapabilities,
+ qemuCaps->sgxCapabilities) < 0)
+ goto error;
+
return ret;
error:
@@ -2036,6 +2062,7 @@ void virQEMUCapsDispose(void *obj)
g_free(qemuCaps->gicCapabilities);
virSEVCapabilitiesFree(qemuCaps->sevCapabilities);
+ virSGXCapabilitiesFree(qemuCaps->sgxCapabilities);
virQEMUCapsAccelClear(&qemuCaps->kvm);
virQEMUCapsAccelClear(&qemuCaps->tcg);
@@ -2556,6 +2583,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps)
}
+virSGXCapabilityPtr
+virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps)
+{
+ return qemuCaps->sgxCapabilities;
+}
+
+
static int
virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps,
qemuMonitor *mon)
@@ -2582,6 +2616,7 @@ virQEMUCapsProbeQMPObjectTypes(virQEMUCaps *qemuCaps,
if (qemuMonitorGetObjectTypes(mon, &values) < 0)
return -1;
+
Another stray whitespace change
virQEMUCapsProcessStringFlags(qemuCaps,
G_N_ELEMENTS(virQEMUCapsObjectTypes),
virQEMUCapsObjectTypes,
diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
index 8e5af9f79a..213fc05dc9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -44,6 +44,7 @@
# include "libvirt_qemu_probes.h"
#endif
+#define KB 1024
This feels rather uncessary to me, and it has way too generic a
name in any case. Just inline the value since its only used
once.
#define VIR_FROM_THIS VIR_FROM_QEMU
VIR_LOG_INIT("qemu.qemu_monitor_json");
@@ -6862,6 +6863,94 @@ 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;
+ 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) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-sgx reply was missing"
+ " 'sgx' field"));
+ goto cleanup;
+ }
+ if (!sgx) {
+ VIR_WARN("sgx is not support %d\n", sgx);
Is it really possible that query-sgx-capabilities will return
without the 'sgx' field set ? I would have thought that since
you already called qemuMonitorJSONCheckError, that 'sgx' should
thus always be present at this point, and this can be a fatal
error report, not a warning ?
Essentially VIR_WARN is almost never something we wnat todo.
Either its an expected scenario in which case VIR_DEBUG is
the most we should do, or its a fatal problem in which
case virReportError() and return -1.
+ ret = 0;
+ 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",
§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/(KB);
+ *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 fbeab2bf6d..057b62833f 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);
--
2.17.1
Regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|