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@xxxxxxxxx>
>>
>> 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>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>> 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
