[PATCH v14 08/15] qemu: Get SGX capabilities form QMP

[Peter Krempa]

On Wed, Jul 27, 2022 at 12:34:54 +0200, Michal Privoznik wrote:
> From: Haibin Huang <haibin.huang at intel.com>
> 
> 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 at intel.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_monitor.c      |  10 ++++
>  src/qemu/qemu_monitor.h      |   3 +
>  src/qemu/qemu_monitor_json.c | 107 +++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |   4 ++
>  4 files changed, 124 insertions(+)

[...]

> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 941596563a..b045efa203 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6395,6 +6395,113 @@ 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;
> +    g_autoptr(virSGXCapability) capability = NULL;
> +    virJSONValue *sections = NULL;
> +    virJSONValue *caps;
> +    bool flc = false;
> +    bool sgx1 = false;
> +    bool sgx2 = false;

These temporary booleans feel a bit redundant ...

> +    unsigned long long section_size = 0;
> +    unsigned long long size;
> +    size_t i;
> +
> +    *capabilities = NULL;
> +    capability = g_new0(virSGXCapability, 1);
> +
> +    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;
> +    }
> +    capability->flc = flc;

Because you assign the value directly back to the struct. Passing the
pointer to the field in the struct directly to
virJSONValueObjectGetBoolean avoids the need.

> +
> +    if (virJSONValueObjectGetBoolean(caps, "sgx1", &sgx1) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-sgx-capabilities reply was missing 'sgx1' field"));
> +        return -1;
> +    }
> +    capability->sgx1 = sgx1;
> +
> +    if (virJSONValueObjectGetBoolean(caps, "sgx2", &sgx2) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-sgx-capabilities reply was missing 'sgx2' field"));
> +        return -1;
> +    }
> +    capability->sgx2 = sgx2;
> +
> +    if (virJSONValueObjectGetNumberUlong(caps, "section-size", &section_size) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-sgx-capabilities reply was missing 'section-size' field"));
> +        return -1;
> +    }
> +    capability->section_size = section_size / 1024;

The 'section-size' field is marked as deprecated in the QMP schema. Thus
we must not report error if it vanishes.

Is there any reason to extract it in the first place?

If yes, the code must be fixed to handle the possibility properly.

> +
> +    if ((sections = virJSONValueObjectGetArray(caps, "sections"))) {
> +        /* Got the section, the QEMU version is above 7.0.0 */
> +        capability->nsections = virJSONValueArraySize(sections);
> +        capability->sections = g_new0(virSection, capability->nsections);
> +
> +        for (i = 0; i < capability->nsections; i++) {
> +            virJSONValue *elem = virJSONValueArrayGet(sections, i);
> +
> +            if (virJSONValueObjectGetNumberUlong(elem, "size", &size) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("query-sgx-capabilities reply was missing 'size' field"));
> +                return -1;
> +            }
> +            capability->sections[i].size = size / 1024;
> +
> +            if (virJSONValueObjectGetNumberInt(elem, "node",
> +                                               &capability->sections[i].node) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("query-sgx-capabilities reply was missing 'node' field"));
> +                return -1;
> +            }
> +        }
> +    }
> +
> +    *capabilities = g_steal_pointer(&capability);
> +    return 1;
> +}
> +
> +
>  static virJSONValue *
>  qemuMonitorJSONBuildInetSocketAddress(const char *host,
>                                        const char *port)