<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=Windows-1252"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> On 5/30/22, 6:09 AM, "Michal Prívozník" <mprivozn@redhat.com> wrote:<o:p></o:p> > On 5/18/22 09:59, Haibin Huang wrote:<o:p></o:p> > > From: Lin Yang <lin.a.yang@intel.com><o:p></o:p> > > <o:p></o:p> > > According to the result parsing from xml, add the argument of<o:p></o:p> > > SGX EPC memory backend into QEMU command line:<o:p></o:p> > > <o:p></o:p> > > #qemu-system-x86_64 \<o:p></o:p> > > ...... \<o:p></o:p> > > -object memory-backend-epc,id=memepc0,size=64M,prealloc=on \<o:p></o:p> > > -object memory-backend-epc,id=memepc1,size=28M \<o:p></o:p> > > -machine sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1<o:p></o:p> > > <o:p></o:p> > > Signed-off-by: Lin Yang <lin.a.yang@intel.com><o:p></o:p> > > Signed-off-by: Haibin Huang <haibin.huang@intel.com><o:p></o:p> > > ---<o:p></o:p> > > src/qemu/qemu_alias.c | 3 +-<o:p></o:p> > > src/qemu/qemu_command.c | 53 +++++++++++++++++--<o:p></o:p> > > .../sgx-epc.x86_64-6.2.0.args | 37 +++++++++++++<o:p></o:p> > > tests/qemuxml2argvtest.c | 2 +<o:p></o:p> > > 4 files changed, 90 insertions(+), 5 deletions(-)<o:p></o:p> > > create mode 100644 tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args<o:p></o:p> > > <o:p></o:p> > > diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c<o:p></o:p> > > index e5a946cbed..03c79bcf0e 100644<o:p></o:p> > > --- a/src/qemu/qemu_alias.c<o:p></o:p> > > +++ b/src/qemu/qemu_alias.c<o:p></o:p> > > @@ -467,7 +467,8 @@ qemuDeviceMemoryGetAliasID(virDomainDef *def,<o:p></o:p> > > * valid */<o:p></o:p> > > if (!oldAlias &&<o:p></o:p> > > mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM &&<o:p></o:p> > > - mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM)<o:p></o:p> > > + mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM &&<o:p></o:p> > > + mem->model != VIR_DOMAIN_MEMORY_MODEL_SGX_EPC)<o:p></o:p> > > return mem->info.addr.dimm.slot;<o:p></o:p> > > <o:p></o:p> > > for (i = 0; i < def->nmems; i++) {<o:p></o:p> > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c<o:p></o:p> > > index 4807b137b6..9c83f0e168 100644<o:p></o:p> > > --- a/src/qemu/qemu_command.c<o:p></o:p> > > +++ b/src/qemu/qemu_command.c<o:p></o:p> > > @@ -3774,6 +3774,10 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,<o:p></o:p> > > if (systemMemory)<o:p></o:p> > > disableCanonicalPath = true;<o:p></o:p> > > <o:p></o:p> > > + } else if (mem->model == VIR_DOMAIN_MEMORY_MODEL_SGX_EPC) {<o:p></o:p> > > + backendType = "memory-backend-epc";<o:p></o:p> > > + if (!priv->memPrealloc)<o:p></o:p> > > + prealloc = true;<o:p></o:p> > > } else if (useHugepage || mem->nvdimmPath || memAccess ||<o:p></o:p> > > def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {<o:p></o:p> > > <o:p></o:p> > > @@ -3934,6 +3938,11 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,<o:p></o:p> > > _("this qemu doesn't support the "<o:p></o:p> > > "memory-backend-memfd object"));<o:p></o:p> > > return -1;<o:p></o:p> > > + } else if (STREQ(backendType, "memory-backend-epc") &&<o:p></o:p> > > + !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SGX_EPC)) {<o:p></o:p> > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",<o:p></o:p> > > + _("this qemu doesn't support the memory-backend-epc object"));<o:p></o:p> > > + return -1;<o:p></o:p> > > }<o:p></o:p> > > <o:p></o:p> > > rc = 0;<o:p></o:p> > > @@ -6979,6 +6988,7 @@ qemuBuildMachineCommandLine(virCommand *cmd,<o:p></o:p> > > virCPUDef *cpu = def->cpu;<o:p></o:p> > > g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;<o:p></o:p> > > size_t i;<o:p></o:p> > > + int epcNum = 0;<o:p></o:p> > > <o:p></o:p> > > virCommandAddArg(cmd, "-machine");<o:p></o:p> > > virBufferAdd(&buf, def->os.machine, -1);<o:p></o:p> > > @@ -7199,6 +7209,25 @@ qemuBuildMachineCommandLine(virCommand *cmd,<o:p></o:p> > > virBufferAddLit(&buf, ",graphics=off");<o:p></o:p> > > }<o:p></o:p> > > <o:p></o:p> > > + /* add sgx epc memory to -machine parameter */<o:p></o:p> > > + for (i = 0; i < def->nmems; i++) {<o:p></o:p> > > + switch ((virDomainMemoryModel) def->mems[i]->model) {<o:p></o:p> > > + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:<o:p></o:p> > > + virBufferAsprintf(&buf, ",sgx-epc.%d.memdev=mem%s", epcNum++,<o:p></o:p> > > + def->mems[i]->info.alias);<o:p></o:p> > <o:p></o:p> > So there really isn't any better way to specify sgx-epc than through -M?<o:p></o:p> > This way libvirt loses capability to set th device @id attribute which<o:p></o:p> > means we have to jump through hoops (like in your patch 2/6) when the<o:p></o:p> > @id value is expected.<o:p></o:p> > <o:p></o:p> > I vaguely pointing this out earlier (but maybe I just thought about<o:p></o:p> > pointing it out and never did - it's been a while since I've reviewed<o:p></o:p> > these patches), but I don't know what the consensus was.<o:p></o:p> <o:p> </o:p> I was not involved in the QEMU SGX patches review process and don’t<o:p></o:p> know the reason why it use -machine instead of -device. The initial<o:p></o:p> patch introduce separate QEMU argument -sgx-epc, but finally was<o:p></o:p> updated to -machine according to the comments.<o:p></o:p> <o:p> </o:p> <a href="https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg00644.html">https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg00644.html</a> <o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Lin. <o:p></o:p> </div> </body> </html>