On 5/18/22 09:59, Haibin Huang wrote:
> Generate the QMP command for query-sgx-capabilities and the command
> return sgx capabilities from QMP.
>
> {"execute":"query-sgx-capabilities"}
>
> the right reply:
> {"return":
> {
> "sgx": true,
> "section-size": 197132288,
> "flc": true
> }
> }
>
> the error reply:
> {"error":
> {"class": "GenericError", "desc": "SGX is not enabled in KVM"}
> }
>
> Signed-off-by: Haibin Huang <haibin.huang@xxxxxxxxx>
> ---
> src/qemu/qemu_monitor.c | 10 ++++
> src/qemu/qemu_monitor.h | 3 +
> src/qemu/qemu_monitor_json.c | 104 ++++++++++++++++++++++++++++++++---
> src/qemu/qemu_monitor_json.h | 9 +++
> 4 files changed, 119 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index d44c7f0c60..6b82e8c853 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3648,6 +3648,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 b1484fdff8..ed87185e5d 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -900,6 +900,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 a092bf420f..38c3d018f3 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6433,6 +6433,69 @@ 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)
> +{
> + 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)
> + return -1;
> +
> + /* QEMU has only compiled-in support of SGX */
> + if (qemuMonitorJSONHasError(reply, "GenericError"))
> + return 0;
> +
> + if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> + return -1;
> +
> + caps = virJSONValueObjectGetObject(reply, "return");
> +
> + if (virJSONValueObjectGetBoolean(caps, "flc", &flc) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-sgx-capabilities reply was missing 'flc' field"));
> + return -1;
> + }
> +
> + if (virJSONValueObjectGetNumberUint(caps, "section-size", §ion_size) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-sgx-capabilities reply was missing 'section-size' field"));
> + return -1;
> + }
> +
> + capability = g_new0(virSGXCapability, 1);
> + capability->flc = flc;
> + capability->epc_size = section_size/1024;
> +
> + *capabilities = g_steal_pointer(&capability);
> + return 1;
> +}
> +
> static virJSONValue *
> qemuMonitorJSONBuildInetSocketAddress(const char *host,
> const char *port)
> @@ -7469,13 +7532,25 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor *mon,
> return -1;
> }
>
> - /* While 'id' attribute is marked as optional in QEMU's QAPI
> - * specification, Libvirt always sets it. Thus we can fail if not
> - * present. */
> - if (!(devalias = virJSONValueObjectGetString(dimminfo, "id"))) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("dimm memory info data is missing 'id'"));
> - return -1;
> + if (STREQ(type, "dimm") || STREQ(type, "nvdimm") || STREQ(type, "virtio-mem")) {
> + /* While 'id' attribute is marked as optional in QEMU's QAPI
> + * specification, Libvirt always sets it. Thus we can fail if not
> + * present. */
> + if (!(devalias = virJSONValueObjectGetString(dimminfo, "id"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("dimm memory info data is missing 'id'"));
> + return -1;
> + }
> + } else if (STREQ(type, "sgx-epc")) {
> + if (!(devalias = virJSONValueObjectGetString(dimminfo, "memdev"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("sgx-epc memory info data is missing 'memdev'"));
> + return -1;
> + }
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("%s memory device info is not handled yet"), type);
> + return -1;
> }
This hunk ^^ ...
>
> meminfo = g_new0(qemuMonitorMemoryDeviceInfo, 1);
> @@ -7519,6 +7594,21 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor *mon,
> _("malformed/missing size in virtio memory info"));
> return -1;
> }
> + } else if (STREQ(type, "sgx-epc")) {
> + /* sgx-epc memory devices */
> + if (virJSONValueObjectGetNumberUlong(dimminfo, "memaddr",
> + &meminfo->address) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("malformed/missing memaddr in sgx-epc memory info"));
> + return -1;
> + }
> +
> + if (virJSONValueObjectGetNumberUlong(dimminfo, "size",
> + &meminfo->size) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("malformed/missing size in sgx-epc memory info"));
> + return -1;
> + }
.. and this hunk ^^ don't belong into this patch. They belong to the
very last patch where libvirt is able to start a guest with sgx-epc
memdev. In other words: these two hunks are conceptually different to
the rest of the patch. It just so happens that they live in the same file.
Since we've been struggling with proper split of the code into patches I
worry that maybe I'm not expressing my thoughts comprehensibly. Or maybe
it's lost in translation. I don't know. I've tried to fix your patches
and they're available here:
https://gitlab.com/MichalPrivoznik/libvirt/-/commits/sgx_fixups
Pleaso do take a look, ideally rather sooner than later - I was pinged
by a colleague of yours couple of months after I've done this the first
time - by that time I was tired of rebasing that branch constantly and
just deleted it. This branch is not going to stay there forever either.
> } else {
> /* type not handled yet */
> continue;
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index 3c442d669f..dbe772c3f7 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -255,6 +255,15 @@ qemuMonitorJSONAddFileHandleToSet(qemuMonitor *mon,
> int fdset,
> const char *opaque);
>
> +int qemuMonitorJSONGetSGXCapabilities(qemuMonitor *mon,
> + virSGXCapability **capabilities);
> +
> +int qemuMonitorJSONMigrate(qemuMonitor *mon,
> + unsigned int flags,
> + const char *uri);
> +int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitor *mon,
> + bool *spice_migrated);
> +
This is probably a leftover from bad conflict resolution. You are not
introducing either of qemuMonitorJSONMigrate()
qemuMonitorJSONGetSpiceMigrationStatus() and they are already declared.
No need to declare them again.
Michal
