9:05 a.m.
On 7/28/22 10:15, Peter Krempa wrote:
On Wed, Jul 27, 2022 at 12:34:54 +0200, Michal Privoznik wrote:
> From: Haibin Huang <haibin.huang(a)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(a)intel.com>
> Signed-off-by: Michal Privoznik <mprivozn(a)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",
§ion_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.
The idea is that this allows us to work with qemu-6.2.0 and qemu-7.0.0;
The former reports section-size only, the latter marked it obsolete and
reports array of 'sections' so that sections per NUMA node can be
reported. Now, section-size is nothing but a sum of individual per NUMA
node sections. So I guess we can do the summation once QEMU stops
reporting it.
NB, presence of per NUMA node sections (this code below) is then used
when generating cmd line, because qemu-7.0.0 requires slightly different
cmd line (due to those NUMA nodes).
Alternatively, we may pronounce qemu-6.2.0 not worth supporting and aim
on 7.0.0 only and not deal with deprecated interface at all (i.e. don't
parse/report aggregated sum).
> +
> + 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)
Michal