Re: [PATCH v9 4/5] qemu: Add command-line to generate SGX EPC memory backend

[Michal Prívozník <mprivozn@xxxxxxxxxx>]

On 12/15/21 04:40, Haibin Huang wrote:
> From: Lin Yang <lin.a.yang@xxxxxxxxx>
> 
> According to the result parsing from xml, add the argument of
> SGX EPC memory backend into QEMU command line:
> 
>     #qemu-system-x86_64 \
>         ...... \
>         -object memory-backend-epc,id=mem1,size=64M,prealloc=on \
>         -object memory-backend-epc,id=mem2,size=28M \
>         -M sgx-epc.0.memdev=mem1,sgx-epc.1.memdev=mem2
> 
> Signed-off-by: Lin Yang <lin.a.yang@xxxxxxxxx>
> ---
>  src/qemu/qemu_alias.c   |  3 ++-
>  src/qemu/qemu_command.c | 40 ++++++++++++++++++++++++++++++++++++----
>  src/qemu/qemu_domain.c  | 10 +++++++++-
>  3 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index 5795924754..89afea8778 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -489,7 +489,8 @@ qemuDeviceMemoryGetAliasID(virDomainDef *def,
>       * valid */
>      if (!oldAlias &&
>          mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM &&
> -        mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM)
> +        mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM &&
> +        mem->model != VIR_DOMAIN_MEMORY_MODEL_SGX_EPC)
>          return mem->info.addr.dimm.slot;
>  
>      for (i = 0; i < def->nmems; i++) {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 36281a69e2..ebb3aa1023 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3555,6 +3555,10 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
>          if (systemMemory)
>              disableCanonicalPath = true;
>  
> +    } else if (mem->model == VIR_DOMAIN_MEMORY_MODEL_SGX_EPC) {
> +        backendType = "memory-backend-epc";
> +        if (!priv->memPrealloc)
> +            prealloc = true;
>      } else {
>          backendType = "memory-backend-ram";
>      }
> @@ -7838,6 +7842,8 @@ qemuBuildMemoryDeviceCommandLine(virCommand *cmd,
>                                   qemuDomainObjPrivate *priv)
>  {
>      size_t i;
> +    g_auto(virBuffer) epcBuf = VIR_BUFFER_INITIALIZER;
> +    int epcNum = 0;
>  
>      /* memory hotplug requires NUMA to be enabled - we already checked
>       * that memory devices are present only when NUMA is */
> @@ -7847,11 +7853,37 @@ qemuBuildMemoryDeviceCommandLine(virCommand *cmd,
>          if (qemuBuildMemoryDimmBackendStr(cmd, def->mems[i], def, cfg, priv) < 0)
>              return -1;
>  
> -        if (!(props = qemuBuildMemoryDeviceProps(def, def->mems[i])))
> -            return -1;
> +        switch ((virDomainMemoryModel) def->mems[i]->model) {
> +        case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
> +        case VIR_DOMAIN_MEMORY_MODEL_DIMM:
> +        case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
> +        case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
> +            if (!(props = qemuBuildMemoryDeviceProps(def, def->mems[i])))
> +                return -1;
>  
> -        if (qemuBuildDeviceCommandlineFromJSON(cmd, props, priv->qemuCaps) < 0)
> -            return -1;
> +            if (qemuBuildDeviceCommandlineFromJSON(cmd, props, priv->qemuCaps) < 0)
> +                return -1;
> +
> +            break;
> +
> +        case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
> +            if (virBufferUse(&epcBuf) > 0)
> +                virBufferAddChar(&epcBuf, ',');
> +
> +            virBufferAsprintf(&epcBuf, "sgx-epc.%d.memdev=%s", epcNum++,
> +                              g_strdup_printf("mem%s", def->mems[i]->info.alias));

IIUC, there's also .node= attribute which tells QEMU which NUMA node
should the memory be at. Should this be also reflected? The NUMA node is
stored in to def->mems[i]->targetNode. Mind you, this is guest NUMA node
I'm talking about. Does the attribute refer to the host NUMA node?

Also, virBufferAsprintf() hold its promise and behaves like asprintf().
There's no need for additional g_strdup_printf(), more so when it's leaked.

Finally, there are multiple ways that hugepages can sneak in. For
instance the following input generates memory-backend-file instead of
memory-backend-epc:

<domain type='qemu'>
  <name>QEMUGuest1</name>
  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
  <memory unit='KiB'>219100</memory>
  <currentMemory unit='KiB'>219100</currentMemory>
  <memoryBacking>
    <hugepages>
      <page size='2048' unit='KiB'/>
    </hugepages>
  </memoryBacking>
  <vcpu placement='static'>1</vcpu>
  ...
  <devices>
    <emulator>/usr/bin/qemu-system-x86_64</emulator>
    ...
    <memory model='sgx-epc'>
      <target>
        <size unit='KiB'>65536</size>
      </target>
      <address type='dimm' slot='2'/>
    </memory>
  </devices>
</domain>


Michal