On 7/1/22 21:14, Lin Yang wrote:

From: Haibin Huang <haibin.huang@intel.com>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Haibin Huang <haibin.huang@intel.com> --- src/conf/domain_capabilities.c | 10 ++++++++++ src/conf/domain_capabilities.h | 24 ++++++++++++++++++++++++ src/libvirt_private.syms | 1 + 3 files changed, 35 insertions(+)

diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 895e8d00e8..27f3fb8d36 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -76,6 +76,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap) }

+void +virSGXCapabilitiesFree(virSGXCapability *cap) +{ + if (!cap) + return; +

This leaks cap->pSections.

+ g_free(cap); +} + + static void virDomainCapsDispose(void *obj) { diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index f2eed80b15..dac1098e98 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -192,6 +192,24 @@ struct _virSEVCapability { unsigned int max_es_guests; };

+typedef struct _virSection virSection; +typedef virSection *virSectionPtr;

No, if we can help it I'd rather avoid introducing virXXXPtr typedef. We've worked hard to get rid of them and I don't quite see a reason to reintroduce them.

+struct _virSection { + unsigned long long size; + unsigned int node;

While it's true that QEMU with its current code does not ever report a negative number for 'node', at the QAPI level it's declared as signed integer and thus we ought to reflect that.

+}; + +typedef struct _virSGXCapability virSGXCapability; +typedef virSGXCapability *virSGXCapabilityPtr; +struct _virSGXCapability { + bool flc; + bool sgx1; + bool sgx2; + unsigned long long section_size; + size_t nSections; + virSectionPtr pSections;

We tend to use: 'nitems' and 'items', or 'nsections' and 'sections' in cases like this.

+}; +

Michal

Nice comments, I accepted all comments.

-----Original Message----- From: Michal Prívozník <mprivozn@redhat.com> Sent: Wednesday, July 20, 2022 7:27 PM To: Yang, Lin A <lin.a.yang@intel.com>; libvir-list@redhat.com; Huang, Haibin <haibin.huang@intel.com>; Ding, Jian-feng <jian-feng.ding@intel.com> Subject: Re: [libvirt][PATCH v13 2/6] Get SGX capabilities form QMP

On 7/1/22 21:14, Lin Yang wrote:

...

From: Haibin Huang <haibin.huang@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: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Haibin Huang <haibin.huang@intel.com> --- src/qemu/qemu_monitor.c | 10 ++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 113 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++ 4 files changed, 130 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fda5d2f368..a1b2138921 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3653,6 +3653,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 95267ec6c7..451ac8eec9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -864,6 +864,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 3aad2ab212..c900956f82 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6445,6 +6445,119 @@ 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; + 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; + + 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;

Spaces around the operand please. Ok, I will modify it.

...

+ + if (!(sections = virJSONValueObjectGetArray(caps, "sections"))) { + capability->nSections = 0; + capability->pSections = NULL;

This is needless. g_new0() used above made sure that the memory is zeroed out upon allocation, so these two lines are NOP.

...

+ VIR_INFO("Sections was not obtained, so QEMU version is + 6.2.0");

VIR_INFO() is probably too harsh. It's not anything that users should be concerned about. VIR_DEBUG() is much better. But then again, when debug logs are enabled then the QMP command and its reply are seen in the log and thus anybody reading it sees immediately whether a part is missing or not. [Haibin] OK

...

+ *capabilities = g_steal_pointer(&capability); + return 1; + } + + // Got the section, the QEMU version is above 7.0.0 + capability->nSections = virJSONValueArraySize(sections); + capability->pSections = g_new0(virSection, capability->nSections + + 1);

This + 1 looks wrong to me. We already have nSections, and pSections is not an array of pointers either. So NULL terminated array is a bit overkill.

[Haibin] OK

...

+ + 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->pSections[i].size = size/1024; + + if (virJSONValueObjectGetNumberUint(elem, "node", &(capability- pSections[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; +}

Michal

On 7/1/22 21:14, Lin Yang wrote:

From: Haibin Huang <haibin.huang@intel.com>

the QMP capabilities: {"return": { "sgx": true, "section-size": 1024, "flc": true } }

the domain capabilities: <sgx> <flc>yes</flc> <epc_size>1</epc_size> </sgx>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Haibin Huang <haibin.huang@intel.com> --- src/qemu/qemu_capabilities.c | 230 ++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 + .../caps_6.2.0.x86_64.replies | 30 ++- .../caps_6.2.0.x86_64.xml | 7 + .../caps_7.0.0.x86_64.replies | 34 ++- .../caps_7.0.0.x86_64.xml | 11 + .../caps_7.1.0.x86_64.replies | 34 ++- .../caps_7.1.0.x86_64.xml | 11 + 8 files changed, 346 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2c3be3ecec..57b5acb150 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -671,6 +671,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "chardev.qemu-vdagent", /* QEMU_CAPS_CHARDEV_QEMU_VDAGENT */ "display-dbus", /* QEMU_CAPS_DISPLAY_DBUS */ "iothread.thread-pool-max", /* QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX */ + "sgx-epc", /* QEMU_CAPS_SGX_EPC */ );

@@ -752,6 +753,8 @@ struct _virQEMUCaps {

virSEVCapability *sevCapabilities;

+ virSGXCapability *sgxCapabilities; + /* Capabilities which may differ depending on the accelerator. */ virQEMUCapsAccel kvm; virQEMUCapsAccel hvf; @@ -1394,6 +1397,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST }, { "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI }, { "virtio-iommu-pci", QEMU_CAPS_DEVICE_VIRTIO_IOMMU_PCI }, + { "sgx-epc", QEMU_CAPS_SGX_EPC }, };

@@ -1972,6 +1976,32 @@ virQEMUCapsSEVInfoCopy(virSEVCapability **dst, }

+static int +virQEMUCapsSGXInfoCopy(virSGXCapability **dst, + virSGXCapability *src) +{ + g_autoptr(virSGXCapability) tmp = NULL; +

For a convenience of caller, we can have a src == NULL check here and return early.

+ tmp = g_new0(virSGXCapability, 1); + + tmp->flc = src->flc; + tmp->sgx1 = src->sgx1; + tmp->sgx2 = src->sgx2; + tmp->section_size = src->section_size; + + if (src->nSections == 0) { + tmp->nSections = 0; + tmp->pSections = NULL; + } else { + tmp->nSections = src->nSections; + tmp->pSections = src->pSections;

Ouch, this will definitely lead to a double free. If I run qemucapabilitiestest after this patch I can see it clearly. I don't quite understand why you didn't though. Fortunately, we can use plain memcpy() since virSection struct does not contain any pointer, just a pair of integers.

+ } + + *dst = g_steal_pointer(&tmp); + return 0; +} + + static void virQEMUCapsAccelCopyMachineTypes(virQEMUCapsAccel *dst, virQEMUCapsAccel *src) @@ -2053,6 +2083,12 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps) qemuCaps->sevCapabilities) < 0) return NULL;

+ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC) && + virQEMUCapsSGXInfoCopy(&ret->sgxCapabilities, + qemuCaps->sgxCapabilities) < 0) + return NULL; + return g_steal_pointer(&ret); }

@@ -2091,6 +2127,7 @@ void virQEMUCapsDispose(void *obj) virCPUDataFree(qemuCaps->cpuData);

virSEVCapabilitiesFree(qemuCaps->sevCapabilities); + virSGXCapabilitiesFree(qemuCaps->sgxCapabilities);

virQEMUCapsAccelClear(&qemuCaps->kvm); virQEMUCapsAccelClear(&qemuCaps->hvf); @@ -2616,6 +2653,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps) }

+virSGXCapabilityPtr +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps) +{ + return qemuCaps->sgxCapabilities; +} + + static int virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps, qemuMonitor *mon) @@ -3442,6 +3486,31 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCaps *qemuCaps, }

+static int +virQEMUCapsProbeQMPSGXCapabilities(virQEMUCaps *qemuCaps, + qemuMonitor *mon) +{ + int rc = -1; + virSGXCapability *caps = NULL; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC)) + return 0; + + if ((rc = qemuMonitorGetSGXCapabilities(mon, &caps)) < 0) + return -1; + + /* SGX isn't actually supported */ + if (rc == 0) { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_SGX_EPC); + return 0; + } + + virSGXCapabilitiesFree(qemuCaps->sgxCapabilities); + qemuCaps->sgxCapabilities = caps; + return 0; +} + + /* * Filter for features which should never be passed to QEMU. Either because * QEMU never supported them or they were dropped as they never did anything @@ -4220,6 +4289,116 @@ virQEMUCapsParseSEVInfo(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) }

+static int +virQEMUCapsParseSGXInfo(virQEMUCaps *qemuCaps, + xmlXPathContextPtr ctxt) +{ + g_autoptr(virSGXCapability) sgx = NULL; + xmlNodePtr node; + + g_autofree xmlNodePtr *nodes = NULL; + g_autofree xmlNodePtr *sectionNodes = NULL; + g_autofree char *flc = NULL; + g_autofree char *sgx1 = NULL; + g_autofree char *sgx2 = NULL; + + int n = 0; + int nsections = 0;

No need for extra empty lines. It's okay if this is one block.

+ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC)) + return 0; + + if (virXPathBoolean("boolean(./sgx)", ctxt) == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing SGX platform data in QEMU capabilities cache")); + return -1; + } + + sgx = g_new0(virSGXCapability, 1); + + if ((!(flc = virXPathString("string(./sgx/flc)", ctxt))) || + virStringParseYesNo(flc, &sgx->flc) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing or invalid SGX platform flc in QEMU capabilities cache")); + return -1; + } + + if ((!(sgx1 = virXPathString("string(./sgx/sgx1)", ctxt))) || + virStringParseYesNo(sgx1, &sgx->sgx1) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing or invalid SGX platform sgx1 in QEMU capabilities cache")); + return -1; + } + + if ((!(sgx2 = virXPathString("string(./sgx/sgx2)", ctxt))) || + virStringParseYesNo(sgx2, &sgx->sgx2) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing or invalid SGX platform sgx2 in QEMU capabilities cache")); + return -1; + } + + if (virXPathULongLong("string(./sgx/section_size)", ctxt, &sgx->section_size) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing or malformed SGX platform section_size in QEMU capabilities cache")); + return -1; + } + + if ((n = virXPathNodeSet("./sgx/sections", ctxt, &nodes)) < 0) {

Here, were intrested in a single occurrance, thus virXPathNode() could be used.

+ sgx->nSections = 0; + sgx->pSections = NULL; + VIR_INFO("Sections was not obtained, so QEMU version is 6.2.0");

Again, no need for extra NOPs, VIR_INFO()...

+ qemuCaps->sgxCapabilities = g_steal_pointer(&sgx); + return 0; + } + + if (n == 0) { + qemuCaps->sgxCapabilities = g_steal_pointer(&sgx); + return 0; + } + + // Got the section, the QEMU version is above 7.0.0

We like c89 style of comments.

+ node = ctxt->node; + ctxt->node = nodes[0]; + nsections = virXPathNodeSet("./section", ctxt, §ionNodes); + ctxt->node = node; + + if (nsections < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse CPU blockers in QEMU capabilities")); + return -1; + } + + if (nsections > 0) { + size_t i; + g_autofree char * strNode = NULL; + g_autofree char * strSize = NULL; + sgx->nSections = nsections; + sgx->pSections = g_new0(virSection, nsections + 1); + + for (i = 0; i < nsections; i++) { + if ((strNode = virXMLPropString(sectionNodes[i], "node")) && + (virStrToLong_ui(strNode, NULL, 10, &(sgx->pSections[i].node)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing node name in QEMU " + "capabilities cache"));

Error messages are extempt from 80 chars line rule. The reason is: simpler git-grep. I, as a developer, can take whatever portion of error message and 'git grep' it and find it easily. But if the message is broken into multiple lines it's more tricky to do.

+ return -1; + } + + if ((strSize = virXMLPropString(sectionNodes[i], "size")) && + (virStrToLong_ull(strSize, NULL, 10, &(sgx->pSections[i].size)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing size name in QEMU " + "capabilities cache")); + return -1; + } + } + } + + qemuCaps->sgxCapabilities = g_steal_pointer(&sgx); + return 0; +} + + static int virQEMUCapsParseFlags(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) { @@ -4522,6 +4701,9 @@ virQEMUCapsLoadCache(virArch hostArch, if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0) return -1;

+ if (virQEMUCapsParseSGXInfo(qemuCaps, ctxt) < 0) + return -1; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF)) @@ -4707,6 +4889,49 @@ virQEMUCapsFormatSEVInfo(virQEMUCaps *qemuCaps, virBuffer *buf) }

+static void +virQEMUCapsFormatSGXInfo(virQEMUCaps *qemuCaps, + virBuffer *buf) +{ + virSGXCapabilityPtr sgx = virQEMUCapsGetSGXCapabilities(qemuCaps); + size_t i; + + virBufferAddLit(buf, "<sgx supported='yes'>\n"); + virBufferAdjustIndent(buf, 2); + if (sgx->flc) { + virBufferAsprintf(buf, "<flc>%s</flc>\n", "yes"); + } else { + virBufferAsprintf(buf, "<flc>%s</flc>\n", "no"); + }

Well, this works. But how about: if (sgx->flc) { virBufferAddLit(buf, "<flc>yes</flc>\n"); } else { virBufferAddLit(buf, "<flc>no</flc>\n"); } which saves and extra call to printf() for a static string? Or even better: virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no");

+ if (sgx->sgx1) { + virBufferAsprintf(buf, "<sgx1>%s</sgx1>\n", "yes"); + } else { + virBufferAsprintf(buf, "<sgx1>%s</sgx1>\n", "no"); + } + if (sgx->sgx2) { + virBufferAsprintf(buf, "<sgx2>%s</sgx2>\n", "yes"); + } else { + virBufferAsprintf(buf, "<sgx2>%s</sgx2>\n", "no"); + } + virBufferAsprintf(buf, "<section_size unit='KiB'>%llu</section_size>\n", sgx->section_size); + + if (sgx->nSections > 0) { + virBufferAddLit(buf, "<sections>\n"); + + for (i = 0; i < sgx->nSections; i++) { + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<section node='%u' ", sgx->pSections[i].node); + virBufferAsprintf(buf, "size='%llu'/>\n", sgx->pSections[i].size); + virBufferAdjustIndent(buf, -2); + } + virBufferAddLit(buf, "</sections>\n"); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</sgx>\n"); +} + + char * virQEMUCapsFormatCache(virQEMUCaps *qemuCaps) { @@ -4788,6 +5013,9 @@ virQEMUCapsFormatCache(virQEMUCaps *qemuCaps) if (qemuCaps->sevCapabilities) virQEMUCapsFormatSEVInfo(qemuCaps, &buf);

+ if (qemuCaps->sgxCapabilities) + virQEMUCapsFormatSGXInfo(qemuCaps, &buf); + if (qemuCaps->kvmSupportsNesting) virBufferAddLit(&buf, "<kvmSupportsNesting/>\n");

@@ -5455,6 +5683,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps, return -1; if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) return -1; + if (virQEMUCapsProbeQMPSGXCapabilities(qemuCaps, mon) < 0) + return -1;

virQEMUCapsInitProcessCaps(qemuCaps);

diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 6f35ba1485..fc8c0fde1b 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -650,6 +650,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_CHARDEV_QEMU_VDAGENT, /* -chardev qemu-vdagent */ QEMU_CAPS_DISPLAY_DBUS, /* -display dbus */ QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX, /* -object iothread.thread-pool-max */ + QEMU_CAPS_SGX_EPC, /* -object sgx-epc,... */

QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; @@ -843,6 +844,9 @@ virQEMUCapsCPUFeatureFromQEMU(virQEMUCaps *qemuCaps, virSEVCapability * virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps);

+virSGXCapabilityPtr +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps); + bool virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) G_GNUC_NO_INLINE;

diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies index e235532d62..0151ab07fa 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies @@ -7459,15 +7459,15 @@ "type": "bool" }, { - "name": "sgx1", + "name": "flc", "type": "bool" }, { - "name": "sgx2", + "name": "sgx1", "type": "bool" }, { - "name": "flc", + "name": "sgx2", "type": "bool" }, {

This move does not seem to be warranted. When Peter generated the file I'm quite certain that this was genuine order of attributes in which QEMU replied. Furthermore, nothing in our code relies on ordering of these particular attributes. Why is this necessary? In fact, the QEMU I've built from git recently (v7.0.0-2668-gf9d9fff72e) replied in this order: {"return": {"sgx": true, "sgx2": false, "sgx1": true, "sections": [{"size": 98041856, "node": 0}], "section-size": 98041856, "flc": false}, "id": "libvirt-50"} Michal

Ok, nice comments Thank you very much!

-----Original Message----- From: Michal Prívozník <mprivozn@redhat.com> Sent: Wednesday, July 20, 2022 7:27 PM To: Yang, Lin A <lin.a.yang@intel.com>; libvir-list@redhat.com; Huang, Haibin <haibin.huang@intel.com>; Ding, Jian-feng <jian-feng.ding@intel.com> Subject: Re: [libvirt][PATCH v13 4/6] conf: expose SGX feature in domain capabilities

...

From: Haibin Huang <haibin.huang@intel.com>

Extend hypervisor capabilities to include sgx feature. When available, the hypervisor supports launching an VM with SGX on Intel platfrom. The SGX feature tag privides additional details like section size and sgx1 or sgx2.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Haibin Huang <haibin.huang@intel.com> --- docs/formatdomaincaps.rst | 40 ++++++++++++++++ src/conf/domain_capabilities.c | 48 +++++++++++++++++++ src/conf/schemas/domaincaps.rng | 42 ++++++++++++++++ src/qemu/qemu_capabilities.c | 28 +++++++++++ tests/domaincapsdata/bhyve_basic.x86_64.xml | 1 + tests/domaincapsdata/bhyve_fbuf.x86_64.xml | 1 + tests/domaincapsdata/bhyve_uefi.x86_64.xml | 1 + tests/domaincapsdata/empty.xml | 1 + tests/domaincapsdata/libxl-xenfv.xml | 1 + tests/domaincapsdata/libxl-xenpv.xml | 1 + .../domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_2.11.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + .../qemu_2.12.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_2.12.0.s390x.xml | 1 + tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + .../domaincapsdata/qemu_3.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_3.0.0.x86_64.xml | 1 + .../domaincapsdata/qemu_3.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_3.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_4.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 1 + .../qemu_4.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_4.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_4.0.0.x86_64.xml | 1 + .../domaincapsdata/qemu_4.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 + .../qemu_4.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.s390x.xml | 1 + tests/domaincapsdata/qemu_4.2.0.x86_64.xml | 1 + .../domaincapsdata/qemu_5.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 + .../qemu_5.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_5.0.0.x86_64.xml | 1 + .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_5.1.0.sparc.xml | 1 + tests/domaincapsdata/qemu_5.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_5.2.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 + .../qemu_5.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_5.2.0.s390x.xml | 1 + tests/domaincapsdata/qemu_5.2.0.x86_64.xml | 1 + .../domaincapsdata/qemu_6.0.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml | 1 + .../qemu_6.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_6.0.0.x86_64.xml | 1 + .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_6.1.0.x86_64.xml | 1 + .../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 6 +++ .../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 6 +++ .../qemu_6.2.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_6.2.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_6.2.0.x86_64.xml | 6 +++ .../domaincapsdata/qemu_7.0.0-q35.x86_64.xml | 10 ++++ .../domaincapsdata/qemu_7.0.0-tcg.x86_64.xml | 10 ++++ .../qemu_7.0.0-virt.aarch64.xml | 1 + tests/domaincapsdata/qemu_7.0.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_7.0.0.x86_64.xml | 10 ++++ .../domaincapsdata/qemu_7.1.0-q35.x86_64.xml | 10 ++++ .../domaincapsdata/qemu_7.1.0-tcg.x86_64.xml | 10 ++++ tests/domaincapsdata/qemu_7.1.0.x86_64.xml | 10 ++++ 88 files changed, 311 insertions(+)

diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst index 933469b2a2..d3bdee28d4 100644 --- a/docs/formatdomaincaps.rst +++ b/docs/formatdomaincaps.rst @@ -519,6 +519,16 @@ capabilities. All features occur as children of the

On 7/1/22 21:14, Lin Yang wrote: main ``features`` element.

...

<cbitpos>47</cbitpos> <reduced-phys-bits>1</reduced-phys-bits> </sev> + <sgx supported='yes'> + <flc>no</flc> + <sgx1>yes</sgx1> + <sgx2>no</sgx2> + <section_size>2</section_size> + <sections> + <section node='0' size='1'/> + <section node='1' size='1'/> + </sections> + </sgx> </features> </domainCapabilities>

@@ -598,3 +608,33 @@ in domain XML <formatdomain.html#launch-security>`__ ``maxESGuests`` The maximum number of SEV-ES guests that can be launched on the host.

This

...

value may be configurable in the firmware for some hosts. + +SGX capabilities +^^^^^^^^^^^^^^^^ + +Intel Software Guard Extensions (Intel SGX) capabilities are exposed +under the ``sgx`` element. + +Intel SGX helps protect data in use via unique application isolation

technology.

...

+Protect selected code and data from modification using hardened +enclaves with Intel SGX. + +For more details on the SGX feature, please follow resources in the +SGX developer's document store. In order to use SGX with libvirt have +a look at formatdomain.rst Memory devices. + +``flc`` + FLC (Flexible Launch Control), not strictly part of SGX2, but was not part of + original SGX hardware either. + +``sgx1`` + the sgx version 1. + +``sgx2`` + The sgx version 2. + +``section_size`` + The size of the SGX enclave page cache (called EPC). + +``sections`` + The sections of the SGX enclave page cache (called EPC). diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 27f3fb8d36..fa29f69807 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -98,6 +98,7 @@ virDomainCapsDispose(void *obj) virObjectUnref(caps->cpu.custom); virCPUDefFree(caps->cpu.hostModel); virSEVCapabilitiesFree(caps->sev); + virSGXCapabilitiesFree(caps->sgx);

values = &caps->os.loader.values; for (i = 0; i < values->nvalues; i++) @@ -620,6 +621,52 @@ virDomainCapsFeatureSEVFormat(virBuffer *buf, return; }

+static void +virDomainCapsFeatureSGXFormat(virBuffer *buf, + const virSGXCapability *sgx) { + size_t i; + + if (!sgx) { + virBufferAddLit(buf, "<sgx supported='no'/>\n"); + } else { + virBufferAddLit(buf, "<sgx supported='yes'>\n"); + virBufferAdjustIndent(buf, 2); + if (sgx->flc) { + virBufferAsprintf(buf, "<flc>%s</flc>\n", "yes"); + } else { + virBufferAsprintf(buf, "<flc>%s</flc>\n", "no"); + }

Same comment here as in the previous patch.

...

+ if (sgx->sgx1) { + virBufferAsprintf(buf, "<sgx1>%s</sgx1>\n", "yes"); + } else { + virBufferAsprintf(buf, "<sgx1>%s</sgx1>\n", "no"); + } + if (sgx->sgx2) { + virBufferAsprintf(buf, "<sgx2>%s</sgx2>\n", "yes"); + } else { + virBufferAsprintf(buf, "<sgx2>%s</sgx2>\n", "no"); + } + virBufferAsprintf(buf, "<section_size + unit='KiB'>%llu</section_size>\n", sgx->section_size); + + if (sgx->nSections > 0) { + virBufferAddLit(buf, "<sections>\n"); + + for (i = 0; i < sgx->nSections; i++) { + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<section node='%u' ", sgx- pSections[i].node); + virBufferAsprintf(buf, "size='%llu'/>\n", sgx->pSections[i].size); + virBufferAdjustIndent(buf, -2); + } + virBufferAddLit(buf, "</sections>\n"); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</sgx>\n"); + } + + return; +}

static void virDomainCapsFormatFeatures(const virDomainCaps *caps, @@ -640,6 +687,7 @@ virDomainCapsFormatFeatures(const virDomainCaps *caps, }

virDomainCapsFeatureSEVFormat(&childBuf, caps->sev); + virDomainCapsFeatureSGXFormat(&childBuf, caps->sgx);

virXMLFormatElement(buf, "features", NULL, &childBuf); } diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng index 9cbc2467ab..6975ebacb9 100644 --- a/src/conf/schemas/domaincaps.rng +++ b/src/conf/schemas/domaincaps.rng @@ -270,6 +270,9 @@ <optional> <ref name="sev"/> </optional> + <optional> + <ref name="sgx"/> + </optional> </element> </define>

@@ -330,6 +333,45 @@ </element> </define>

+ <define name="sgx"> + <element name="sgx"> + <ref name="supported"/> + <optional> + <element name="flc"> + <data type="string"/>

Or, since we know we are formatting yes/no, we can use virYesNo instead of too generic string type.

...

+ </element> + <element name="sgx1"> + <data type="string"/> + </element> + <element name="sgx2"> + <data type="string"/> + </element> + <element name="section_size"> + <attribute name="unit"> + <value>KiB</value> + </attribute> + <data type="unsignedInt"/> + </element> + <optional> + <element name="sections"> + <interleave>

I think the order is well defined and thus we don't really need <interleave/> here. The domain capabilities XML is not provided by user, it's generated by libvirt, in virDomainCapsFormat() where the order is well defined.

...

+ <zeroOrMore> + <element name="section"> + <attribute name="node"> + <data type="string"/>

This is an integer ..

...

+ </attribute> + <attribute name="size"> + <data type="string"/>

.. and so is this.

...

+ </attribute> + </element> + </zeroOrMore> + </interleave> + </element> + </optional> + </optional> + </element> + </define> + <define name="value"> <zeroOrMore> <element name="value"> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 57b5acb150..598c694738 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -6744,6 +6744,33 @@ virQEMUCapsFillDomainFeatureS390PVCaps(virQEMUCaps *qemuCaps, } }

+/** + * virQEMUCapsFillDomainFeatureiSGXCaps: + * @qemuCaps: QEMU capabilities + * @domCaps: domain capabilities + * + * Take the information about SGX capabilities that has been obtained + * using the 'query-sgx-capabilities' QMP command and stored in +@qemuCaps + * and convert it to a form suitable for @domCaps. + */ +static void +virQEMUCapsFillDomainFeatureSGXCaps(virQEMUCaps *qemuCaps, + virDomainCaps *domCaps) { + virSGXCapability *cap = qemuCaps->sgxCapabilities; + + if (!cap) + return; + + domCaps->sgx = g_new0(virSGXCapability, 1); + + domCaps->sgx->flc = cap->flc; + domCaps->sgx->sgx1 = cap->sgx1; + domCaps->sgx->sgx2 = cap->sgx2; + domCaps->sgx->section_size = cap->section_size; + domCaps->sgx->nSections = cap->nSections; + domCaps->sgx->pSections = cap->pSections;

This is a verbatim copy of virQEMUCapsSGXInfoCopy(), any reason for not just call the function?

...

+}

Michal

On 7/1/22 21:14, Lin Yang wrote:

With NUMA config:

<devices> ... <memory model='sgx-epc'> <source> <nodemask>0-1</nodemask> </source> <target> <size unit='KiB'>512</size> <node>0</node> </target> </memory> ... </devices>

Without NUMA config:

<devices> ... <memory model='sgx-epc'> <target> <size unit='KiB'>512</size> </target> </memory> ... </devices>

Signed-off-by: Lin Yang <lin.a.yang@intel.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.rst | 27 +++++++- src/conf/domain_conf.c | 27 ++++++++ src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 9 +++ src/conf/schemas/domaincommon.rng | 1 + src/qemu/qemu_alias.c | 3 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 48 ++++++++++---- src/qemu/qemu_domain_address.c | 6 ++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_process.c | 2 + src/qemu/qemu_validate.c | 8 +++ src/security/security_apparmor.c | 1 + src/security/security_dac.c | 2 + src/security/security_selinux.c | 2 + tests/qemuxml2argvdata/sgx-epc-numa.xml | 50 +++++++++++++++ tests/qemuxml2argvdata/sgx-epc.xml | 36 +++++++++++ .../sgx-epc-numa.x86_64-latest.xml | 64 +++++++++++++++++++ .../sgx-epc.x86_64-6.2.0.xml | 52 +++++++++++++++ tests/qemuxml2xmltest.c | 3 + 20 files changed, 329 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2argvdata/sgx-epc-numa.xml create mode 100644 tests/qemuxml2argvdata/sgx-epc.xml create mode 100644 tests/qemuxml2xmloutdata/sgx-epc-numa.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/sgx-epc.x86_64-6.2.0.xml

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 62a94890f0..b95c930d73 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7910,6 +7910,20 @@ Example: usage of the memory devices <current unit='KiB'>524288</current> </target> </memory> + <memory model='sgx-epc'> + <source> + <nodemask>0-1</nodemask> + </source> + <target> + <size unit='KiB'>16384</size> + <node>0</node> + </target> + </memory> + <memory model='sgx-epc'> + <target> + <size unit='KiB'>16384</size> + </target> + </memory> </devices> ...

@@ -7918,7 +7932,9 @@ Example: usage of the memory devices 1.2.14` Provide ``nvdimm`` model that adds a Non-Volatile DIMM module. :since:`Since 3.2.0` Provide ``virtio-pmem`` model to add a paravirtualized persistent memory device. :since:`Since 7.1.0` Provide ``virtio-mem`` model - to add paravirtualized memory device. :since:`Since 7.9.0` + to add paravirtualized memory device. :since:`Since 7.9.0` Provide + ``sgx-epc`` model to add a SGX enclave page cache (EPC) memory to the guest. + :since:`Since 8.6.0 and QEMU 6.2.0`

``access`` An optional attribute ``access`` ( :since:`since 3.2.0` ) that provides @@ -7978,6 +7994,13 @@ Example: usage of the memory devices Represents a path in the host that backs the virtio memory module in the guest. It is mandatory.

+ For model ``sgx-epc`` this element is optional. The following optional + elements may be used: + + ``nodemask`` + This element can be used to override the default set of NUMA nodes where + the memory would be allocated. :since:`Since 8.6.0 and QEMU 7.0.0` + ``target`` The mandatory ``target`` element configures the placement and sizing of the added memory from the perspective of the guest. @@ -7988,6 +8011,8 @@ Example: usage of the memory devices

The ``node`` subelement configures the guest NUMA node to attach the memory to. The element shall be used only if the guest has NUMA nodes configured. + For model ``sgx-epc`` this element is optional.

This looks redudnand. The sentence right before suggests that the element is optional.

It will be set to 0 as + default. :since:`Since 8.6.0 and QEMU 7.0.0`

This is a bit misleading as we don't report this back in the domain XML. If you really want to document the default then say it's hypervisor dependant. But I'd just leave these two sentences out.

The following optional elements may be used:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 709ca53790..f8b67eb375 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1431,6 +1431,7 @@ VIR_ENUM_IMPL(virDomainMemoryModel, "nvdimm", "virtio-pmem", "virtio-mem", + "sgx-epc", );

VIR_ENUM_IMPL(virDomainShmemModel, @@ -5680,6 +5681,7 @@ virDomainMemoryDefPostParse(virDomainMemoryDef *mem,

case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -14697,6 +14699,20 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, def->nvdimmPath = virXPathString("string(./path)", ctxt); break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + if ((nodemask = virXPathString("string(./nodemask)", ctxt))) { + if (virBitmapParse(nodemask, &def->sourceNodes, + VIR_DOMAIN_CPUMASK_LEN) < 0) + return -1; + + if (virBitmapIsAllClear(def->sourceNodes)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodemask': %s"), nodemask); + return -1; + } + } + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -14765,6 +14781,7 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; } @@ -16561,6 +16578,7 @@ virDomainMemoryFindByDefInternal(virDomainDef *def, continue; break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -26153,6 +26171,15 @@ virDomainMemorySourceDefFormat(virBuffer *buf, virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->nvdimmPath); break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + if (def->sourceNodes) { + if (!(bitmap = virBitmapFormat(def->sourceNodes))) + return -1; + + virBufferAsprintf(&childBuf, "<nodemask>%s</nodemask>\n", bitmap); + } + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4c8c42b7eb..a15798463c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2559,6 +2559,7 @@ typedef enum { VIR_DOMAIN_MEMORY_MODEL_NVDIMM, /* nvdimm memory device */ VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM, /* virtio-pmem memory device */ VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM, /* virtio-mem memory device */ + VIR_DOMAIN_MEMORY_MODEL_SGX_EPC, /* SGX enclave page cache */

VIR_DOMAIN_MEMORY_MODEL_LAST } virDomainMemoryModel; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 28b0481d4e..c8f03cb844 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2231,6 +2231,15 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, case VIR_DOMAIN_MEMORY_MODEL_DIMM: break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device address is not supported for model '%s'"), + virDomainMemoryModelTypeToString(mem->model)); + return -1; + } + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: default: diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index da2fb0d5cb..c6377b2e90 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6833,6 +6833,7 @@ <value>nvdimm</value> <value>virtio-pmem</value> <value>virtio-mem</value> + <value>sgx-epc</value> </choice> </attribute> <optional> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 7efd91051e..5ebeb82f92 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -515,6 +515,9 @@ qemuAssignDeviceMemoryAlias(virDomainDef *def, case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: prefix = "virtiomem"; break; + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + prefix = "epc"; + break; case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: default: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b307d3139c..7ca183ded7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4010,6 +4010,7 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, return NULL; break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: default: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 60ed358871..fc64a5ca9c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8448,6 +8448,7 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriver *driver, break;

case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -9129,6 +9130,12 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, } break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("hotplug is not supported for the %s device"), + virDomainMemoryModelTypeToString(mem->model)); + return -1; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: return -1; @@ -9164,7 +9171,7 @@ int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, const virDomainMemoryDef *mem) { - unsigned int nmems = def->nmems; + unsigned int hotplugNum = 0; unsigned long long hotplugSpace; unsigned long long hotplugMemory = 0; size_t i; @@ -9172,15 +9179,37 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, hotplugSpace = def->mem.max_memory - virDomainDefGetMemoryInitial(def);

if (mem) { - nmems++; + hotplugNum++; hotplugMemory = mem->size;

if (qemuDomainDefValidateMemoryHotplugDevice(mem, def) < 0) return -1; }

+ for (i = 0; i < def->nmems; i++) { + switch (def->mems[i]->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + hotplugMemory += def->mems[i]->size; + hotplugNum++; + /* already existing devices don't need to be checked on hotplug */ + if (!mem && + qemuDomainDefValidateMemoryHotplugDevice(def->mems[i], def) < 0) + return -1; + break; + + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + /* sgx epc memory does not support hotplug */ + case VIR_DOMAIN_MEMORY_MODEL_LAST: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + break; + } + } + if (!virDomainDefHasMemoryHotplug(def)) { - if (nmems) { + if (hotplugNum) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cannot use/hotplug a memory device when domain " "'maxMemory' is not defined")); @@ -9203,22 +9232,13 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, } }

- if (nmems > def->mem.memory_slots) { + if (hotplugNum > def->mem.memory_slots) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("memory device count '%u' exceeds slots count '%u'"), - nmems, def->mem.memory_slots); + hotplugNum, def->mem.memory_slots); return -1; }

- for (i = 0; i < def->nmems; i++) { - hotplugMemory += def->mems[i]->size; - - /* already existing devices don't need to be checked on hotplug */ - if (!mem && - qemuDomainDefValidateMemoryHotplugDevice(def->mems[i], def) < 0) - return -1; - } - if (hotplugMemory > hotplugSpace) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("memory device total size exceeds hotplug space")); diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 753733d1b9..a111ae4d0c 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -389,6 +389,7 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDef *def, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; } @@ -1039,6 +1040,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: return 0; } @@ -2421,6 +2423,7 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; } @@ -3081,6 +3084,7 @@ qemuDomainAssignMemoryDeviceSlot(virDomainObj *vm, return qemuDomainEnsurePCIAddress(vm, &dev); break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -3107,6 +3111,7 @@ qemuDomainReleaseMemoryDeviceSlot(virDomainObj *vm, qemuDomainReleaseDeviceAddress(vm, &mem->info); break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -3140,6 +3145,7 @@ qemuDomainAssignMemorySlots(virDomainDef *def) case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: /* handled in qemuDomainAssignPCIAddresses() */ break; + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3b5c3db67c..79a6c420c0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7156,6 +7156,7 @@ qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef, case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot modify memory of model '%s'"), diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 771a623ef7..b907387f21 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3935,6 +3935,7 @@ qemuProcessDomainMemoryDefNeedHugepagesPath(const virDomainMemoryDef *mem, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: /* None of these can be backed by hugepages. */ return false; @@ -4009,6 +4010,7 @@ qemuProcessNeedMemoryBackingPath(virDomainDef *def, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: /* Backed by user provided path. Not stored in memory * backing dir anyway. */ diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 39210ba65b..fbbfedd095 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -5173,6 +5173,14 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDef *mem, } break;

+ case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sgx epc isn't supported by this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 008384dee8..36e8ce42b5 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -687,6 +687,7 @@ AppArmorSetMemoryLabel(virSecurityManager *mgr, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; } diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 21cebae694..d94995c9cf 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1853,6 +1853,7 @@ virSecurityDACRestoreMemoryLabel(virSecurityManager *mgr,

case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: case VIR_DOMAIN_MEMORY_MODEL_NONE: ret = 0; @@ -2040,6 +2041,7 @@ virSecurityDACSetMemoryLabel(virSecurityManager *mgr,

case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: case VIR_DOMAIN_MEMORY_MODEL_NONE: ret = 0; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e2f34a27dc..6657663483 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1580,6 +1580,7 @@ virSecuritySELinuxSetMemoryLabel(virSecurityManager *mgr, case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; } @@ -1608,6 +1609,7 @@ virSecuritySELinuxRestoreMemoryLabel(virSecurityManager *mgr,

case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: ret = 0; diff --git a/tests/qemuxml2argvdata/sgx-epc-numa.xml b/tests/qemuxml2argvdata/sgx-epc-numa.xml new file mode 100644 index 0000000000..f28b06dd79 --- /dev/null +++ b/tests/qemuxml2argvdata/sgx-epc-numa.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>2</vcpu> + <cpu> + <numa> + <cell id="0" cpus="0" memory="109550" unit="KiB"/> + <cell id="1" cpus="1" memory="109550" unit="KiB"/> + </numa> + </cpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='usb' index='0' model='none'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memory model='sgx-epc'> + <source> + <nodemask>0-1</nodemask> + </source> + <target> + <size unit='MiB'>64</size> + <node>0</node> + </target> + </memory> + <memory model='sgx-epc'> + <source> + <nodemask>2-3</nodemask> + </source> + <target> + <size unit='MiB'>16</size> + <node>1</node> + </target> + </memory> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/sgx-epc.xml b/tests/qemuxml2argvdata/sgx-epc.xml new file mode 100644 index 0000000000..65ae8ae296 --- /dev/null +++ b/tests/qemuxml2argvdata/sgx-epc.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='usb' index='0' model='none'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memory model='sgx-epc'> + <target> + <size unit='MiB'>64</size> + </target> + </memory> + <memory model='sgx-epc'> + <target> + <size unit='MiB'>16</size> + </target> + </memory> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/sgx-epc-numa.x86_64-latest.xml b/tests/qemuxml2xmloutdata/sgx-epc-numa.x86_64-latest.xml new file mode 100644 index 0000000000..ceab82110c --- /dev/null +++ b/tests/qemuxml2xmloutdata/sgx-epc-numa.x86_64-latest.xml

This, and the other xmloutdata file can be just a symlink to their originals in xml2argvdata/ dirs. Michal

On 7/1/22 21:14, Lin Yang wrote:

According to the result parsing from xml, add the argument of SGX EPC memory backend into QEMU command line.

With NUMA config:

#qemu-system-x86_64 \ ...... \ -object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864,"host-nodes":[0,1],"policy":"bind"}' \ -object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216,"host-nodes":[2,3],"policy":"bind"}' \ -machine sgx-epc.0.memdev=memepc0,sgx-epc.0.node=0,sgx-epc.1.memdev=memepc1,sgx-epc.1.node=1

Without NUMA config:

#qemu-system-x86_64 \ ...... \ -object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864}' \ -object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216}' \ -machine sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1

Except, ...

Signed-off-by: Lin Yang <lin.a.yang@intel.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Haibin Huang <haibin.huang@intel.com> --- src/qemu/qemu_alias.c | 3 +- src/qemu/qemu_command.c | 86 ++++++++++++++++++- src/qemu/qemu_monitor_json.c | 41 +++++++-- .../sgx-epc-numa.x86_64-latest.args | 40 +++++++++ .../sgx-epc.x86_64-6.2.0.args | 37 ++++++++ tests/qemuxml2argvtest.c | 3 + 6 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/sgx-epc-numa.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args

--- /dev/null +++ b/tests/qemuxml2argvdata/sgx-epc-numa.x86_64-latest.args @@ -0,0 +1,40 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine q35,usb=off,dump-guest-core=off,sgx-epc.0.memdev=memepc0,sgx-epc.0.node=0,sgx-epc.1.memdev=memepc1,sgx-epc.1.node=1 \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 2,sockets=2,cores=1,threads=1 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node0","size":112197632}' \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node1","size":112197632}' \ +-numa node,nodeid=1,cpus=1,memdev=ram-node1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ +-device '{"driver":"pcie-root-port","port":9,"chassis":2,"id":"pci.2","bus":"pcie.0","addr":"0x1.0x1"}' \ +-object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864,"host-nodes":[0,1],"policy":"bind"}' \ +-object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216,"host-nodes":[2,3],"policy":"bind"}' \

... nor here ...

+-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.1","addr":"0x0"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args b/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args new file mode 100644 index 0000000000..56c476b777 --- /dev/null +++ b/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc-q35-6.2,usb=off,dump-guest-core=off,memory-backend=pc.ram,sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1 \ +-accel tcg \ +-cpu qemu64 \ +-m 134 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":140509184}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device pcie-root-port,port=8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x1 \ +-device pcie-root-port,port=9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \ +-object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864}' \ +-object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216}' \

... nor here is epc backend generated before -machine argument which makes QEMU fail at startup: qemu-system-x86_64: Invalid parameter type for 'memdev', expected: memory-backend-epc At least I think that's the root cause. Michal